Bug 63127 - DatabaseTask synchronization error
Summary: DatabaseTask synchronization error
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-22 03:55 PDT by Dmitry Vyukov
Modified: 2011-06-23 01:59 PDT (History)
4 users (show)

See Also:


Attachments
Proposed patch (537 bytes, patch)
2011-06-22 03:58 PDT, Dmitry Vyukov
levin: review-
Details | Formatted Diff | Diff
Proposed patch (1.19 KB, patch)
2011-06-23 01:47 PDT, Dmitry Vyukov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Vyukov 2011-06-22 03:55:08 PDT
The problem is with
Source/WebCore/storage/DatabaseTask.cpp
void DatabaseTaskSynchronizer::waitForTaskCompletion()
{
    m_synchronousMutex.lock();
    if (!m_taskCompleted)
        m_synchronousCondition.wait(m_synchronousMutex);
    m_synchronousMutex.unlock();
}

Since POSIX condition variables are amenable to spurious wake-ups, waits must always be expressed as loops. Current implementation leads to premature returns from waitForTaskCompletion() and subsequent bad memory/stack corruptions. I observe crashes like:

Program terminated with signal 11, Segmentation fault.
#0  0x0000000002452d10 in WTF::RefPtr<WebCore::CSSRuleList>::RefPtr<WebCore::CSSRuleList> (this=0x0, o=<value optimized out>) at third_party/WebKit/Source/JavaScriptCore/wtf/RefPtr.h:94
94	        : m_ptr(o.leakRef())
(gdb) bt
#0  0x0000000002452d10 in WTF::RefPtr<WebCore::CSSRuleList>::RefPtr<WebCore::CSSRuleList> (this=0x0, o=<value optimized out>) at third_party/WebKit/Source/JavaScriptCore/wtf/RefPtr.h:94
#1  0x000000000283edd1 in append<WTF::PassRefPtr<WebCore::CSSRuleList> > (this=0x7fff75788480) at third_party/WebKit/Source/JavaScriptCore/wtf/Vector.h:992
#2  WebCore::CSSParser::createRuleList (this=0x7fff75788480) at third_party/WebKit/Source/WebCore/css/CSSParser.cpp:6316
#3  0x0000000003d3aebe in cssyyparse (parser=0x7fff75788480) at /usr/local/google/home/dvyukov/chrome/src/third_party/WebKit/Source/WebCore/css/CSSGrammar.y:430
#4  0x000000000282aa4d in WebCore::CSSParser::parseSheet (this=0x7fff75788480, sheet=0x969efe0, string=..., startLineNumber=0, ruleRangeMap=0x0) at third_party/WebKit/Source/WebCore/css/CSSParser.cpp:266
#5  0x00000000028b4c74 in WebCore::CSSStyleSheet::parseStringAtLine (this=0x969efe0, string=..., strict=true, startLineNumber=0) at third_party/WebKit/Source/WebCore/css/CSSStyleSheet.cpp:204
#6  0x0000000003ec23ff in WebCore::StyleElement::createSheet (this=0x7f765d52f868, e=0x7f765d52f7f0, startLineNumber=0, text=...) at third_party/WebKit/Source/WebCore/dom/StyleElement.cpp:157
#7  0x0000000003ec28d8 in WebCore::StyleElement::process (this=0x7f765d52f868, e=0x7f765d52f7f0) at third_party/WebKit/Source/WebCore/dom/StyleElement.cpp:133
#8  0x0000000003ec2ac8 in WebCore::StyleElement::insertedIntoDocument (this=0x7f765d52f868, document=0x6883310, element=0x7f765d52f7f0) at third_party/WebKit/Source/WebCore/dom/StyleElement.cpp:67
#9  0x0000000003d13fd5 in WebCore::HTMLStyleElement::insertedIntoDocument (this=0x7f765d52f7f0) at third_party/WebKit/Source/WebCore/html/HTMLStyleElement.cpp:75
#10 0x00000000028e8fa0 in WebCore::notifyChildInserted (child=0xb5d52f7f0) at third_party/WebKit/Source/WebCore/dom/ContainerNode.cpp:1066
#11 0x00000000028eba98 in WebCore::ContainerNode::appendChild (this=0x6892900, newChild=<value optimized out>, ec=@0x7fff757889ac, shouldLazyAttach=true) at third_party/WebKit/Source/WebCore/dom/ContainerNode.cpp:645
#12 0x00000000029725de in WebCore::Node::appendChild (this=0x6892900, newChild=..., ec=@0x7fff757889ac, shouldLazyAttach=true) at third_party/WebKit/Source/WebCore/dom/Node.cpp:668
#13 0x00000000027d36a8 in WebCore::V8Node::appendChildCallback (args=...) at third_party/WebKit/Source/WebCore/bindings/v8/custom/V8NodeCustom.cpp:124
#14 0x0000000001bb55c3 in v8::internal::HandleApiCallHelper<false> (args=..., isolate=0x67da280) at v8/src/builtins.cc:1105
#15 0x0000000001bb5745 in v8::internal::Builtin_HandleApiCall (args=..., isolate=0x67da280) at v8/src/builtins.cc:1122

There are a lot of similar crashes in the crash database, but due to the nature of the bug it's difficult to match them.
In order to fix it, replace 'if' with 'while'.

Btw, the bug is found with GCC-ThreadSanitizer which forces such bugs to occur more frequently. Below are usage instructions for GCC-ThreadSanitizer if you are interested:
http://code.google.com/p/data-race-test/wiki/GccInstrumentation
Comment 1 Dmitry Vyukov 2011-06-22 03:58:03 PDT
Created attachment 98151 [details]
Proposed patch
Comment 2 Dmitry Lomov 2011-06-22 12:15:45 PDT
(In reply to comment #1)
> Created an attachment (id=98151) [details]
> Proposed patch

Patch looks good to me.
Comment 3 David Levin 2011-06-22 12:17:44 PDT
Comment on attachment 98151 [details]
Proposed patch

Nice fix.

This needs a changelog before it can go in. Please read the info at http://www.webkit.org/coding/contributing.html#changelogs

Thanks!
Comment 4 Dmitry Vyukov 2011-06-23 01:47:17 PDT
Created attachment 98322 [details]
Proposed patch
Comment 5 Dmitry Vyukov 2011-06-23 01:47:52 PDT
PTAL
Comment 6 David Levin 2011-06-23 01:49:41 PDT
Comment on attachment 98322 [details]
Proposed patch

Actually a statement about how it isn't testable (or how it is covered by existing tests) would have been better for the ChangeLog, but ok.

Thanks!
Comment 7 WebKit Review Bot 2011-06-23 01:59:22 PDT
Comment on attachment 98322 [details]
Proposed patch

Clearing flags on attachment: 98322

Committed r89544: <http://trac.webkit.org/changeset/89544>
Comment 8 WebKit Review Bot 2011-06-23 01:59:27 PDT
All reviewed patches have been landed.  Closing bug.