Bug 145508 – Purge PassRefPtr in WebCore/Modules - 3

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Gyuyoung Kim]

SSIA

[Gyuyoung Kim]

Created attachment 253990 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 253990 [details]
Patch

Clearing flags on attachment: 253990

Committed r185091: <http://trac.webkit.org/changeset/185091>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Joseph Pecoraro]

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?

[Joseph Pecoraro]

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)
...

[Gyuyoung Kim]

(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.

[Gyuyoung Kim]

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.