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:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Proposed patch.	darin: review+, dimich: commit-queue-

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