[webkit-reviews] review denied: [Bug 188029] Speculative fix for StorageProcess crash : [Attachment 345801] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has denied Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 188029: Speculative fix for StorageProcess crash
https://bugs.webkit.org/show_bug.cgi?id=188029

Attachment 345801: Patch

https://bugs.webkit.org/attachment.cgi?id=345801&action=review




--- Comment #2 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 345801
  --> https://bugs.webkit.org/attachment.cgi?id=345801
Patch

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

I am not in favor of making such a change:
1. We do not know for sure this is related to threading.
2. AFAIK, UniqueIDBDatabaseConnection should not be dealt with from non-main
thread, they are not even ThreadSafeRefCounted

If we suspect threading, I think we should look at call sites. Also, adding the
isMainThread() assertions is a good idea (maybe even release assertions?).

> Source/WebCore/Modules/indexeddb/server/IDBConnectionToClient.h:87
> +    Lock m_databaseConnectionsLock;

What about IDBConnectionToClient::connectionToClientClosed() ? You're failing
to lock there so the Lock is not useful.