Unreviewed, rolling out r219060.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 3 Jul 2017 23:12:40 +0000 (23:12 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 3 Jul 2017 23:12:40 +0000 (23:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174108

crashing constantly when initializing UIWebView (Requested by
thorton on #webkit).

Reverted changeset:

"WTF::Thread should have the threads stack bounds."
https://bugs.webkit.org/show_bug.cgi?id=173975
http://trac.webkit.org/changeset/219060

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@219105 268f45cc-cd09-0410-ab3c-d52691b4dbfc

19 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/heap/MachineStackMarker.cpp
Source/JavaScriptCore/heap/MachineStackMarker.h
Source/JavaScriptCore/runtime/InitializeThreading.cpp
Source/JavaScriptCore/runtime/VM.cpp
Source/JavaScriptCore/runtime/VM.h
Source/JavaScriptCore/runtime/VMEntryScope.cpp
Source/JavaScriptCore/runtime/VMInlines.h
Source/JavaScriptCore/runtime/VMTraps.cpp
Source/JavaScriptCore/yarr/YarrPattern.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/StackBounds.h
Source/WTF/wtf/StackStats.cpp
Source/WTF/wtf/Threading.cpp
Source/WTF/wtf/Threading.h
Source/WTF/wtf/ThreadingPthreads.cpp
Source/WTF/wtf/ThreadingWin.cpp
Source/WTF/wtf/WTFThreadData.cpp
Source/WTF/wtf/WTFThreadData.h

index bbe4fdf..f1c9ca6 100644 (file)
@@ -1,3 +1,17 @@
+2017-07-03  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r219060.
+        https://bugs.webkit.org/show_bug.cgi?id=174108
+
+        crashing constantly when initializing UIWebView (Requested by
+        thorton on #webkit).
+
+        Reverted changeset:
+
+        "WTF::Thread should have the threads stack bounds."
+        https://bugs.webkit.org/show_bug.cgi?id=173975
+        http://trac.webkit.org/changeset/219060
+
 2017-07-03  Matt Lewis  <jlewis3@apple.com>
 
         Unreviewed, rolling out r219103.
index 8b9bb68..6763ea3 100644 (file)
@@ -239,6 +239,9 @@ void MachineThreads::gatherFromCurrentThread(ConservativeRoots& conservativeRoot
 MachineThreads::MachineThread::MachineThread()
     : m_thread(WTF::Thread::current())
 {
+    auto stackBounds = wtfThreadData().stack();
+    m_stackBase = stackBounds.origin();
+    m_stackEnd = stackBounds.end();
 }
 
 size_t MachineThreads::MachineThread::getRegisters(MachineThread::Registers& registers)
@@ -299,15 +302,15 @@ static inline int osRedZoneAdjustment()
 
 std::pair<void*, size_t> MachineThreads::MachineThread::captureStack(void* stackTop)
 {
-    char* begin = reinterpret_cast_ptr<char*>(stackBase());
+    char* begin = reinterpret_cast_ptr<char*>(m_stackBase);
     char* end = bitwise_cast<char*>(WTF::roundUpToMultipleOf<sizeof(void*)>(reinterpret_cast<uintptr_t>(stackTop)));
     ASSERT(begin >= end);
 
     char* endWithRedZone = end + osRedZoneAdjustment();
     ASSERT(WTF::roundUpToMultipleOf<sizeof(void*)>(reinterpret_cast<uintptr_t>(endWithRedZone)) == reinterpret_cast<uintptr_t>(endWithRedZone));
 
-    if (endWithRedZone < stackEnd())
-        endWithRedZone = reinterpret_cast_ptr<char*>(stackEnd());
+    if (endWithRedZone < m_stackEnd)
+        endWithRedZone = reinterpret_cast_ptr<char*>(m_stackEnd);
 
     std::swap(begin, endWithRedZone);
     return std::make_pair(begin, endWithRedZone - begin);
index c1510d6..b62eb43 100644 (file)
@@ -72,10 +72,12 @@ public:
         std::pair<void*, size_t> captureStack(void* stackTop);
 
         WTF::ThreadIdentifier threadID() const { return m_thread->id(); }
-        void* stackBase() const { return m_thread->stack().origin(); }
-        void* stackEnd() const { return m_thread->stack().end(); }
+        void* stackBase() const { return m_stackBase; }
+        void* stackEnd() const { return m_stackEnd; }
 
         Ref<WTF::Thread> m_thread;
+        void* m_stackBase;
+        void* m_stackEnd;
         MachineThread* m_next { nullptr };
         MachineThread* m_prev { nullptr };
     };
@@ -102,7 +104,7 @@ private:
 #define DECLARE_AND_COMPUTE_CURRENT_THREAD_STATE(stateName) \
     CurrentThreadState stateName; \
     stateName.stackTop = &stateName; \
-    stateName.stackOrigin = Thread::current().stack().origin(); \
+    stateName.stackOrigin = wtfThreadData().stack().origin(); \
     ALLOCATE_AND_GET_REGISTER_STATE(stateName ## _registerState); \
     stateName.registerState = &stateName ## _registerState
 
index 2e81427..9ef143a 100644 (file)
@@ -75,7 +75,8 @@ void initializeThreading()
         DisallowVMReentry::initialize();
 #endif
         initializeSuperSampler();
-        wtfThreadData().setSavedLastStackTop(Thread::current().stack().origin());
+        WTFThreadData& threadData = wtfThreadData();
+        threadData.setSavedLastStackTop(threadData.stack().origin());
 
 #if ENABLE(WEBASSEMBLY)
         Wasm::Thunks::initialize();
index 6e6863f..48727a6 100644 (file)
@@ -211,7 +211,7 @@ VM::VM(VMType vmType, HeapType heapType)
     , m_shadowChicken(std::make_unique<ShadowChicken>())
 {
     interpreter = new Interpreter(*this);
-    StackBounds stack = Thread::current().stack();
+    StackBounds stack = wtfThreadData().stack();
     updateSoftReservedZoneSize(Options::softReservedZoneSize());
     setLastStackTop(stack.origin());
 
@@ -671,7 +671,7 @@ inline void VM::updateStackLimits()
     void* lastSoftStackLimit = m_softStackLimit;
 #endif
 
-    const StackBounds& stack = Thread::current().stack();
+    const StackBounds& stack = wtfThreadData().stack();
     size_t reservedZoneSize = Options::reservedZoneSize();
     // We should have already ensured that Options::reservedZoneSize() >= minimumReserveZoneSize at
     // options initialization time, and the option value should not have been changed thereafter.
@@ -885,9 +885,9 @@ size_t VM::committedStackByteCount()
 #if ENABLE(JIT)
     // When using the C stack, we don't know how many stack pages are actually
     // committed. So, we use the current stack usage as an estimate.
-    ASSERT(Thread::current().stack().isGrowingDownward());
+    ASSERT(wtfThreadData().stack().isGrowingDownward());
     int8_t* current = reinterpret_cast<int8_t*>(&current);
-    int8_t* high = reinterpret_cast<int8_t*>(Thread::current().stack().origin());
+    int8_t* high = reinterpret_cast<int8_t*>(wtfThreadData().stack().origin());
     return high - current;
 #else
     return CLoopStack::committedByteCount();
index 2e75991..db5af35 100644 (file)
@@ -689,7 +689,7 @@ private:
 
     bool isSafeToRecurse(void* stackLimit) const
     {
-        ASSERT(Thread::current().stack().isGrowingDownward());
+        ASSERT(wtfThreadData().stack().isGrowingDownward());
         void* curr = reinterpret_cast<void*>(&curr);
         return curr >= stackLimit;
     }
index d11a8ff..c2d9bda 100644 (file)
@@ -41,7 +41,7 @@ VMEntryScope::VMEntryScope(VM& vm, JSGlobalObject* globalObject)
     , m_globalObject(globalObject)
 {
     ASSERT(!DisallowVMReentry::isInEffectOnCurrentThread());
-    ASSERT(Thread::current().stack().isGrowingDownward());
+    ASSERT(wtfThreadData().stack().isGrowingDownward());
     if (!vm.entryScope) {
         vm.entryScope = this;
 
index bc0c25f..5a77619 100644 (file)
@@ -34,7 +34,7 @@ namespace JSC {
 bool VM::ensureStackCapacityFor(Register* newTopOfStack)
 {
 #if ENABLE(JIT)
-    ASSERT(Thread::current().stack().isGrowingDownward());
+    ASSERT(wtfThreadData().stack().isGrowingDownward());
     return newTopOfStack >= m_softStackLimit;
 #else
     return ensureStackCapacityForCLoop(newTopOfStack);
index 3d84ab9..02fa4fd 100644 (file)
@@ -269,7 +269,25 @@ protected:
                     return;
 
                 Thread& thread = *ownerThread->get();
-                vm.traps().tryInstallTrapBreakpoints(context, thread.stack());
+                StackBounds stackBounds = StackBounds::emptyBounds();
+                {
+                    // FIXME: We need to use the machine threads because it is the only non-TLS source
+                    // for the stack bounds of this thread. We should keep in on the WTF::Thread instead.
+                    // see: https://bugs.webkit.org/show_bug.cgi?id=173975
+                    MachineThreads& machineThreads = vm.heap.machineThreads();
+                    auto machineThreadsLock = tryHoldLock(machineThreads.getLock());
+                    if (!machineThreadsLock)
+                        return; // Try again later.
+
+                    auto& threadList = machineThreads.threadsListHead(machineThreadsLock);
+                    for (MachineThreads::MachineThread* machineThread = threadList.head(); machineThread; machineThread = machineThread->next()) {
+                        if (machineThread->m_thread.get() == thread)
+                            stackBounds = StackBounds(machineThread->stackBase(), machineThread->stackEnd());
+                    }
+                    RELEASE_ASSERT(!stackBounds.isEmpty());
+                }
+
+                vm.traps().tryInstallTrapBreakpoints(context, stackBounds);
             });
         }
 
index 5f27306..1c2dd75 100644 (file)
@@ -880,7 +880,7 @@ private:
     {
         if (!m_stackLimit)
             return true;
-        ASSERT(Thread::current().stack().isGrowingDownward());
+        ASSERT(wtfThreadData().stack().isGrowingDownward());
         int8_t* curr = reinterpret_cast<int8_t*>(&curr);
         int8_t* limit = reinterpret_cast<int8_t*>(m_stackLimit);
         return curr >= limit;
index ed9b732..d93683e 100644 (file)
@@ -1,3 +1,17 @@
+2017-07-03  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r219060.
+        https://bugs.webkit.org/show_bug.cgi?id=174108
+
+        crashing constantly when initializing UIWebView (Requested by
+        thorton on #webkit).
+
+        Reverted changeset:
+
+        "WTF::Thread should have the threads stack bounds."
+        https://bugs.webkit.org/show_bug.cgi?id=173975
+        http://trac.webkit.org/changeset/219060
+
 2017-07-03  Matt Lewis  <jlewis3@apple.com>
 
         Unreviewed, rolling out r219103.
index 3c911ef..cd62c14 100644 (file)
@@ -40,7 +40,7 @@ class StackBounds {
     const static size_t s_defaultAvailabilityDelta = 64 * 1024;
 
 public:
-    static constexpr StackBounds emptyBounds() { return StackBounds(); }
+    static StackBounds emptyBounds() { return StackBounds(); }
 
     static StackBounds currentThreadStackBounds()
     {
@@ -127,9 +127,9 @@ public:
     }
 
 private:
-    constexpr StackBounds()
-        : m_origin(nullptr)
-        , m_bound(nullptr)
+    StackBounds()
+        : m_origin(0)
+        , m_bound(0)
     {
     }
 
index 56130b0..064f2f0 100644 (file)
@@ -59,7 +59,7 @@ int StackStats::s_maxLayoutReentryDepth = 0;
 
 StackStats::PerThreadStats::PerThreadStats()
 {
-    const StackBounds& stack = Thread::current().stack();
+    const StackBounds& stack = wtfThreadData().stack();
     m_reentryDepth = 0;
     m_stackStart = (char*)stack.origin();
     m_currentCheckPoint = 0;
index 82bcfb9..ddc28c2 100644 (file)
 
 namespace WTF {
 
-enum class Stage {
-    Start, Initialized
-};
-
 struct NewThreadContext {
+    WTF_MAKE_FAST_ALLOCATED;
+public:
     const char* name;
     Function<void()> entryPoint;
-    Stage stage;
-    Mutex mutex;
-    ThreadCondition condition;
+    Mutex creationMutex;
 };
 
 const char* Thread::normalizeThreadName(const char* threadName)
@@ -88,46 +84,31 @@ const char* Thread::normalizeThreadName(const char* threadName)
 static void threadEntryPoint(void* contextData)
 {
     NewThreadContext* context = static_cast<NewThreadContext*>(contextData);
-    Function<void()> entryPoint;
+
+    // Block until our creating thread has completed any extra setup work, including
+    // establishing ThreadIdentifier.
     {
-        // Block until our creating thread has completed any extra setup work, including establishing ThreadIdentifier.
-        MutexLocker locker(context->mutex);
+        MutexLocker locker(context->creationMutex);
+    }
 
-        Thread::initializeCurrentThreadInternal(context->name);
-        entryPoint = WTFMove(context->entryPoint);
+    Thread::initializeCurrentThreadInternal(context->name);
 
-        // Ack completion of initialization to the creating thread.
-        context->stage = Stage::Initialized;
-        context->condition.signal();
-    }
+    auto entryPoint = WTFMove(context->entryPoint);
+
+    // Delete the context before starting the thread.
+    delete context;
 
     entryPoint();
 }
 
 RefPtr<Thread> Thread::create(const char* name, Function<void()>&& entryPoint)
 {
-    NewThreadContext context { name, WTFMove(entryPoint), Stage::Start, { }, { } };
+    NewThreadContext* context = new NewThreadContext { name, WTFMove(entryPoint), { } };
 
-    MutexLocker locker(context.mutex);
-    RefPtr<Thread> result = Thread::createInternal(threadEntryPoint, &context, name);
-    // After establishing Thread, release the mutex and wait for completion of initialization.
-    while (context.stage != Stage::Initialized)
-        context.condition.wait(context.mutex);
+    // Prevent the thread body from executing until we've established the thread identifier.
+    MutexLocker locker(context->creationMutex);
 
-    return result;
-}
-
-Thread* Thread::currentMayBeNull()
-{
-    ThreadHolder* data = ThreadHolder::current();
-    if (data)
-        return &data->thread();
-    return nullptr;
-}
-
-void Thread::initialize()
-{
-    m_stack = StackBounds::currentThreadStackBounds();
+    return Thread::createInternal(threadEntryPoint, context, name);
 }
 
 void Thread::didExit()
index 165d319..b74f381 100644 (file)
@@ -38,7 +38,6 @@
 #include <wtf/Function.h>
 #include <wtf/PlatformRegisters.h>
 #include <wtf/RefPtr.h>
-#include <wtf/StackBounds.h>
 #include <wtf/ThreadSafeRefCounted.h>
 
 #if USE(PTHREADS) && !OS(DARWIN)
@@ -68,7 +67,7 @@ public:
 
     // Returns Thread object.
     WTF_EXPORT_PRIVATE static Thread& current();
-    static Thread* currentMayBeNull();
+    WTF_EXPORT_PRIVATE static Thread* currentMayBeNull();
 
     // Returns ThreadIdentifier directly. It is useful if the user only cares about identity
     // of threads. At that time, users should know that holding this ThreadIdentifier does not ensure
@@ -109,7 +108,6 @@ public:
     // Called in the thread during initialization.
     // Helpful for platforms where the thread name must be set from within the thread.
     static void initializeCurrentThreadInternal(const char* threadName);
-    static void initializeCurrentThreadEvenIfNonWTFCreated();
 
     WTF_EXPORT_PRIVATE void dump(PrintStream& out) const;
 
@@ -127,11 +125,6 @@ public:
 
     static void initializePlatformThreading();
 
-    const StackBounds& stack() const
-    {
-        return m_stack;
-    }
-
 #if OS(DARWIN)
     mach_port_t machThread() { return m_platformThread; }
 #endif
@@ -147,7 +140,6 @@ protected:
 #else
     void establish(HANDLE, ThreadIdentifier);
 #endif
-    void initialize();
 
 #if USE(PTHREADS) && !OS(DARWIN)
     static void signalHandlerSuspendResume(int, siginfo_t*, void* ucontext);
@@ -179,7 +171,6 @@ protected:
     std::mutex m_mutex;
     ThreadIdentifier m_id { 0 };
     JoinableState m_joinableState { Joinable };
-    StackBounds m_stack { StackBounds::emptyBounds() };
     bool m_didExit { false };
 #if USE(PTHREADS)
     pthread_t m_handle;
index af8e67e..d6c0a17 100644 (file)
@@ -188,9 +188,8 @@ void Thread::initializePlatformThreading()
 #endif
 }
 
-void Thread::initializeCurrentThreadEvenIfNonWTFCreated()
+static void initializeCurrentThreadEvenIfNonWTFCreated()
 {
-    Thread::current().initialize();
 #if !OS(DARWIN)
     sigset_t mask;
     sigemptyset(&mask);
@@ -300,17 +299,25 @@ void Thread::detach()
         didBecomeDetached();
 }
 
+Thread* Thread::currentMayBeNull()
+{
+    ThreadHolder* data = ThreadHolder::current();
+    if (data)
+        return &data->thread();
+    return nullptr;
+}
+
 Thread& Thread::current()
 {
     if (Thread* current = currentMayBeNull())
         return *current;
 
     // Not a WTF-created thread, ThreadIdentifier is not established yet.
-    Ref<Thread> thread = adoptRef(*new Thread());
+    RefPtr<Thread> thread = adoptRef(new Thread());
     thread->establish(pthread_self());
-    ThreadHolder::initialize(thread.get());
+    ThreadHolder::initialize(*thread);
     initializeCurrentThreadEvenIfNonWTFCreated();
-    return thread.get();
+    return *thread;
 }
 
 ThreadIdentifier Thread::currentID()
index b4752ef..bd615fb 100644 (file)
@@ -116,11 +116,6 @@ Thread::~Thread()
         CloseHandle(m_handle);
 }
 
-void Thread::initializeCurrentThreadEvenIfNonWTFCreated()
-{
-    Thread::current().initialize();
-}
-
 // MS_VC_EXCEPTION, THREADNAME_INFO, and setThreadNameInternal all come from <http://msdn.microsoft.com/en-us/library/xcb2z8hs.aspx>.
 static const DWORD MS_VC_EXCEPTION = 0x406D1388;
 
@@ -150,7 +145,6 @@ void Thread::initializeCurrentThreadInternal(const char* szThreadName)
     } __except (EXCEPTION_CONTINUE_EXECUTION) {
     }
 #endif
-    initializeCurrentThreadEvenIfNonWTFCreated();
 }
 
 void Thread::initializePlatformThreading()
@@ -267,8 +261,9 @@ size_t Thread::getRegisters(PlatformRegisters& registers)
 
 Thread& Thread::current()
 {
-    if (Thread* current = currentMayBeNull())
-        return *current;
+    ThreadHolder* data = ThreadHolder::current();
+    if (data)
+        return data->thread();
 
     // Not a WTF-created thread, ThreadIdentifier is not established yet.
     Ref<Thread> thread = adoptRef(*new Thread());
@@ -279,7 +274,6 @@ Thread& Thread::current()
 
     thread->establish(handle, currentID());
     ThreadHolder::initialize(thread.get(), Thread::currentID());
-    initializeCurrentThreadEvenIfNonWTFCreated();
     return thread.get();
 }
 
index f3a3e8a..cdf3dcb 100644 (file)
@@ -44,11 +44,12 @@ WTFThreadData::WTFThreadData()
     , m_currentAtomicStringTable(0)
     , m_defaultAtomicStringTable(0)
     , m_atomicStringTableDestructor(0)
+    , m_stackBounds(StackBounds::currentThreadStackBounds())
 #if ENABLE(STACK_STATS)
     , m_stackStats()
 #endif
     , m_savedStackPointerAtVMEntry(0)
-    , m_savedLastStackTop(Thread::current().stack().origin())
+    , m_savedLastStackTop(stack().origin())
 {
     AtomicStringTable::create(*this);
     m_currentAtomicStringTable = m_defaultAtomicStringTable;
index 84e3c30..a43a6eb 100644 (file)
@@ -29,6 +29,7 @@
 
 #include <wtf/FastTLS.h>
 #include <wtf/Noncopyable.h>
+#include <wtf/StackBounds.h>
 #include <wtf/StackStats.h>
 #include <wtf/ThreadSpecific.h>
 
@@ -56,6 +57,16 @@ public:
         return oldAtomicStringTable;
     }
 
+    const StackBounds& stack()
+    {
+        // We need to always get a fresh StackBounds from the OS due to how fibers work.
+        // See https://bugs.webkit.org/show_bug.cgi?id=102411
+#if OS(WINDOWS)
+        m_stackBounds = StackBounds::currentThreadStackBounds();
+#endif
+        return m_stackBounds;
+    }
+
 #if ENABLE(STACK_STATS)
     StackStats::PerThreadStats& stackStats()
     {
@@ -90,6 +101,7 @@ private:
     AtomicStringTable* m_defaultAtomicStringTable;
     AtomicStringTableDestructor m_atomicStringTableDestructor;
 
+    StackBounds m_stackBounds;
 #if ENABLE(STACK_STATS)
     StackStats::PerThreadStats m_stackStats;
 #endif