Commit 43103335 authored by Wes Kocher's avatar Wes Kocher
Browse files

Backed out changeset 7f7f0a43f051 (bug 956899) for testThreadingExclusiveData failures CLOSED TREE

MozReview-Commit-ID: BHrjbFNJxfW

--HG--
rename : js/src/jsapi-tests/testThreadingExclusiveData.cpp => js/src/jsapi-tests/testMutex.cpp
rename : js/src/threading/ExclusiveData.cpp => js/src/vm/Mutex.cpp
rename : js/src/threading/ExclusiveData.h => js/src/vm/Mutex.h
parent 7d10d8fc
Loading
Loading
Loading
Loading
+1 −1
Original line number Diff line number Diff line
@@ -59,6 +59,7 @@ UNIFIED_SOURCES += [
    'testLooselyEqual.cpp',
    'testMappedArrayBuffer.cpp',
    'testMutedErrors.cpp',
    'testMutex.cpp',
    'testNewObject.cpp',
    'testNewTargetInvokeConstructor.cpp',
    'testNullRoot.cpp',
@@ -82,7 +83,6 @@ UNIFIED_SOURCES += [
    'testStringBuffer.cpp',
    'testStructuredClone.cpp',
    'testSymbol.cpp',
    'testThreadingExclusiveData.cpp',
    'testToIntWidth.cpp',
    'testTypedArrays.cpp',
    'testUbiNode.cpp',
+21 −16
Original line number Diff line number Diff line
@@ -7,18 +7,17 @@
#include "mozilla/IntegerRange.h"
#include "js/Vector.h"
#include "jsapi-tests/tests.h"
#include "threading/ExclusiveData.h"
#include "vm/Mutex.h"

// One thread for each bit in our counter.
const static uint8_t numThreads = 64;
const static bool showDiagnostics = false;

struct CounterAndBit
{
    uint8_t bit;
    const js::ExclusiveData<uint64_t>& counter;
    const js::Mutex<uint64_t>& counter;

    CounterAndBit(uint8_t bit, const js::ExclusiveData<uint64_t>& counter)
    CounterAndBit(uint8_t bit, const js::Mutex<uint64_t>& counter)
      : bit(bit)
      , counter(counter)
    {
@@ -26,10 +25,14 @@ struct CounterAndBit
    }
};

const static bool shouldPrint = false;

void
printDiagnosticMessage(uint64_t seen)
{
    if (showDiagnostics) {
    if (!shouldPrint)
        return;

    fprintf(stderr, "Thread %p saw ", PR_GetCurrentThread());
    for (auto i : mozilla::MakeRange(numThreads)) {
        if (seen & (uint64_t(1) << i))
@@ -39,7 +42,6 @@ printDiagnosticMessage(uint64_t seen)
    }
    fprintf(stderr, "\n");
}
}

void
setBitAndCheck(void* arg)
@@ -67,9 +69,12 @@ setBitAndCheck(void* arg)
    }
}

