Bug 31637

Summary: Need to ASSERT(isMainThread()) in ThreadShared methods.
Product: WebKit Reporter: Dmitry Titov <dimich>
Component: WebCore Misc.Assignee: Dmitry Titov <dimich>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, darin, eric, levin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Proposed patch. darin: review+, dimich: commit-queue-

Dmitry Titov
Reported 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.
Attachments
Proposed patch. (2.24 KB, patch)
2009-11-18 16:47 PST, Dmitry Titov 		darin: review+
dimich: commit-queue-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Dmitry Titov
Comment 1 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.
Darin Adler
Comment 2 2009-11-18 16:48:28 PST
Comment on attachment 43471 [details] Proposed patch. r=me
Darin Adler
Comment 3 2009-11-18 16:48:45 PST
So this assertion is not hit at all anywhere in the regression tests?
Dmitry Titov
Comment 4 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.
Dmitry Titov
Comment 5 2009-11-18 17:28:42 PST
Landed: http://trac.webkit.org/changeset/51158
Note You need to log in before you can comment on or make changes to this bug.