IndexedDB: leak WebIDBConnectionToClient for retain cycle
authorsihui_liu@apple.com <sihui_liu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 12 Jan 2019 01:11:58 +0000 (01:11 +0000)
committersihui_liu@apple.com <sihui_liu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 12 Jan 2019 01:11:58 +0000 (01:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=193097
<rdar://problem/46899601>

Reviewed by Brady Eidson.

Source/WebCore:

Let IDBConnectionToClient hold a WeakPtr of IDBConnectionToClientDelegate.

* Modules/indexeddb/server/IDBConnectionToClient.cpp:
(WebCore::IDBServer::IDBConnectionToClient::IDBConnectionToClient):
(WebCore::IDBServer::IDBConnectionToClient::identifier const):
(WebCore::IDBServer::IDBConnectionToClient::didDeleteDatabase):
(WebCore::IDBServer::IDBConnectionToClient::didOpenDatabase):
(WebCore::IDBServer::IDBConnectionToClient::didAbortTransaction):
(WebCore::IDBServer::IDBConnectionToClient::didCreateObjectStore):
(WebCore::IDBServer::IDBConnectionToClient::didDeleteObjectStore):
(WebCore::IDBServer::IDBConnectionToClient::didRenameObjectStore):
(WebCore::IDBServer::IDBConnectionToClient::didClearObjectStore):
(WebCore::IDBServer::IDBConnectionToClient::didCreateIndex):
(WebCore::IDBServer::IDBConnectionToClient::didDeleteIndex):
(WebCore::IDBServer::IDBConnectionToClient::didRenameIndex):
(WebCore::IDBServer::IDBConnectionToClient::didPutOrAdd):
(WebCore::IDBServer::IDBConnectionToClient::didGetRecord):
(WebCore::IDBServer::IDBConnectionToClient::didGetAllRecords):
(WebCore::IDBServer::IDBConnectionToClient::didGetCount):
(WebCore::IDBServer::IDBConnectionToClient::didDeleteRecord):
(WebCore::IDBServer::IDBConnectionToClient::didOpenCursor):
(WebCore::IDBServer::IDBConnectionToClient::didIterateCursor):
(WebCore::IDBServer::IDBConnectionToClient::didCommitTransaction):
(WebCore::IDBServer::IDBConnectionToClient::fireVersionChangeEvent):
(WebCore::IDBServer::IDBConnectionToClient::didStartTransaction):
(WebCore::IDBServer::IDBConnectionToClient::didCloseFromServer):
(WebCore::IDBServer::IDBConnectionToClient::notifyOpenDBRequestBlocked):
(WebCore::IDBServer::IDBConnectionToClient::didGetAllDatabaseNames):
* Modules/indexeddb/server/IDBConnectionToClient.h:
* Modules/indexeddb/server/IDBConnectionToClientDelegate.h:

Source/WebKit:

Let WebIDBConnectionToClient hold reference to IPC::Connection instead of NetworkConnectionToWebProcess to break
the cycle.

* NetworkProcess/IndexedDB/WebIDBConnectionToClient.cpp:
(WebKit::WebIDBConnectionToClient::create):
(WebKit::WebIDBConnectionToClient::WebIDBConnectionToClient):
(WebKit::WebIDBConnectionToClient::messageSenderConnection):
* NetworkProcess/IndexedDB/WebIDBConnectionToClient.h:
* NetworkProcess/NetworkConnectionToWebProcess.cpp:
(WebKit::NetworkConnectionToWebProcess::establishIDBConnectionToServer):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239887 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/server/IDBConnectionToClient.cpp
Source/WebCore/Modules/indexeddb/server/IDBConnectionToClient.h
Source/WebCore/Modules/indexeddb/server/IDBConnectionToClientDelegate.h
Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/IndexedDB/WebIDBConnectionToClient.cpp
Source/WebKit/NetworkProcess/IndexedDB/WebIDBConnectionToClient.h
Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp

index 2a630b5..19c8576 100644 (file)
@@ -1,3 +1,42 @@
+2019-01-11  Sihui Liu  <sihui_liu@apple.com>
+
+        IndexedDB: leak WebIDBConnectionToClient for retain cycle
+        https://bugs.webkit.org/show_bug.cgi?id=193097
+        <rdar://problem/46899601>
+
+        Reviewed by Brady Eidson.
+
+        Let IDBConnectionToClient hold a WeakPtr of IDBConnectionToClientDelegate.
+
+        * Modules/indexeddb/server/IDBConnectionToClient.cpp:
+        (WebCore::IDBServer::IDBConnectionToClient::IDBConnectionToClient):
+        (WebCore::IDBServer::IDBConnectionToClient::identifier const):
+        (WebCore::IDBServer::IDBConnectionToClient::didDeleteDatabase):
+        (WebCore::IDBServer::IDBConnectionToClient::didOpenDatabase):
+        (WebCore::IDBServer::IDBConnectionToClient::didAbortTransaction):
+        (WebCore::IDBServer::IDBConnectionToClient::didCreateObjectStore):
+        (WebCore::IDBServer::IDBConnectionToClient::didDeleteObjectStore):
+        (WebCore::IDBServer::IDBConnectionToClient::didRenameObjectStore):
+        (WebCore::IDBServer::IDBConnectionToClient::didClearObjectStore):
+        (WebCore::IDBServer::IDBConnectionToClient::didCreateIndex):
+        (WebCore::IDBServer::IDBConnectionToClient::didDeleteIndex):
+        (WebCore::IDBServer::IDBConnectionToClient::didRenameIndex):
+        (WebCore::IDBServer::IDBConnectionToClient::didPutOrAdd):
+        (WebCore::IDBServer::IDBConnectionToClient::didGetRecord):
+        (WebCore::IDBServer::IDBConnectionToClient::didGetAllRecords):
+        (WebCore::IDBServer::IDBConnectionToClient::didGetCount):
+        (WebCore::IDBServer::IDBConnectionToClient::didDeleteRecord):
+        (WebCore::IDBServer::IDBConnectionToClient::didOpenCursor):
+        (WebCore::IDBServer::IDBConnectionToClient::didIterateCursor):
+        (WebCore::IDBServer::IDBConnectionToClient::didCommitTransaction):
+        (WebCore::IDBServer::IDBConnectionToClient::fireVersionChangeEvent):
+        (WebCore::IDBServer::IDBConnectionToClient::didStartTransaction):
+        (WebCore::IDBServer::IDBConnectionToClient::didCloseFromServer):
+        (WebCore::IDBServer::IDBConnectionToClient::notifyOpenDBRequestBlocked):
+        (WebCore::IDBServer::IDBConnectionToClient::didGetAllDatabaseNames):
+        * Modules/indexeddb/server/IDBConnectionToClient.h:
+        * Modules/indexeddb/server/IDBConnectionToClientDelegate.h:
+
 2019-01-11  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Fix the build after r239844
index 55bea54..8c8b720 100644 (file)
@@ -39,128 +39,152 @@ Ref<IDBConnectionToClient> IDBConnectionToClient::create(IDBConnectionToClientDe
 }
 
 IDBConnectionToClient::IDBConnectionToClient(IDBConnectionToClientDelegate& delegate)
-    : m_delegate(delegate)
+    : m_delegate(makeWeakPtr(delegate))
 {
 }
 
 uint64_t IDBConnectionToClient::identifier() const
 {
+    ASSERT(m_delegate);
     return m_delegate->identifier();
 }
 
 void IDBConnectionToClient::didDeleteDatabase(const IDBResultData& result)
 {
-    m_delegate->didDeleteDatabase(result);
+    if (m_delegate)
+        m_delegate->didDeleteDatabase(result);
 }
 
 void IDBConnectionToClient::didOpenDatabase(const IDBResultData& result)
 {
-    m_delegate->didOpenDatabase(result);
+    if (m_delegate)
+        m_delegate->didOpenDatabase(result);
 }
 
 void IDBConnectionToClient::didAbortTransaction(const IDBResourceIdentifier& transactionIdentifier, const IDBError& error)
 {
-    m_delegate->didAbortTransaction(transactionIdentifier, error);
+    if (m_delegate)
+        m_delegate->didAbortTransaction(transactionIdentifier, error);
 }
 
 void IDBConnectionToClient::didCreateObjectStore(const IDBResultData& result)
 {
-    m_delegate->didCreateObjectStore(result);
+    if (m_delegate)
+        m_delegate->didCreateObjectStore(result);
 }
 
 void IDBConnectionToClient::didDeleteObjectStore(const IDBResultData& result)
 {
-    m_delegate->didDeleteObjectStore(result);
+    if (m_delegate)
+        m_delegate->didDeleteObjectStore(result);
 }
 
 void IDBConnectionToClient::didRenameObjectStore(const IDBResultData& result)
 {
-    m_delegate->didRenameObjectStore(result);
+    if (m_delegate)
+        m_delegate->didRenameObjectStore(result);
 }
 
 void IDBConnectionToClient::didClearObjectStore(const IDBResultData& result)
 {
-    m_delegate->didClearObjectStore(result);
+    if (m_delegate)
+        m_delegate->didClearObjectStore(result);
 }
 
 void IDBConnectionToClient::didCreateIndex(const IDBResultData& result)
 {
-    m_delegate->didCreateIndex(result);
+    if (m_delegate)
+        m_delegate->didCreateIndex(result);
 }
 
 void IDBConnectionToClient::didDeleteIndex(const IDBResultData& result)
 {
-    m_delegate->didDeleteIndex(result);
+    if (m_delegate)
+        m_delegate->didDeleteIndex(result);
 }
 
 void IDBConnectionToClient::didRenameIndex(const IDBResultData& result)
 {
-    m_delegate->didRenameIndex(result);
+    if (m_delegate)
+        m_delegate->didRenameIndex(result);
 }
 
 void IDBConnectionToClient::didPutOrAdd(const IDBResultData& result)
 {
-    m_delegate->didPutOrAdd(result);
+    if (m_delegate)
+        m_delegate->didPutOrAdd(result);
 }
 
 void IDBConnectionToClient::didGetRecord(const IDBResultData& result)
 {
-    m_delegate->didGetRecord(result);
+    if (m_delegate)
+        m_delegate->didGetRecord(result);
 }
 
 void IDBConnectionToClient::didGetAllRecords(const IDBResultData& result)
 {
-    m_delegate->didGetAllRecords(result);
+    if (m_delegate)
+        m_delegate->didGetAllRecords(result);
 }
 
 void IDBConnectionToClient::didGetCount(const IDBResultData& result)
 {
-    m_delegate->didGetCount(result);
+    if (m_delegate)
+        m_delegate->didGetCount(result);
 }
 
 void IDBConnectionToClient::didDeleteRecord(const IDBResultData& result)
 {
-    m_delegate->didDeleteRecord(result);
+    if (m_delegate)
+        m_delegate->didDeleteRecord(result);
 }
 
 void IDBConnectionToClient::didOpenCursor(const IDBResultData& result)
 {
-    m_delegate->didOpenCursor(result);
+    if (m_delegate)
+        m_delegate->didOpenCursor(result);
 }
 
 void IDBConnectionToClient::didIterateCursor(const IDBResultData& result)
 {
-    m_delegate->didIterateCursor(result);
+    if (m_delegate)
+        m_delegate->didIterateCursor(result);
 }
 
 void IDBConnectionToClient::didCommitTransaction(const IDBResourceIdentifier& transactionIdentifier, const IDBError& error)
 {
-    m_delegate->didCommitTransaction(transactionIdentifier, error);
+    if (m_delegate)
+        m_delegate->didCommitTransaction(transactionIdentifier, error);
 }
 
 void IDBConnectionToClient::fireVersionChangeEvent(UniqueIDBDatabaseConnection& connection, const IDBResourceIdentifier& requestIdentifier, uint64_t requestedVersion)
 {
-    m_delegate->fireVersionChangeEvent(connection, requestIdentifier, requestedVersion);
+    if (m_delegate)
+        m_delegate->fireVersionChangeEvent(connection, requestIdentifier, requestedVersion);
 }
 
 void IDBConnectionToClient::didStartTransaction(const IDBResourceIdentifier& transactionIdentifier, const IDBError& error)
 {
-    m_delegate->didStartTransaction(transactionIdentifier, error);
+    if (m_delegate)
+        m_delegate->didStartTransaction(transactionIdentifier, error);
 }
 
 void IDBConnectionToClient::didCloseFromServer(UniqueIDBDatabaseConnection& connection, const IDBError& error)
 {
-    m_delegate->didCloseFromServer(connection, error);
+    if (m_delegate)
+        m_delegate->didCloseFromServer(connection, error);
 }
 
 void IDBConnectionToClient::notifyOpenDBRequestBlocked(const IDBResourceIdentifier& requestIdentifier, uint64_t oldVersion, uint64_t newVersion)
 {
-    m_delegate->notifyOpenDBRequestBlocked(requestIdentifier, oldVersion, newVersion);
+    if (m_delegate)
+        m_delegate->notifyOpenDBRequestBlocked(requestIdentifier, oldVersion, newVersion);
 }
 
 void IDBConnectionToClient::didGetAllDatabaseNames(uint64_t callbackID, const Vector<String>& databaseNames)
 {
-    m_delegate->didGetAllDatabaseNames(callbackID, databaseNames);
+    if (m_delegate)
+        m_delegate->didGetAllDatabaseNames(callbackID, databaseNames);
 }
 
 void IDBConnectionToClient::registerDatabaseConnection(UniqueIDBDatabaseConnection& connection)
index 642813d..160abfe 100644 (file)
@@ -31,6 +31,7 @@
 #include <wtf/HashSet.h>
 #include <wtf/Ref.h>
 #include <wtf/RefCounted.h>
+#include <wtf/WeakPtr.h>
 
 namespace WebCore {
 
@@ -82,7 +83,7 @@ public:
 private:
     IDBConnectionToClient(IDBConnectionToClientDelegate&);
     
-    Ref<IDBConnectionToClientDelegate> m_delegate;
+    WeakPtr<IDBConnectionToClientDelegate> m_delegate;
     HashSet<UniqueIDBDatabaseConnection*> m_databaseConnections;
 };
 
index 479742a..fdf6640 100644 (file)
@@ -28,6 +28,7 @@
 #if ENABLE(INDEXED_DATABASE)
 
 #include <wtf/Forward.h>
+#include <wtf/WeakPtr.h>
 #include <wtf/text/WTFString.h>
 
 namespace WebCore {
@@ -40,7 +41,7 @@ namespace IDBServer {
 
 class UniqueIDBDatabaseConnection;
 
-class IDBConnectionToClientDelegate {
+class IDBConnectionToClientDelegate : public CanMakeWeakPtr<IDBConnectionToClientDelegate> {
 public:
     virtual ~IDBConnectionToClientDelegate() = default;
     
index 81fa822..671cddf 100644 (file)
@@ -1,5 +1,24 @@
 2019-01-11  Sihui Liu  <sihui_liu@apple.com>
 
+        IndexedDB: leak WebIDBConnectionToClient for retain cycle
+        https://bugs.webkit.org/show_bug.cgi?id=193097
+        <rdar://problem/46899601>
+
+        Reviewed by Brady Eidson.
+
+        Let WebIDBConnectionToClient hold reference to IPC::Connection instead of NetworkConnectionToWebProcess to break
+        the cycle.
+
+        * NetworkProcess/IndexedDB/WebIDBConnectionToClient.cpp:
+        (WebKit::WebIDBConnectionToClient::create):
+        (WebKit::WebIDBConnectionToClient::WebIDBConnectionToClient):
+        (WebKit::WebIDBConnectionToClient::messageSenderConnection):
+        * NetworkProcess/IndexedDB/WebIDBConnectionToClient.h:
+        * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+        (WebKit::NetworkConnectionToWebProcess::establishIDBConnectionToServer):
+
+2019-01-11  Sihui Liu  <sihui_liu@apple.com>
+
         Make "Disable Web SQL" on by default
         https://bugs.webkit.org/show_bug.cgi?id=193354
         <rdar://problem/46524584>
index 7fb2b97..a1fecd2 100644 (file)
 namespace WebKit {
 using namespace WebCore;
 
-Ref<WebIDBConnectionToClient> WebIDBConnectionToClient::create(NetworkProcess& networkProcess, NetworkConnectionToWebProcess& connection, uint64_t serverConnectionIdentifier, PAL::SessionID sessionID)
+Ref<WebIDBConnectionToClient> WebIDBConnectionToClient::create(NetworkProcess& networkProcess, IPC::Connection& connection, uint64_t serverConnectionIdentifier, PAL::SessionID sessionID)
 {
     return adoptRef(*new WebIDBConnectionToClient(networkProcess, connection, serverConnectionIdentifier, sessionID));
 }
 
-WebIDBConnectionToClient::WebIDBConnectionToClient(NetworkProcess& networkProcess, NetworkConnectionToWebProcess& connection, uint64_t serverConnectionIdentifier, PAL::SessionID sessionID)
+WebIDBConnectionToClient::WebIDBConnectionToClient(NetworkProcess& networkProcess, IPC::Connection& connection, uint64_t serverConnectionIdentifier, PAL::SessionID sessionID)
     : m_connection(connection)
     , m_networkProcess(networkProcess)
     , m_identifier(serverConnectionIdentifier)
@@ -71,7 +71,7 @@ void WebIDBConnectionToClient::disconnectedFromWebProcess()
 
 IPC::Connection* WebIDBConnectionToClient::messageSenderConnection()
 {
-    return &m_connection->connection();
+    return m_connection.ptr();
 }
 
 WebCore::IDBServer::IDBConnectionToClient& WebIDBConnectionToClient::connectionToClient()
index 9c74a58..54b1359 100644 (file)
@@ -54,7 +54,7 @@ class NetworkProcess;
 
 class WebIDBConnectionToClient final : public WebCore::IDBServer::IDBConnectionToClientDelegate, public IPC::MessageSender, public RefCounted<WebIDBConnectionToClient> {
 public:
-    static Ref<WebIDBConnectionToClient> create(NetworkProcess&, NetworkConnectionToWebProcess&, uint64_t serverConnectionIdentifier, PAL::SessionID);
+    static Ref<WebIDBConnectionToClient> create(NetworkProcess&, IPC::Connection&, uint64_t serverConnectionIdentifier, PAL::SessionID);
 
     virtual ~WebIDBConnectionToClient();
 
@@ -128,13 +128,13 @@ public:
     void didReceiveMessage(IPC::Connection&, IPC::Decoder&);
 
 private:
-    WebIDBConnectionToClient(NetworkProcess&, NetworkConnectionToWebProcess&, uint64_t serverConnectionIdentifier, PAL::SessionID);
+    WebIDBConnectionToClient(NetworkProcess&, IPC::Connection&, uint64_t serverConnectionIdentifier, PAL::SessionID);
 
     IPC::Connection* messageSenderConnection() final;
 
     template<class MessageType> void handleGetResult(const WebCore::IDBResultData&);
 
-    Ref<NetworkConnectionToWebProcess> m_connection;
+    Ref<IPC::Connection> m_connection;
     Ref<NetworkProcess> m_networkProcess;
 
     uint64_t m_identifier;
index bc19695..73cf708 100644 (file)
@@ -701,7 +701,7 @@ void NetworkConnectionToWebProcess::establishIDBConnectionToServer(PAL::SessionI
     LOG(IndexedDB, "NetworkConnectionToWebProcess::establishIDBConnectionToServer - %" PRIu64, serverConnectionIdentifier);
     ASSERT(!m_webIDBConnections.contains(serverConnectionIdentifier));
     
-    m_webIDBConnections.set(serverConnectionIdentifier, WebIDBConnectionToClient::create(m_networkProcess, *this, serverConnectionIdentifier, sessionID));
+    m_webIDBConnections.set(serverConnectionIdentifier, WebIDBConnectionToClient::create(m_networkProcess, m_connection.get(), serverConnectionIdentifier, sessionID));
 }
 
 void NetworkConnectionToWebProcess::removeIDBConnectionToServer(uint64_t serverConnectionIdentifier)