BEGIN_TEST(testExclusiveData)
BEGIN_TEST(testMutex)
{
    js::ExclusiveData<uint64_t> counter(0);
    auto maybeCounter = js::Mutex<uint64_t>::Create(0);
    CHECK(maybeCounter.isSome());

    js::Mutex<uint64_t> counter(mozilla::Move(*maybeCounter));

    js::Vector<PRThread*> threads(cx);
    CHECK(threads.reserve(numThreads));
@@ -94,4 +99,4 @@ BEGIN_TEST(testExclusiveData)

    return true;
}
END_TEST(testExclusiveData)
END_TEST(testMutex)
+1 −1
Original line number Diff line number Diff line
@@ -294,7 +294,6 @@ UNIFIED_SOURCES += [
    'proxy/ScriptedIndirectProxyHandler.cpp',
    'proxy/SecurityWrapper.cpp',
    'proxy/Wrapper.cpp',
    'threading/ExclusiveData.cpp',
    'vm/ArgumentsObject.cpp',
    'vm/ArrayBufferObject.cpp',
    'vm/CallNonGenericMethod.cpp',
@@ -314,6 +313,7 @@ UNIFIED_SOURCES += [
    'vm/JSONParser.cpp',
    'vm/MemoryMetrics.cpp',
    'vm/Monitor.cpp',
    'vm/Mutex.cpp',
    'vm/NativeObject.cpp',
    'vm/ObjectGroup.cpp',
    'vm/PIC.cpp',
+7 −7
Original line number Diff line number Diff line
@@ -4,34 +4,34 @@
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "threading/ExclusiveData.h"
#include "vm/Mutex.h"

namespace js {

/* static */ mozilla::Maybe<detail::ExclusiveDataBase>
detail::ExclusiveDataBase::Create()
/* static */ mozilla::Maybe<detail::MutexBase>
detail::MutexBase::Create()
{
    auto lock = PR_NewLock();
    if (!lock)
        return mozilla::Nothing();

    return mozilla::Some(detail::ExclusiveDataBase(lock));
    return mozilla::Some(detail::MutexBase(lock));
}

detail::ExclusiveDataBase::~ExclusiveDataBase()
detail::MutexBase::~MutexBase()
{
    if (lock_)
        PR_DestroyLock(lock_);
}

void
detail::ExclusiveDataBase::acquire() const
detail::MutexBase::acquire() const
{
    PR_Lock(lock_);
}

void
detail::ExclusiveDataBase::release() const
detail::MutexBase::release() const
{
    MOZ_RELEASE_ASSERT(PR_Unlock(lock_) == PR_SUCCESS);
}
+35 −37
Original line number Diff line number Diff line
@@ -4,8 +4,8 @@
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef threading_ExclusiveData_h
#define threading_ExclusiveData_h
#ifndef js_Mutex_h
#define js_Mutex_h

#include "mozilla/Maybe.h"
#include "mozilla/Move.h"
@@ -16,33 +16,33 @@ namespace js {

namespace detail {

class ExclusiveDataBase
class MutexBase
{
  private:
    mutable PRLock* lock_;

    ExclusiveDataBase(const ExclusiveDataBase&) = delete;
    ExclusiveDataBase& operator=(const ExclusiveDataBase&) = delete;
    MutexBase(const MutexBase&) = delete;
    MutexBase& operator=(const MutexBase&) = delete;

  public:
    // This move constructor is only public for `mozilla::Forward`.
    ExclusiveDataBase(ExclusiveDataBase&& rhs)
    MutexBase(MutexBase&& rhs)
      : lock_(rhs.lock_)
    {
        MOZ_ASSERT(&rhs != this, "self-move disallowed!");
        rhs.lock_ = nullptr;
    }

    ~ExclusiveDataBase();
    ~MutexBase();

  protected:
    explicit ExclusiveDataBase(PRLock* lock)
    explicit MutexBase(PRLock* lock)
      : lock_(lock)
    {
        MOZ_ASSERT(lock_);
    }

    static mozilla::Maybe<ExclusiveDataBase> Create();
    static mozilla::Maybe<MutexBase> Create();

    void acquire() const;
    void release() const;
@@ -53,7 +53,7 @@ class ExclusiveDataBase
/**
 * A mutual exclusion lock class.
 *
 * `ExclusiveData` provides an RAII guard to automatically lock and unlock when
 * `Mutex` provides an RAII guard to automatically lock and unlock when
 * accessing the protected inner value.
 *
 * Unlike the STL's `std::mutex`, the protected value is internal to this
@@ -77,8 +77,7 @@ class ExclusiveDataBase
 *
 *     class SharedCounter
 *     {
 *         // Remember to acquire `counter_lock` when accessing `counter`,
 *         // pretty please!
 *         // Remember to acquire `counter_lock` when accessing `counter`, pretty please!
 *         Counter counter;
 *         std::mutex counter_lock;
 *
@@ -89,12 +88,12 @@ class ExclusiveDataBase
 *         }
 *     };
 *
 * In contrast, `ExclusiveData` wraps the protected value, enabling the type
 * system to enforce that we acquire the lock before accessing the value:
 * In contrast, `Mutex` wraps the protected value, enabling the type system to
 * enforce that we acquire the lock before accessing the value:
 *
 *     class SharedCounter
 *     {
 *         ExclusiveData<Counter> counter;
 *         Mutex<Counter> counter;
 *
 *       public:
 *         void inc(size_t n) {
@@ -107,57 +106,56 @@ class ExclusiveDataBase
 *
 * [0]: Of course, we don't have a borrow checker in C++, so the type system
 *      cannot guarantee that you don't stash references received from
 *      `ExclusiveData<T>::Guard` somewhere such that the reference outlives the
 *      guard's lifetime and therefore becomes invalid. To help avoid this last
 *      `Mutex<T>::Guard` somewhere such that the reference outlives the guard's
 *      lifetime and therefore becomes invalid. To help avoid this last
 *      foot-gun, prefer using the guard directly! Do not store raw references
 *      to the protected value in other structures!
 */
template <typename T>
class ExclusiveData : private detail::ExclusiveDataBase
class Mutex : private detail::MutexBase
{
    mutable T value_;

    ExclusiveData(const ExclusiveData&) = delete;
    ExclusiveData& operator=(const ExclusiveData&) = delete;
    Mutex(const Mutex&) = delete;
    Mutex& operator=(const Mutex&) = delete;

    template <typename U>
    explicit ExclusiveData(U&& u, ExclusiveDataBase&& base)
      : ExclusiveDataBase(mozilla::Move(base))
    explicit Mutex(U&& u, MutexBase&& base)
      : MutexBase(mozilla::Move(base))
      , value_(mozilla::Forward<U>(u))
    { }

  public:
    /**
     * Create a new `ExclusiveData`, with perfect forwarding of the protected
     * value.
     * Create a new `Mutex`, with perfect forwarding of the protected value.
     *
     * On success, `mozilla::Some` is returned. On failure, `mozilla::Nothing`
     * is returned.
     */
    template <typename U>
    static mozilla::Maybe<ExclusiveData<T>> Create(U&& u) {
        auto base = detail::ExclusiveDataBase::Create();
    static mozilla::Maybe<Mutex<T>> Create(U&& u) {
        auto base = detail::MutexBase::Create();
        if (base.isNothing())
            return mozilla::Nothing();
        return mozilla::Some(ExclusiveData(mozilla::Forward<U>(u), mozilla::Move(*base)));
        return mozilla::Some(Mutex(mozilla::Forward<U>(u), mozilla::Move(*base)));
    }

    ExclusiveData(ExclusiveData&& rhs)
      : ExclusiveDataBase(mozilla::Move(static_cast<ExclusiveDataBase&&>(rhs)))
    Mutex(Mutex&& rhs)
      : MutexBase(mozilla::Move(static_cast<MutexBase&&>(rhs)))
      , value_(mozilla::Move(rhs.value_))
    {
        MOZ_ASSERT(&rhs != this, "self-move disallowed!");
    }

    ExclusiveData& operator=(ExclusiveData&& rhs) {
        this->~ExclusiveData();
        new (this) ExclusiveData(mozilla::Move(rhs));
    Mutex& operator=(Mutex&& rhs) {
        this->~Mutex();
        new (this) Mutex(mozilla::Move(rhs));
        return *this;
    }

    /**
     * An RAII class that provides exclusive access to a `ExclusiveData<T>`'s
     * protected inner `T` value.
     * An RAII class that provides exclusive access to a `Mutex<T>`'s protected
     * inner `T` value.
     *
     * Note that this is intentionally marked MOZ_STACK_CLASS instead of
     * MOZ_RAII_CLASS, as the latter disallows moves and returning by value, but
@@ -165,13 +163,13 @@ class ExclusiveData : private detail::ExclusiveDataBase
     */
    class MOZ_STACK_CLASS Guard
    {
        const ExclusiveData* parent_;
        const Mutex* parent_;

        Guard(const Guard&) = delete;
        Guard& operator=(const Guard&) = delete;

      public:
        explicit Guard(const ExclusiveData& parent)
        explicit Guard(const Mutex& parent)
          : parent_(&parent)
        {
            parent_->acquire();
@@ -214,4 +212,4 @@ class ExclusiveData : private detail::ExclusiveDataBase

} // namespace js

#endif // threading_ExclusiveData_h
#endif // js_Mutex_h