Bug 95786 – ASSERTion failure in DatabaseSync destructor (called on wrong thread)

• Component: WebCore Misc.
• Status: NEW

[Tim Horton]

Sample log: http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK1%20(Tests)/r127174%20(376)/http/tests/workers/terminate-during-sync-operation-crash-log.txt

On the main thread:

0   com.apple.WebCore             	0x00000001037b8812 WebCore::DatabaseSync::~DatabaseSync() + 130 (DatabaseSync.cpp:87)
1   com.apple.WebCore             	0x00000001037b8785 WebCore::DatabaseSync::~DatabaseSync() + 21 (DatabaseSync.cpp:93)
2   com.apple.WebCore             	0x00000001037b8759 WebCore::DatabaseSync::~DatabaseSync() + 25 (DatabaseSync.cpp:86)
3   com.apple.WebCore             	0x00000001037b25a3 WTF::ThreadSafeRefCounted<WebCore::AbstractDatabase>::deref() + 83 (ThreadSafeRefCounted.h:138)
4   com.apple.WebCore             	0x00000001037d2e5b void WTF::derefIfNotNull<WebCore::AbstractDatabase>(WebCore::AbstractDatabase*) + 59 (PassRefPtr.h:54)
5   com.apple.WebCore             	0x00000001037d2e18 WTF::RefPtr<WebCore::AbstractDatabase>::~RefPtr() + 24 (RefPtr.h:56)
6   com.apple.WebCore             	0x00000001037c9065 WTF::RefPtr<WebCore::AbstractDatabase>::~RefPtr() + 21 (RefPtr.h:56)
7   com.apple.WebCore             	0x00000001037dc01f WTF::VectorDestructor<true, WTF::RefPtr<WebCore::AbstractDatabase> >::destruct(WTF::RefPtr<WebCore::AbstractDatabase>*, WTF::RefPtr<WebCore::AbstractDatabase>*) + 47 (Vector.h:57)
8   com.apple.WebCore             	0x00000001037dbfdd WTF::VectorTypeOperations<WTF::RefPtr<WebCore::AbstractDatabase> >::destruct(WTF::RefPtr<WebCore::AbstractDatabase>*, WTF::RefPtr<WebCore::AbstractDatabase>*) + 29 (Vector.h:221)
9   com.apple.WebCore             	0x00000001037dbf21 WTF::Vector<WTF::RefPtr<WebCore::AbstractDatabase>, 0ul>::shrink(unsigned long) + 145 (Vector.h:905)
10  com.apple.WebCore             	0x00000001037dbe74 WTF::Vector<WTF::RefPtr<WebCore::AbstractDatabase>, 0ul>::~Vector() + 52 (Vector.h:511)
11  com.apple.WebCore             	0x00000001037c8565 WTF::Vector<WTF::RefPtr<WebCore::AbstractDatabase>, 0ul>::~Vector() + 21 (Vector.h:511)
12  com.apple.WebCore             	0x00000001037c34a3 WebCore::DatabaseTracker::interruptAllDatabasesForContext(WebCore::ScriptExecutionContext const*) + 723 (DatabaseTracker.cpp:265)
13  com.apple.WebCore             	0x0000000104d0e062 WebCore::WorkerThread::stop() + 146 (WorkerThread.cpp:269)
14  com.apple.WebCore             	0x0000000104d0210f WebCore::WorkerMessagingProxy::terminateWorkerContext() + 95 (WorkerMessagingProxy.cpp:469)
15  com.apple.WebCore             	0x0000000104cf145d WebCore::Worker::terminate() + 29 (Worker.cpp:119)
16  com.apple.WebCore             	0x00000001043f1667 WebCore::jsWorkerPrototypeFunctionTerminate(JSC::ExecState*) + 311 (JSWorker.cpp:223)
...

It looks to me like DatabaseSync::~DatabaseSync is expecting to be called on the worker's thread, but is getting called on the main thread instead. Seems like this is probably a legitimate concern.

I'm going to skip the test on Mac for now.

<rdar://problem/11911986>

[Tim Horton]

I can pretty easily reproduce the failure with --repeat-each=100, but I can't imagine why it's not 100% repro.

[Tim Horton]

Actually, I'm not sure it makes sense to skip the test - is there any guarantee that it's just the one test?

[Alexey Proskuryakov]

CC'ing some folks from DatabaseSync.cpp file blame.

[Michael Nordman]

void DatabaseTracker::interruptAllDatabasesForContext(const ScriptExecutionContext* context)
{
    Vector<RefPtr<AbstractDatabase> > openDatabases;
    // grabbing these refs on the main thread is the problem

This was fixed in the chromium port at some point by not taking those refs on the main thread. Take a look at the diffs between DatabaseTracker::interruptAllDatabasesForContext() in DatabaseTracker.cpp and in DatabaseTrackerChromium.cpp.
http://trac.webkit.org/changeset/97546

[Simon Fraser (smfr)]

Why did identical code exist in both DatabaseTracker.cpp and DatabaseTrackerChromium.cpp, and why wasn't the non-Chromium code fixed in http://trac.webkit.org/changeset/97546?

[Michael Nordman]

(In reply to comment #5)
> Why did identical code exist in both DatabaseTracker.cpp and DatabaseTrackerChromium.cpp, and why wasn't the non-Chromium code fixed in http://trac.webkit.org/changeset/97546?

I remember that change. At the time it was committed, I was very uncertain about whether it would cause more problems than it solved. Also the non-chromium class is considerably larger and more complicated. These things added up to not making the corresponding change to DatabaseTracker.cpp in the same patch.

I probably could have / should have followed up with followed up with a maintainer of a port that uses the non-chromium file after seeing good results on the chromium test bots.

[Simon Fraser (smfr)]

Can you do that? Mac Safari still has bad code.

[Alexey Proskuryakov]

Simon, I think that Akosh is going to look into this.

[Brady Eidson]

(In reply to comment #8)
> Simon, I think that Akosh is going to look into this.

Akash*