RESOLVED FIXED 31637
Need to ASSERT(isMainThread()) in ThreadShared methods.
https://bugs.webkit.org/show_bug.cgi?id=31637
Summary Need to ASSERT(isMainThread()) in ThreadShared methods.
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-
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
Note You need to log in before you can comment on or make changes to this bug.