Summary: | [chromium] UnlockNonLocked condition reached in WorkerFileSystemsCallbackBridge::mayPostTaskToWorker | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | cbentzel | ||||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, commit-queue, dimich, kinuko, levin | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | Linux | ||||||||||
Attachments: |
|
Description
cbentzel
2011-03-29 13:29:12 PDT
Hmm, not sure how I could unlock an non-locked lock using a scoped locker? I'll look into it... Memory corruption? Created attachment 87468 [details]
Patch
This got introduced recently when a race was being fixed for a ref counted object but in turn the ref counting basically got moved inside of the mutex lock which led to this issue. Comment on attachment 87468 [details]
Patch
This patch is blowing my mind.
I have re-read the comment and it makes sense to me. (At least now I'm sure that the previous code could cause a memory corruption.) Thanks for looking into this one. Comment on attachment 87468 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=87468&action=review > Source/WebKit/chromium/src/WorkerFileSystemCallbacksBridge.cpp:366 > + // Take the lock to ensure that it has been released on the other thread. Does it actually mean "Take the lock to wait until the other thread releases it"? If so, what ensures that the other thread grabbed the lock by that time? I'm not sure I really understand what you are doing here though :-) Comment on attachment 87468 [details]
Patch
Ok, this was just a quick fix and no one understands it.
I'm looking at this class and trying to solve this issue and the one that caused it. I think I can simplify a few things in it.
Created attachment 87636 [details]
Patch
Comment on attachment 87636 [details] Patch This change looks good to me now. I wanted to get rid of m_selfRef but wasn't able to think of a solution. Thanks very much! View in context: https://bugs.webkit.org/attachment.cgi?id=87636&action=review > Source/WebKit/chromium/src/WorkerFileSystemCallbacksBridge.cpp:345 > + if (!bridge->m_worker) // Since this check is just an optimization, it doesn't matter that the usage isn't threadsafe. I think we can just remove these lines? (I'm afraid tsan will warn on this check.) Created attachment 87638 [details]
Patch
Comment on attachment 87638 [details]
Patch
r=me
Comment on attachment 87638 [details] Patch Clearing flags on attachment: 87638 Committed r82540: <http://trac.webkit.org/changeset/82540> All reviewed patches have been landed. Closing bug. |