Bug 65785 - ThreadRestrictionVerifier needs a mode where an object is tied to a particular dispatch queue
Summary: ThreadRestrictionVerifier needs a mode where an object is tied to a particula...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Rowe (bdash)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-05 12:58 PDT by Mark Rowe (bdash)
Modified: 2011-08-07 06:44 PDT (History)
1 user (show)

See Also:


Attachments
Patch (5.83 KB, patch)
2011-08-05 14:21 PDT, Mark Rowe (bdash)
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Rowe (bdash) 2011-08-05 12:58:40 PDT
It’s not an uncommon practice to serialize access to an object by routing all access through a serial dispatch queue.  We have some code built on top of WTF::RefCounted that uses this model and currently fails the ThreadRestrictionVerifier asserts all over the place since it doesn’t know about this model.
Comment 1 Mark Rowe (bdash) 2011-08-05 13:00:34 PDT
I’ve typed up a patch and am waiting on a build to confirm that it makes the asserts go away.  It should be ready for review in an hour or so.
Comment 2 Mark Rowe (bdash) 2011-08-05 14:21:20 PDT
Created attachment 103112 [details]
Patch
Comment 3 Adam Roben (:aroben) 2011-08-05 14:29:23 PDT
Comment on attachment 103112 [details]
Patch

I wonder if we could add a test to TestWebKitAPI for this?
Comment 4 Anders Carlsson 2011-08-05 15:24:44 PDT
Comment on attachment 103112 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=103112&action=review

> Source/JavaScriptCore/wtf/ThreadRestrictionVerifier.h:57
> +        , m_owningQueue(0)

This should be inside an #if HAVE(DISPATCH_H)
Comment 5 Mark Rowe (bdash) 2011-08-05 15:25:36 PDT
(In reply to comment #3)
> (From update of attachment 103112 [details])
> I wonder if we could add a test to TestWebKitAPI for this?

I have no idea.  The original patch also didn’t have tests.
Comment 6 Mark Rowe (bdash) 2011-08-05 15:37:22 PDT
Landed in r92523.
Comment 7 David Levin 2011-08-07 06:44:38 PDT
Thanks for the fix (and for getting someone to review it quickly).

I'm out and without much internet access right now.

I'll add a bug to add tests for the original patch and this one.