Make sure cookies get flushed to disk before exiting or suspending the network process
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Aug 2018 23:45:44 +0000 (23:45 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Aug 2018 23:45:44 +0000 (23:45 +0000)
https://bugs.webkit.org/show_bug.cgi?id=188241
<rdar://problem/42831831>

Reviewed by Alex Christensen and Geoffrey Garen.

Make sure cookies get flushed to disk before exiting or suspending the network process,
to make sure they do not get lost.

* NetworkProcess/NetworkProcess.cpp:
(WebKit::NetworkProcess::didClose):
(WebKit::NetworkProcess::actualPrepareToSuspend):
(WebKit::NetworkProcess::platformSyncAllCookies):
* NetworkProcess/NetworkProcess.h:
* NetworkProcess/cocoa/NetworkProcessCocoa.mm:
(WebKit::NetworkProcess::syncAllCookies):
(WebKit::NetworkProcess::platformSyncAllCookies):
* Shared/ChildProcess.cpp:
(WebKit::ChildProcess::didClose):
(WebKit::callExitNow):
(WebKit::callExitSoon):
(WebKit::ChildProcess::initialize):
(WebKit::didCloseOnConnectionWorkQueue): Deleted.
* Shared/ChildProcess.h:
(WebKit::ChildProcess::shouldCallExitWhenConnectionIsClosed const):

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

Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/NetworkProcess.cpp
Source/WebKit/NetworkProcess/NetworkProcess.h
Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm
Source/WebKit/Shared/ChildProcess.cpp
Source/WebKit/Shared/ChildProcess.h

index 2507ace..4b46a28 100644 (file)
@@ -1,3 +1,31 @@
+2018-08-01  Chris Dumez  <cdumez@apple.com>
+
+        Make sure cookies get flushed to disk before exiting or suspending the network process
+        https://bugs.webkit.org/show_bug.cgi?id=188241
+        <rdar://problem/42831831>
+
+        Reviewed by Alex Christensen and Geoffrey Garen.
+
+        Make sure cookies get flushed to disk before exiting or suspending the network process,
+        to make sure they do not get lost.
+
+        * NetworkProcess/NetworkProcess.cpp:
+        (WebKit::NetworkProcess::didClose):
+        (WebKit::NetworkProcess::actualPrepareToSuspend):
+        (WebKit::NetworkProcess::platformSyncAllCookies):
+        * NetworkProcess/NetworkProcess.h:
+        * NetworkProcess/cocoa/NetworkProcessCocoa.mm:
+        (WebKit::NetworkProcess::syncAllCookies):
+        (WebKit::NetworkProcess::platformSyncAllCookies):
+        * Shared/ChildProcess.cpp:
+        (WebKit::ChildProcess::didClose):
+        (WebKit::callExitNow):
+        (WebKit::callExitSoon):
+        (WebKit::ChildProcess::initialize):
+        (WebKit::didCloseOnConnectionWorkQueue): Deleted.
+        * Shared/ChildProcess.h:
+        (WebKit::ChildProcess::shouldCallExitWhenConnectionIsClosed const):
+
 2018-08-01  Alex Christensen  <achristensen@webkit.org>
 
         Move all calls to ResourceLoader::start to WebKitLegacy
index 7c0cdd4..14b4ef5 100644 (file)
@@ -190,6 +190,16 @@ void NetworkProcess::didReceiveSyncMessage(IPC::Connection& connection, IPC::Dec
     didReceiveSyncNetworkProcessMessage(connection, decoder, replyEncoder);
 }
 
+void NetworkProcess::didClose(IPC::Connection&)
+{
+    ASSERT(RunLoop::isMain());
+
+    // Make sure we flush all cookies to disk before exiting.
+    platformSyncAllCookies([this] {
+        stopRunLoop();
+    });
+}
+
 void NetworkProcess::didCreateDownload()
 {
     disableTermination();
@@ -846,6 +856,8 @@ void NetworkProcess::actualPrepareToSuspend(ShouldAcknowledgeWhenReadyToSuspend
         }));
     }
 
+    platformSyncAllCookies([delayedTaskCounter] { });
+
     for (auto& connection : m_webProcessConnections)
         connection->cleanupForSuspension([delayedTaskCounter] { });
 }
@@ -981,6 +993,11 @@ void NetworkProcess::syncAllCookies()
 {
 }
 
+void NetworkProcess::platformSyncAllCookies(CompletionHandler<void()>&& completionHandler)
+{
+    completionHandler();
+}
+
 #endif
 
 } // namespace WebKit
index c9e9da2..de81924 100644 (file)
@@ -189,10 +189,12 @@ private:
     void initializeSandbox(const ChildProcessInitializationParameters&, SandboxInitializationParameters&) override;
     void initializeConnection(IPC::Connection*) override;
     bool shouldTerminate() override;
