Summary: | Data race between ~WorkerFileSystemCallbacksBridge and runTasksOnWorkerThread | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Klein <adamk> | ||||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | kinuko, levin | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Other | ||||||||||
OS: | OS X 10.5 | ||||||||||
Attachments: |
|
Description
Adam Klein
2011-03-10 13:08:21 PST
Created attachment 85382 [details]
Patch
Comment on attachment 85382 [details]
Patch
I don't understand how this changes a thing. The previous passed in variable bridge would have help the ref count.
Also prpBridge looks like hungarian naming which is simply not used in WebKit.
Working with Kinuko offline to get to the root of the issue, you're right that this doesn't solve the problem. As for the naming, though, this style is recommended by http://www.webkit.org/coding/RefPtr.html and used frequently in WebKit (as a quick codesearch shows). (In reply to comment #3) > As for the naming, though, this style is recommended by http://www.webkit.org/coding/RefPtr.html and used frequently in WebKit (as a quick codesearch shows). Good point! Retitling to better capture the actual problem. Here's the TSAN trace: WARNING: Possible data race during read of size 4 at 0xB16D3C4: {{{ T0 (L{}): #0 WebCore::WorkerContext::Observer::~Observer() third_party/WebKit/Source/WebCore/workers/WorkerContext.cpp:393 #1 WebKit::WorkerFileSystemCallbacksBridge::~WorkerFileSystemCallbacksBridge() third_party/WebKit/Source/WebKit/chromium/src/WorkerFileSystemCallbacksBridge.cpp:311 #2 WTF::ThreadSafeShared<WebKit::WorkerFileSystemCallbacksBridge>::deref() third_party/WebKit/Source/JavaScriptCore/wtf/ThreadSafeShared.h:135 #3 WebKit::MainThreadFileSystemCallbacks::~MainThreadFileSystemCallbacks() third_party/WebKit/Source/JavaScriptCore/wtf/PassRefPtr.h:59 #4 WebKit::MainThreadFileSystemCallbacks::didReadMetadata(WebKit::WebFileInfo const&) third_party/WebKit/Source/WebKit/chromium/src/WorkerFileSystemCallbacksBridge.cpp:119 #5 WebFileSystemCallbackDispatcher::DidReadMetadata(base::PlatformFileInfo const&) content/common/file_system/webfilesystem_callback_dispatcher.cc:44 #6 FileSystemDispatcher::OnDidReadMetadata(int, base::PlatformFileInfo const&) content/common/file_system/file_system_dispatcher.cc:242 ... Concurrent write(s) happened at (OR AFTER) these points: T2 (L{}): #0 WebCore::WorkerContext::Observer::stopObserving() third_party/WebKit/Source/WebCore/workers/WorkerContext.cpp:405 #1 WebKit::WorkerFileSystemCallbacksBridge::runTaskOnWorkerThread(WebCore::ScriptExecutionContext*, WTF::PassRefPtr<WebKit::WorkerFileSystemCallbacksBridge>, WTF::PassOwnPtr<WebCore::ScriptExecutionContext::Task>) third_party/WebKit/Source/WebKit/chromium/src/WorkerFileSystemCallbacksBridge.cpp:368 #2 WebCore::CrossThreadTask2<WTF::PassRefPtr<WebKit::WorkerFileSystemCallbacksBridge>, WTF::PassRefPtr<WebKit::WorkerFileSystemCallbacksBridge>, WTF::PassOwnPtr<WebCore::ScriptExecutionContext::Task>, WTF::PassOwnPtr<WebCore::ScriptExecutionContext::Task> >::performTask(WebCore::ScriptExecutionContext*) third_party/WebKit/Source/WebCore/dom/CrossThreadTask.h:112 #3 WebCore::WorkerRunLoop::Task::performTask(WebCore::ScriptExecutionContext*) third_party/WebKit/Source/WebCore/workers/WorkerRunLoop.cpp:199 #4 WebCore::WorkerRunLoop::runInMode(WebCore::WorkerContext*, WebCore::ModePredicate const&) third_party/WebKit/Source/WebCore/workers/WorkerRunLoop.cpp:163 #5 WebCore::WorkerRunLoop::run(WebCore::WorkerContext*) third_party/WebKit/Source/WebCore/workers/WorkerRunLoop.cpp:134 #6 WebCore::WorkerThread::runEventLoop() third_party/WebKit/Source/WebCore/workers/WorkerThread.cpp:163 #7 WebCore::DedicatedWorkerThread::runEventLoop() third_party/WebKit/Source/WebCore/workers/DedicatedWorkerThread.cpp:66 #8 WebCore::WorkerThread::workerThread() third_party/WebKit/Source/WebCore/workers/WorkerThread.cpp:141 #9 WebCore::WorkerThread::workerThreadStart(void*) third_party/WebKit/Source/WebCore/workers/WorkerThread.cpp:118 Location 0xB16D3C4 is 4 bytes inside a block starting at 0xB16D3C0 of size 52 allocated by T2 from heap: #0 malloc /tmp/valgrind-src/tsan/tsan/ts_valgrind_intercepts.c:410 #1 WTF::fastMalloc(unsigned int) third_party/WebKit/Source/JavaScriptCore/wtf/FastMalloc.cpp:250 #2 WTF::ThreadSafeSharedBase::operator new(unsigned int) third_party/WebKit/Source/JavaScriptCore/wtf/ThreadSafeShared.h:70 #3 WebKit::WorkerFileSystemCallbacksBridge::create(WebKit::WebWorkerBase*, WebCore::ScriptExecutionContext*, WebKit::WebFileSystemCallbacks*) third_party/WebKit/Source/WebKit/chromium/src/WorkerFileSystemCallbacksBridge.h:83 #4 WebCore::WorkerAsyncFileSystemChromium::createWorkerFileSystemCallbacksBridge(WTF::PassOwnPtr<WebCore::AsyncFileSystemCallbacks>) third_party/WebKit/Source/WebKit/chromium/src/WorkerAsyncFileSystemChromium.cpp:219 #5 WebCore::WorkerAsyncFileSystemChromium::readMetadata(WTF::String const&, WTF::PassOwnPtr<WebCore::AsyncFileSystemCallbacks>) third_party/WebKit/Source/WebKit/chromium/src/WorkerAsyncFileSystemChromium.cpp:112 #6 WebCore::DOMFileSystemBase::getMetadata(WebCore::EntryBase const*, WTF::PassRefPtr<WebCore::MetadataCallback>, WTF::PassRefPtr<WebCore::ErrorCallback>) third_party/WebKit/Source/WebCore/fileapi/DOMFileSystemBase.cpp:104 #7 WebCore::Entry::getMetadata(WTF::PassRefPtr<WebCore::MetadataCallback>, WTF::PassRefPtr<WebCore::ErrorCallback>) third_party/WebKit/Source/WebCore/fileapi/Entry.cpp:57 #8 WebCore::EntryInternal::getMetadataCallback(v8::Arguments const&) out/Release/obj/gen/webcore/bindings/V8Entry.cpp:107 #9 v8::internal::MaybeObject* v8::internal::HandleApiCallHelper<false>(v8::internal::(anonymous namespace)::BuiltinArguments<(v8::internal::BuiltinExtraArguments)1>) v8/src/builtins.cc:1069 Race verifier data: 0x9AB7AF9,0x9AB763B }}} See http://crbug.com/75589 for details. Created attachment 86901 [details]
Patch
Comment on attachment 86901 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=86901&action=review Just a few things but they may change the code a bit so I'd like to see it one more time. > Source/WebKit/chromium/src/WorkerFileSystemCallbacksBridge.cpp:89 > static MainThreadFileSystemCallbacks* createLeakedPtr(PassRefPtr<WorkerFileSystemCallbacksBridge> bridge, const String& mode) This should no longer be a PassRefPtr (which implies that it keeps the ref count in some way and that is clearly not the case anymore). > Source/WebKit/chromium/src/WorkerFileSystemCallbacksBridge.cpp:385 > { // Let go of the mutex before possibly deleting this due to m_selfRef.clear(). This comment and the brace are now out of date. Created attachment 86987 [details]
Patch
Committed r81999: <http://trac.webkit.org/changeset/81999> |