Bug 31637 - Need to ASSERT(isMainThread()) in ThreadShared methods.
Summary: Need to ASSERT(isMainThread()) in ThreadShared methods.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Dmitry Titov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-18 12:34 PST by Dmitry Titov
Modified: 2009-11-18 17:28 PST (History)
4 users (show)

See Also:

Attachments
Proposed patch. (2.24 KB, patch)
2009-11-18 16:47 PST, Dmitry Titov 		darin: review+
dimich: commit-queue-
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Titov 2009-11-18 12:34:28 PST 
Recent regression (bug 31615) showed that it's way too easy to pass references to Document and other TreeShared-derived classes using RefPtr/PassRefPtr between main thread and worker threads, causing refcounting of TreeShared to be used unsafe cross-thread.

Need to add asserts in at least ref(0 and deref() methods of TreeShared since they are very easy to misuse. Consider adding asserts to other methods as mentioned in bug 31615.
Comment 1 Dmitry Titov 2009-11-18 16:47:44 PST 
Created attachment 43471 [details]
Proposed patch.

Added ASSERT to constructor, destructor, setParent, parent, ref and deref.

Measured run-webkit-tests on debug build - the difference is not measurable. On my machine, it takes 11 min 20 sec, +/- 10 seconds from run to run, I can not see the difference.
Comment 2 Darin Adler 2009-11-18 16:48:28 PST 
Comment on attachment 43471 [details]
Proposed patch.

r=me
Comment 3 Darin Adler 2009-11-18 16:48:45 PST 
So this assertion is not hit at all anywhere in the regression tests?
Comment 4 Dmitry Titov 2009-11-18 17:22:06 PST 
(In reply to comment #3)
> So this assertion is not hit at all anywhere in the regression tests?

No, not now. It would before rollback of r50919.
Comment 5 Dmitry Titov 2009-11-18 17:28:42 PST 
Landed: http://trac.webkit.org/changeset/51158