Diff
Modified: branches/safari-605-branch/Source/WebCore/ChangeLog (227317 => 227318)
--- branches/safari-605-branch/Source/WebCore/ChangeLog 2018-01-22 17:57:50 UTC (rev 227317)
+++ branches/safari-605-branch/Source/WebCore/ChangeLog 2018-01-22 17:57:53 UTC (rev 227318)
@@ -1,5 +1,49 @@
2018-01-22 Jason Marcell <[email protected]>
+ Cherry-pick r227221. rdar://problem/36722533
+
+ 2018-01-19 Chris Dumez <[email protected]>
+
+ ASSERT(registration || isTerminating()) hit in SWServerWorker::skipWaiting()
+ https://bugs.webkit.org/show_bug.cgi?id=181761
+ <rdar://problem/36594564>
+
+ Reviewed by Youenn Fablet.
+
+ There is a short period of time, early in the registration process where a
+ SWServerWorker object exists for a registration but is not in the registration's
+ installing/waiting/active slots yet. As a result, if a registration is cleared
+ during this period (for e.g. due to the user clearing all website data), that
+ SWServerWorker will not be terminated. We then hit assertion later on when this
+ worker is trying to do things (like call skipWaiting).
+
+ To address the issue, we now keep a reference this SWServerWorker on the
+ registration, via a new SWServerRegistration::m_preInstallationWorker data member.
+ When the registration is cleared, we now take care of terminating this worker.
+
+ No new tests, covered by existing tests that crash flakily in debug builds.
+
+ * workers/WorkerThread.cpp:
+ (WebCore::WorkerThread::stop):
+ if the mutex is locked, then the worker thread is still starting. We spin the
+ runloop and try to stop again later. This avoids the deadlock shown in
+ Bug 181763 as the worker thread may need to interact with the main thread
+ during startup.
+
+ * workers/service/server/SWServer.cpp:
+ (WebCore::SWServer::installContextData):
+ * workers/service/server/SWServerJobQueue.cpp:
+ (WebCore::SWServerJobQueue::scriptContextFailedToStart):
+ (WebCore::SWServerJobQueue::install):
+ * workers/service/server/SWServerRegistration.cpp:
+ (WebCore::SWServerRegistration::~SWServerRegistration):
+ (WebCore::SWServerRegistration::setPreInstallationWorker):
+ (WebCore::SWServerRegistration::clear):
+ * workers/service/server/SWServerRegistration.h:
+ (WebCore::SWServerRegistration::preInstallationWorker const):
+
+2018-01-22 Jason Marcell <[email protected]>
+
Cherry-pick r227220. rdar://problem/36722596
2018-01-19 Chris Dumez <[email protected]>
Modified: branches/safari-605-branch/Source/WebCore/workers/WorkerThread.cpp (227317 => 227318)
--- branches/safari-605-branch/Source/WebCore/workers/WorkerThread.cpp 2018-01-22 17:57:50 UTC (rev 227317)
+++ branches/safari-605-branch/Source/WebCore/workers/WorkerThread.cpp 2018-01-22 17:57:53 UTC (rev 227318)
@@ -265,7 +265,15 @@
// Mutex protection is necessary to ensure that m_workerGlobalScope isn't changed by
// WorkerThread::workerThread() while we're accessing it. Note also that stop() can
// be called before m_workerGlobalScope is fully created.
- LockHolder lock(m_threadCreationAndWorkerGlobalScopeMutex);
+ auto locker = Locker<Lock>::tryLock(m_threadCreationAndWorkerGlobalScopeMutex);
+ if (!locker) {
+ // The thread is still starting, spin the runloop and try again to avoid deadlocks if the worker thread
+ // needs to interact with the main thread during startup.
+ callOnMainThread([this, stoppedCallback = WTFMove(stoppedCallback)]() mutable {
+ stop(WTFMove(stoppedCallback));
+ });
+ return;
+ }
ASSERT(!m_stoppedCallback);
m_stoppedCallback = WTFMove(stoppedCallback);
Modified: branches/safari-605-branch/Source/WebCore/workers/service/server/SWServer.cpp (227317 => 227318)
--- branches/safari-605-branch/Source/WebCore/workers/service/server/SWServer.cpp 2018-01-22 17:57:50 UTC (rev 227317)
+++ branches/safari-605-branch/Source/WebCore/workers/service/server/SWServer.cpp 2018-01-22 17:57:53 UTC (rev 227318)
@@ -503,6 +503,7 @@
return;
}
+ registration->setPreInstallationWorker(worker.ptr());
worker->setState(SWServerWorker::State::Running);
auto result = m_runningOrTerminatingWorkers.add(data.serviceWorkerIdentifier, WTFMove(worker));
ASSERT_UNUSED(result, result.isNewEntry);
Modified: branches/safari-605-branch/Source/WebCore/workers/service/server/SWServerJobQueue.cpp (227317 => 227318)
--- branches/safari-605-branch/Source/WebCore/workers/service/server/SWServerJobQueue.cpp 2018-01-22 17:57:50 UTC (rev 227317)
+++ branches/safari-605-branch/Source/WebCore/workers/service/server/SWServerJobQueue.cpp 2018-01-22 17:57:53 UTC (rev 227318)
@@ -109,6 +109,10 @@
auto* registration = m_server.getRegistration(m_registrationKey);
ASSERT(registration);
+ ASSERT(registration->preInstallationWorker());
+ registration->preInstallationWorker()->terminate();
+ registration->setPreInstallationWorker(nullptr);
+
// Invoke Reject Job Promise with job and TypeError.
m_server.rejectJob(firstJob(), { TypeError, message });
@@ -138,6 +142,9 @@
auto* worker = m_server.workerByID(installingWorker);
RELEASE_ASSERT(worker);
+ ASSERT(registration.preInstallationWorker() == worker);
+ registration.setPreInstallationWorker(nullptr);
+
registration.updateRegistrationState(ServiceWorkerRegistrationState::Installing, worker);
registration.updateWorkerState(*worker, ServiceWorkerState::Installing);
Modified: branches/safari-605-branch/Source/WebCore/workers/service/server/SWServerRegistration.cpp (227317 => 227318)
--- branches/safari-605-branch/Source/WebCore/workers/service/server/SWServerRegistration.cpp 2018-01-22 17:57:50 UTC (rev 227317)
+++ branches/safari-605-branch/Source/WebCore/workers/service/server/SWServerRegistration.cpp 2018-01-22 17:57:53 UTC (rev 227318)
@@ -54,6 +54,7 @@
SWServerRegistration::~SWServerRegistration()
{
+ ASSERT(!m_preInstallationWorker || !m_preInstallationWorker->isRunning());
ASSERT(!m_installingWorker || !m_installingWorker->isRunning());
ASSERT(!m_waitingWorker || !m_waitingWorker->isRunning());
ASSERT(!m_activeWorker || !m_activeWorker->isRunning());
@@ -69,6 +70,12 @@
return m_activeWorker.get();
}
+void SWServerRegistration::setPreInstallationWorker(SWServerWorker* worker)
+{
+ ASSERT(!m_preInstallationWorker || !worker);
+ m_preInstallationWorker = worker;
+}
+
void SWServerRegistration::updateRegistrationState(ServiceWorkerRegistrationState state, SWServerWorker* worker)
{
LOG(ServiceWorker, "(%p) Updating registration state to %i with worker %p", this, (int)state, worker);
@@ -231,6 +238,12 @@
// https://w3c.github.io/ServiceWorker/#clear-registration
void SWServerRegistration::clear()
{
+ if (m_preInstallationWorker) {
+ ASSERT(m_preInstallationWorker->state() == ServiceWorkerState::Redundant);
+ m_preInstallationWorker->terminate();
+ m_preInstallationWorker = nullptr;
+ }
+
clearRegistrationWorker(*this, installingWorker(), ServiceWorkerRegistrationState::Installing);
clearRegistrationWorker(*this, waitingWorker(), ServiceWorkerRegistrationState::Waiting);
clearRegistrationWorker(*this, activeWorker(), ServiceWorkerRegistrationState::Active);
Modified: branches/safari-605-branch/Source/WebCore/workers/service/server/SWServerRegistration.h (227317 => 227318)
--- branches/safari-605-branch/Source/WebCore/workers/service/server/SWServerRegistration.h 2018-01-22 17:57:50 UTC (rev 227317)
+++ branches/safari-605-branch/Source/WebCore/workers/service/server/SWServerRegistration.h 2018-01-22 17:57:53 UTC (rev 227318)
@@ -73,6 +73,8 @@
void addClientServiceWorkerRegistration(SWServerConnectionIdentifier);
void removeClientServiceWorkerRegistration(SWServerConnectionIdentifier);
+ void setPreInstallationWorker(SWServerWorker*);
+ SWServerWorker* preInstallationWorker() const { return m_preInstallationWorker.get(); }
SWServerWorker* installingWorker() const { return m_installingWorker.get(); }
SWServerWorker* waitingWorker() const { return m_waitingWorker.get(); }
SWServerWorker* activeWorker() const { return m_activeWorker.get(); }
@@ -105,6 +107,7 @@
URL m_scriptURL;
bool m_uninstalling { false };
+ RefPtr<SWServerWorker> m_preInstallationWorker; // Implementation detail, not part of the specification.
RefPtr<SWServerWorker> m_installingWorker;
RefPtr<SWServerWorker> m_waitingWorker;
RefPtr<SWServerWorker> m_activeWorker;
