ASSERTION FAILED: registration || isTerminating() in WebCore::SWServerWorker::skipWai...

https://bugs.webkit.org/show_bug.cgi?id=181603
<rdar://problem/36476050>

Reviewed by Youenn Fablet.

No new tests, covered by existing tests that crash flakily.

* workers/service/server/SWServer.cpp:
(WebCore::SWServer::terminateWorkerInternal):
If the connection to the context process is gone, make sure we make the worker as terminated
so that it does not stay in Running state and in SWServer::m_runningOrTerminatingWorkers.

* workers/service/server/SWServerRegistration.cpp:
(WebCore::SWServerRegistration::~SWServerRegistration):
Add assertions to make sure none of the registration's workers are still running when
the registration is destroyed.

(WebCore::SWServerRegistration::updateRegistrationState):
Make sure registration workers that are overwritten are not still running.

* workers/service/server/SWServerWorker.cpp:
(WebCore::SWServerWorker::setState):
If a worker's state is set to redundant, make sure we also terminate it.

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 6ac1efb..16e23f0 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,30 @@
+2018-01-12  Chris Dumez  <cdumez@apple.com>
+
+        ASSERTION FAILED: registration || isTerminating() in WebCore::SWServerWorker::skipWaiting()
+        https://bugs.webkit.org/show_bug.cgi?id=181603
+        <rdar://problem/36476050>
+
+        Reviewed by Youenn Fablet.
+
+        No new tests, covered by existing tests that crash flakily.
+
+        * workers/service/server/SWServer.cpp:
+        (WebCore::SWServer::terminateWorkerInternal):
+        If the connection to the context process is gone, make sure we make the worker as terminated
+        so that it does not stay in Running state and in SWServer::m_runningOrTerminatingWorkers.
+
+        * workers/service/server/SWServerRegistration.cpp:
+        (WebCore::SWServerRegistration::~SWServerRegistration):
+        Add assertions to make sure none of the registration's workers are still running when
+        the registration is destroyed.
+
+        (WebCore::SWServerRegistration::updateRegistrationState):
+        Make sure registration workers that are overwritten are not still running.
+
+        * workers/service/server/SWServerWorker.cpp:
+        (WebCore::SWServerWorker::setState):
+        If a worker's state is set to redundant, make sure we also terminate it.
+
 2018-01-12  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r226927.

diff --git a/Source/WebCore/workers/service/server/SWServer.cpp b/Source/WebCore/workers/service/server/SWServer.cpp
index a430acf..e9d5788 100644 (file)
--- a/Source/WebCore/workers/service/server/SWServer.cpp
+++ b/Source/WebCore/workers/service/server/SWServer.cpp
@@ -556,17 +556,18 @@ void SWServer::syncTerminateWorker(SWServerWorker& worker)
 
 void SWServer::terminateWorkerInternal(SWServerWorker& worker, TerminationMode mode)
 {
+    ASSERT(m_runningOrTerminatingWorkers.get(worker.identifier()) == &worker);
+    ASSERT(!worker.isTerminating());
+
+    worker.setState(SWServerWorker::State::Terminating);
+
     auto* connection = SWServerToContextConnection::connectionForIdentifier(worker.contextConnectionIdentifier());
     if (!connection) {
         LOG_ERROR("Request to terminate a worker whose context connection does not exist");
+        workerContextTerminated(worker);
         return;
     }
 
-    ASSERT(m_runningOrTerminatingWorkers.get(worker.identifier()) == &worker);
-    ASSERT(!worker.isTerminating());
-
-    worker.setState(SWServerWorker::State::Terminating);
-
     switch (mode) {
     case Asynchronous:
         connection->terminateWorker(worker.identifier());

diff --git a/Source/WebCore/workers/service/server/SWServerRegistration.cpp b/Source/WebCore/workers/service/server/SWServerRegistration.cpp
index 0f3f8b8..579eed6 100644 (file)
--- a/Source/WebCore/workers/service/server/SWServerRegistration.cpp
+++ b/Source/WebCore/workers/service/server/SWServerRegistration.cpp
@@ -54,6 +54,9 @@ SWServerRegistration::SWServerRegistration(SWServer& server, const ServiceWorker
 
 SWServerRegistration::~SWServerRegistration()
 {
+    ASSERT(!m_installingWorker || !m_installingWorker->isRunning());
+    ASSERT(!m_waitingWorker || !m_waitingWorker->isRunning());
+    ASSERT(!m_activeWorker || !m_activeWorker->isRunning());
 }
 
 SWServerWorker* SWServerRegistration::getNewestWorker()
@@ -72,12 +75,15 @@ void SWServerRegistration::updateRegistrationState(ServiceWorkerRegistrationStat
     
     switch (state) {
     case ServiceWorkerRegistrationState::Installing:
+        ASSERT(!m_installingWorker || !m_installingWorker->isRunning() || m_waitingWorker == m_installingWorker);
         m_installingWorker = worker;
         break;
     case ServiceWorkerRegistrationState::Waiting:
+        ASSERT(!m_waitingWorker || !m_waitingWorker->isRunning() || m_activeWorker == m_waitingWorker);
         m_waitingWorker = worker;
         break;
     case ServiceWorkerRegistrationState::Active:
+        ASSERT(!m_activeWorker || !m_activeWorker->isRunning());
         m_activeWorker = worker;
         break;
     };

diff --git a/Source/WebCore/workers/service/server/SWServerWorker.cpp b/Source/WebCore/workers/service/server/SWServerWorker.cpp
index 843ccbe..6ed5b1b 100644 (file)
--- a/Source/WebCore/workers/service/server/SWServerWorker.cpp
+++ b/Source/WebCore/workers/service/server/SWServerWorker.cpp
@@ -56,6 +56,8 @@ SWServerWorker::SWServerWorker(SWServer& server, SWServerRegistration& registrat
 
     auto result = allWorkers().add(identifier, this);
     ASSERT_UNUSED(result, result.isNewEntry);
+
+    ASSERT(m_server.getRegistration(m_registrationKey));
 }
 
 SWServerWorker::~SWServerWorker()
@@ -168,6 +170,9 @@ void SWServerWorker::whenActivated(WTF::Function<void(bool)>&& handler)
 
 void SWServerWorker::setState(ServiceWorkerState state)
 {
+    if (state == ServiceWorkerState::Redundant)
+        terminate();
+
     m_data.state = state;
 
     if (state == ServiceWorkerState::Activated || state == ServiceWorkerState::Redundant)
@@ -181,6 +186,12 @@ void SWServerWorker::callWhenActivatedHandler(bool success)
         handler(success);
 }
 
+void SWServerWorker::setState(State state)
+{
+    ASSERT(state != State::Running || m_server.getRegistration(m_registrationKey));
+    m_state = state;
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(SERVICE_WORKER)

diff --git a/Source/WebCore/workers/service/server/SWServerWorker.h b/Source/WebCore/workers/service/server/SWServerWorker.h
index b429b8e..2dddb7c 100644 (file)
--- a/Source/WebCore/workers/service/server/SWServerWorker.h
+++ b/Source/WebCore/workers/service/server/SWServerWorker.h
@@ -68,7 +68,7 @@ public:
     };
     bool isRunning() const { return m_state == State::Running; }
     bool isTerminating() const { return m_state == State::Terminating; }
-    void setState(State state) { m_state = state; }
+    void setState(State);
 
     SWServer& server() { return m_server; }
     const ServiceWorkerRegistrationKey& registrationKey() const { return m_registrationKey; }