Bug 145508 - Purge PassRefPtr in WebCore/Modules - 3
Summary: Purge PassRefPtr in WebCore/Modules - 3
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gyuyoung Kim
URL:
Keywords:
Depends on:
Blocks: 144092 145549
  Show dependency treegraph
 
Reported: 2015-06-01 00:19 PDT by Gyuyoung Kim
Modified: 2015-06-01 23:40 PDT (History)
2 users (show)

See Also:


Attachments
Patch (87.16 KB, patch)
2015-06-01 00:20 PDT, Gyuyoung Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gyuyoung Kim 2015-06-01 00:19:06 PDT
SSIA
Comment 1 Gyuyoung Kim 2015-06-01 00:20:34 PDT
Created attachment 253990 [details]
Patch
Comment 2 WebKit Commit Bot 2015-06-01 18:04:57 PDT
Comment on attachment 253990 [details]
Patch

Clearing flags on attachment: 253990

Committed r185091: <http://trac.webkit.org/changeset/185091>
Comment 3 WebKit Commit Bot 2015-06-01 18:05:02 PDT
All reviewed patches have been landed.  Closing bug.
Comment 4 Joseph Pecoraro 2015-06-01 22:01:48 PDT
There are a few crashing IndexedDB tests on the debug bots:
<https://build.webkit.org/results/Apple%20Yosemite%20Debug%20WK2%20(Tests)/r185101%20(4698)/results.html>

> Regressions: Unexpected crashes (5)
>   storage/indexeddb/error-causes-abort-by-default.html [ Crash ]
>   storage/indexeddb/exception-in-event-aborts.html [ Crash ]
>   storage/indexeddb/mozilla/add-twice-failure.html [ Crash ]
>   storage/indexeddb/request-event-propagation.html [ Crash ]
>   storage/indexeddb/transaction-event-propagation.html [ Crash ]

I am not certain, but could these be a result of this change?
Comment 5 Joseph Pecoraro 2015-06-01 22:03:31 PDT
Comment on attachment 253990 [details]
Patch

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

> Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBServerConnection.cpp:456
> -    serverRequest->completeRequest(resultKey.isNull ? nullptr : resultKey.maybeCreateIDBKey(), errorCode ? IDBDatabaseError::create(errorCode, errorMessage) : nullptr);
> +    serverRequest->completeRequest(resultKey.isNull ? nullptr : resultKey.maybeCreateIDBKey(), errorCode ? IDBDatabaseError::create(errorCode, errorMessage).ptr() : nullptr);

Hmm, seems likely. This did modify the code at the ASSERT crash point:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x000000010eac4d17 WTFCrashWithSecurityImplication + 39
1   com.apple.WebKit              	0x000000010afa4542 WTF::RefCountedBase::derefBase() + 66 (RefCounted.h:94)
2   com.apple.WebKit              	0x000000010b65f39f WTF::RefCounted<WebCore::IDBDatabaseError>::deref() + 31 (RefCounted.h:145)
3   com.apple.WebKit              	0x000000010b65f890 WTF::Ref<WebCore::IDBDatabaseError>::~Ref() + 48 (Ref.h:57)
4   com.apple.WebKit              	0x000000010b65a255 WTF::Ref<WebCore::IDBDatabaseError>::~Ref() + 21 (Ref.h:57)
5   com.apple.WebKit              	0x000000010b663699 WebKit::WebIDBServerConnection::didPutRecord(unsigned long long, WebCore::IDBKeyData const&, unsigned int, WTF::String const&) + 377 (WebIDBServerConnection.cpp:456)
...
Comment 6 Gyuyoung Kim 2015-06-01 22:05:16 PDT
(In reply to comment #5)
> Comment on attachment 253990 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=253990&action=review
> 
> > Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBServerConnection.cpp:456
> > -    serverRequest->completeRequest(resultKey.isNull ? nullptr : resultKey.maybeCreateIDBKey(), errorCode ? IDBDatabaseError::create(errorCode, errorMessage) : nullptr);
> > +    serverRequest->completeRequest(resultKey.isNull ? nullptr : resultKey.maybeCreateIDBKey(), errorCode ? IDBDatabaseError::create(errorCode, errorMessage).ptr() : nullptr);
> 
> Hmm, seems likely. This did modify the code at the ASSERT crash point:
> 
> Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
> 0   com.apple.JavaScriptCore      	0x000000010eac4d17
> WTFCrashWithSecurityImplication + 39
> 1   com.apple.WebKit              	0x000000010afa4542
> WTF::RefCountedBase::derefBase() + 66 (RefCounted.h:94)
> 2   com.apple.WebKit              	0x000000010b65f39f
> WTF::RefCounted<WebCore::IDBDatabaseError>::deref() + 31 (RefCounted.h:145)
> 3   com.apple.WebKit              	0x000000010b65f890
> WTF::Ref<WebCore::IDBDatabaseError>::~Ref() + 48 (Ref.h:57)
> 4   com.apple.WebKit              	0x000000010b65a255
> WTF::Ref<WebCore::IDBDatabaseError>::~Ref() + 21 (Ref.h:57)
> 5   com.apple.WebKit              	0x000000010b663699
> WebKit::WebIDBServerConnection::didPutRecord(unsigned long long,
> WebCore::IDBKeyData const&, unsigned int, WTF::String const&) + 377
> (WebIDBServerConnection.cpp:456)
> ...

Oops, let me check it soon.
Comment 7 Gyuyoung Kim 2015-06-01 23:40:32 PDT
Comment on attachment 253990 [details]
Patch

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

>>> Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBServerConnection.cpp:456
>>> +    serverRequest->completeRequest(resultKey.isNull ? nullptr : resultKey.maybeCreateIDBKey(), errorCode ? IDBDatabaseError::create(errorCode, errorMessage).ptr() : nullptr);
>> 
>> Hmm, seems likely. This did modify the code at the ASSERT crash point:
>> 
>> Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
>> 0   com.apple.JavaScriptCore      	0x000000010eac4d17 WTFCrashWithSecurityImplication + 39
>> 1   com.apple.WebKit              	0x000000010afa4542 WTF::RefCountedBase::derefBase() + 66 (RefCounted.h:94)
>> 2   com.apple.WebKit              	0x000000010b65f39f WTF::RefCounted<WebCore::IDBDatabaseError>::deref() + 31 (RefCounted.h:145)
>> 3   com.apple.WebKit              	0x000000010b65f890 WTF::Ref<WebCore::IDBDatabaseError>::~Ref() + 48 (Ref.h:57)
>> 4   com.apple.WebKit              	0x000000010b65a255 WTF::Ref<WebCore::IDBDatabaseError>::~Ref() + 21 (Ref.h:57)
>> 5   com.apple.WebKit              	0x000000010b663699 WebKit::WebIDBServerConnection::didPutRecord(unsigned long long, WebCore::IDBKeyData const&, unsigned int, WTF::String const&) + 377 (WebIDBServerConnection.cpp:456)
>> ...
> 
> Oops, let me check it soon.

There was my mistake. I should use leakRef() instead of ptr(). Crash happens when doing dereferencing for the instance created by IDBDatabaseError::create() factory function, because .ptr() doesn't reduce reference count. I upload a patch to fix this crash on Bug 145549.