Bug 59801 – fast/workers/storage/use-same-database-in-page-and-workers.html sometimes asserts in WorkerThread::workerThread

• Component: WebCore JavaScript
• Status: NEW

[Adam Roben (:aroben)]

fast/workers/storage/use-same-database-in-page-and-workers.html sometimes asserts in WorkerThread::workerThread. See <http://build.webkit.org/results/Leopard%20Intel%20Debug%20(Tests)/r85325%20(29670)/fast/workers/storage/use-same-database-in-page-and-workers-crash-log.txt> for an example.

[Adam Roben (:aroben)]

<rdar://problem/9358641>

[Michael Nordman]

void* WorkerThread::workerThread()
{
    {
        MutexLocker lock(m_threadCreationMutex);
        m_workerContext = createWorkerContext(m_startupData->m_scriptURL, m_startupData->m_userAgent);

        if (m_runLoop.terminated()) {
            // The worker was terminated before the thread had a chance to run. Since the context didn't exist yet,
            // forbidExecution() couldn't be called from stop().
           m_workerContext->script()->forbidExecution();
        }
    }

    WorkerScriptController* script = m_workerContext->script();
    script->evaluate(ScriptSourceCode(m_startupData->m_sourceCode, m_startupData->m_scriptURL));
    // Free the startup data to cause its member variable deref's happen on the worker's thread (since
    // all ref/derefs of these objects are happening on the thread at this point). Note that
    // WorkerThread::~WorkerThread happens on a different thread where it was created.
    m_startupData.clear();

    runEventLoop();

    ThreadIdentifier threadID = m_threadID;

    ASSERT(m_workerContext->hasOneRef());  ******* TRIPPING ON THIS (?) *******

    // The below assignment will destroy the context, which will in turn notify messaging proxy.
    // We cannot let any objects survive past thread exit, because no other thread will run GC or otherwise destroy them.
    m_workerContext = 0;

    // Clean up WebCore::ThreadGlobalData before WTF::WTFThreadData goes away!
    threadGlobalData().destroy();

    // The thread object may be already destroyed from notification now, don't try to access "this".
    detachThread(threadID);

    return 0;
}

[Adam Roben (:aroben)]

From <http://build.webkit.org/results/Leopard%20Intel%20Debug%20(Tests)/r85325%20(29670)/fast/workers/storage/use-same-database-in-page-and-workers-stderr.txt>:

ASSERTION FAILED: m_workerContext->hasOneRef()
/Volumes/Big/WebKit-BuildSlave/leopard-intel-debug/build/Source/WebCore/workers/WorkerThread.cpp(145) : void* WebCore::WorkerThread::workerThread()
1   WebCore::WorkerThread::workerThreadCount()
2   WebCore::WorkerThread::workerThreadCount()
3   WTF::createThread(void* (*)(void*), void*)
4   _pthread_start
5   thread_start

[Alexey Proskuryakov]

That's a fairly important assertion - if that fails, the worker context may leak (since no GC will happen after the thread has exited).

[Michael Nordman]

This may be contributing to the problem. Databases hold a reference to their ScriptExecutionContext and there is no guarantee that a Database is finally released and deleted on the context thread.

Database::~Database()
{
    // The reference to the ScriptExecutionContext needs to be cleared on the JavaScript thread.
    // If we're on that thread already, we can just let the RefPtr's destruction do the dereffing.
    if (!m_scriptExecutionContext->isContextThread()) {
        // Grab a pointer to the script execution here because we're releasing it when we pass it to
        // DerefContextTask::create.
        ScriptExecutionContext* scriptExecutionContext = m_scriptExecutionContext.get();
        
        scriptExecutionContext->postTask(DerefContextTask::create(m_scriptExecutionContext.release()));
    }
}

A similar situation exists with SQLTransactions and SQLStatements and the callback objects they hold, there is no guarantee that SQLTransactions and SQLStatements are deleted on the context thread, but the callback must be released on the context thread... see class SQLCallbackWrapper.


Maybe something that could help would be to have the safeRelease task created by SQLCallbackWrapper use the isCleanupTask() feature of ScriptExecutionContext::Task as does DerefContextTask?

class SQLCallbackWrapper {
...
    void clear()
    {
        ScriptExecutionContext* context;
        T* callback;
        {
            MutexLocker locker(m_mutex);
            if (!m_callback) {
                ASSERT(!m_scriptExecutionContext);
                return;
            }
            if (m_scriptExecutionContext->isContextThread()) {
                m_callback = 0;
                m_scriptExecutionContext = 0;
                return;
            }
            context = m_scriptExecutionContext.release().leakRef();
            callback = m_callback.release().leakRef();
        }
        context->postTask(createCallbackTask(&safeRelease, callback));   ******* THIS TASK ******
    }


class ScriptExecutionContext {
...
        class Task {
            WTF_MAKE_NONCOPYABLE(Task); WTF_MAKE_FAST_ALLOCATED;
        public:
            Task() { }
            virtual ~Task();
            virtual void performTask(ScriptExecutionContext*) = 0;
            // Certain tasks get marked specially so that they aren't discarded, and are executed,
            // when the context is shutting down its message queue.
            virtual bool isCleanupTask() const { return false; }    ****** USE THIS FEATURE ******
        };

[Ryosuke Niwa]

*** Bug 49869 has been marked as a duplicate of this bug. ***

[Gustavo Noronha (kov)]

*** Bug 56147 has been marked as a duplicate of this bug. ***