Bug 95786 - ASSERTion failure in DatabaseSync destructor (called on wrong thread)
Summary: ASSERTion failure in DatabaseSync destructor (called on wrong thread)
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar, LayoutTestFailure
Depends on:
Blocks:
 
Reported: 2012-09-04 15:07 PDT by Tim Horton
Modified: 2012-09-19 22:10 PDT (History)
9 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2012-09-04 15:07:04 PDT
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>
Comment 1 Tim Horton 2012-09-04 15:09:20 PDT
I can pretty easily reproduce the failure with --repeat-each=100, but I can't imagine why it's not 100% repro.
Comment 2 Tim Horton 2012-09-04 15:11:39 PDT
Actually, I'm not sure it makes sense to skip the test - is there any guarantee that it's just the one test?
Comment 3 Alexey Proskuryakov 2012-09-05 10:00:05 PDT
CC'ing some folks from DatabaseSync.cpp file blame.
Comment 4 Michael Nordman 2012-09-05 11:27:18 PDT
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
Comment 5 Simon Fraser (smfr) 2012-09-18 16:15:43 PDT
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?
Comment 6 Michael Nordman 2012-09-19 17:17:23 PDT
(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.
Comment 7 Simon Fraser (smfr) 2012-09-19 17:24:34 PDT
Can you do that? Mac Safari still has bad code.
Comment 8 Alexey Proskuryakov 2012-09-19 20:18:27 PDT
Simon, I think that Akosh is going to look into this.
Comment 9 Brady Eidson 2012-09-19 22:10:28 PDT
(In reply to comment #8)
> Simon, I think that Akosh is going to look into this.

Akash*