+    bool shouldCallExitWhenConnectionIsClosed() const final { return false; } // We override didClose() and want it to be called.
 
     // IPC::Connection::Client
     void didReceiveMessage(IPC::Connection&, IPC::Decoder&) override;
     void didReceiveSyncMessage(IPC::Connection&, IPC::Decoder&, std::unique_ptr<IPC::Encoder>&) override;
+    void didClose(IPC::Connection&) override;
 
     // DownloadManager::Client
     void didCreateDownload() override;
@@ -252,6 +254,8 @@ private:
     static void setSharedHTTPCookieStorage(const Vector<uint8_t>& identifier);
 #endif
 
+    void platformSyncAllCookies(CompletionHandler<void()>&&);
+
     void registerURLSchemeAsSecure(const String&) const;
     void registerURLSchemeAsBypassingContentSecurityPolicy(const String&) const;
     void registerURLSchemeAsLocal(const String&) const;
index 5bc7498..76bae11 100644 (file)
@@ -207,20 +207,24 @@ void NetworkProcess::setStorageAccessAPIEnabled(bool enabled)
 
 void NetworkProcess::syncAllCookies()
 {
+    platformSyncAllCookies([this] {
+        didSyncAllCookies();
+    });
+}
+
+void NetworkProcess::platformSyncAllCookies(CompletionHandler<void()>&& completionHander) {
     ASSERT(hasProcessPrivilege(ProcessPrivilege::CanAccessRawCookies));
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wdeprecated-declarations"
     
 #if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400) || (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 120000)
-    RefPtr<CallbackAggregator> callbackAggregator = CallbackAggregator::create([this] {
-        didSyncAllCookies();
-    });
+    RefPtr<CallbackAggregator> callbackAggregator = CallbackAggregator::create(WTFMove(completionHander));
     WebCore::NetworkStorageSession::forEach([&] (auto& networkStorageSession) {
         [networkStorageSession.nsCookieStorage() _saveCookies:[callbackAggregator] { }];
     });
 #else
     _CFHTTPCookieStorageFlushCookieStores();
-    didSyncAllCookies();
+    completionHander();
 #endif
 
 #pragma clang diagnostic pop
index 4534243..e663962 100644 (file)
@@ -56,15 +56,28 @@ ChildProcess::~ChildProcess()
 
 void ChildProcess::didClose(IPC::Connection&)
 {
-    // We call _exit() in didCloseOnConnectionWorkQueue.
-    ASSERT_NOT_REACHED();
 }
 
-NO_RETURN static void didCloseOnConnectionWorkQueue(IPC::Connection*)
+NO_RETURN static void callExitNow(IPC::Connection*)
 {
     _exit(EXIT_SUCCESS);
 }
 
+static void callExitSoon(IPC::Connection*)
+{
+    // If the connection has been closed and we haven't responded in the main thread for 10 seconds
+    // the process will exit forcibly.
+    auto watchdogDelay = 10_s;
+
+    WorkQueue::create("com.apple.WebKit.ChildProcess.WatchDogQueue")->dispatchAfter(watchdogDelay, [] {
+        // We use _exit here since the watchdog callback is called from another thread and we don't want
+        // global destructors or atexit handlers to be called from this thread while the main thread is busy
+        // doing its thing.
+        RELEASE_LOG_ERROR(IPC, "Exiting process early due to unacknowledged closed-connection");
+        _exit(EXIT_FAILURE);
+    });
+}
+
 void ChildProcess::initialize(const ChildProcessInitializationParameters& parameters)
 {
     RELEASE_ASSERT_WITH_MESSAGE(parameters.processIdentifier, "Unable to initialize child process without a WebCore process identifier");
@@ -86,7 +99,11 @@ void ChildProcess::initialize(const ChildProcessInitializationParameters& parame
     PAL::SessionID::enableGenerationProtection();
 
     m_connection = IPC::Connection::createClientConnection(parameters.connectionIdentifier, *this);
-    m_connection->setDidCloseOnConnectionWorkQueueCallback(didCloseOnConnectionWorkQueue);
+    if (shouldCallExitWhenConnectionIsClosed())
+        m_connection->setDidCloseOnConnectionWorkQueueCallback(callExitNow);
+    else
+        m_connection->setDidCloseOnConnectionWorkQueueCallback(callExitSoon);
+
     initializeConnection(m_connection.get());
     m_connection->open();
 }
index a28772b..95a3104 100644 (file)
@@ -94,6 +94,7 @@ protected:
     virtual bool shouldTerminate() = 0;
     virtual void terminate();
 
+    virtual bool shouldCallExitWhenConnectionIsClosed() const { return true; }
     virtual void stopRunLoop();
 
 #if USE(APPKIT)
@@ -120,7 +121,7 @@ private:
 
     // IPC::Connection::Client.
     void didReceiveInvalidMessage(IPC::Connection&, IPC::StringReference messageReceiverName, IPC::StringReference messageName) final;
-    void didClose(IPC::Connection&) final;
+    void didClose(IPC::Connection&) override;
 
     void shutDown();