Skip to content

Commit

Permalink
Bug 1540913 - Part 3: Exit early if worker module evaluation is abort…
Browse files Browse the repository at this point in the history
…ed; r=jonco

This change addresses the second issue around worker shutdown with infinitely running dynamic
imports: As the event loop is prevented from running when the worker is dying, we do not want to
delegate to the event loop in this case. This case always has a failing promise. Since this is a
failure case related to the worker dying, we stop running code, rather
than waiting for a tick (that never comes) from the event loop.

Differential Revision: https://phabricator.services.mozilla.com/D171684
  • Loading branch information
codehag committed Mar 14, 2023
1 parent 3004440 commit 21fb3e9
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 8 deletions.
7 changes: 7 additions & 0 deletions dom/workers/loader/WorkerModuleLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "mozilla/dom/WorkerLoadContext.h"
#include "mozilla/dom/WorkerPrivate.h"
#include "mozilla/dom/workerinternals/ScriptLoader.h"
#include "mozilla/dom/WorkerScope.h"
#include "WorkerModuleLoader.h"

#include "nsISupportsImpl.h"
Expand Down Expand Up @@ -123,4 +124,10 @@ void WorkerModuleLoader::OnModuleLoadComplete(ModuleLoadRequest* aRequest) {
}
}

bool WorkerModuleLoader::IsModuleEvaluationAborted(
ModuleLoadRequest* aRequest) {
WorkerPrivate* workerPrivate = GetCurrentThreadWorkerPrivate();
return workerPrivate->GlobalScope()->IsDying();
}

} // namespace mozilla::dom::workerinternals::loader
2 changes: 2 additions & 0 deletions dom/workers/loader/WorkerModuleLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ class WorkerModuleLoader : public JS::loader::ModuleLoaderBase {
JS::MutableHandle<JSObject*> aModuleScript) override;

void OnModuleLoadComplete(ModuleLoadRequest* aRequest) override;

bool IsModuleEvaluationAborted(ModuleLoadRequest* aRequest) override;
};

} // namespace mozilla::dom::workerinternals::loader
Expand Down
15 changes: 7 additions & 8 deletions js/loader/ModuleLoaderBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1227,16 +1227,11 @@ nsresult ModuleLoaderBase::EvaluateModuleInContext(
// unless the user cancels execution.
MOZ_ASSERT_IF(ok, !JS_IsExceptionPending(aCx));

// For long running scripts, the request may be cancelled abruptly. This
// may also happen if the loader is collected before we get here.
if (request->IsCanceled() || !mLoader) {
return NS_ERROR_ABORT;
}

if (!ok) {
if (!ok || IsModuleEvaluationAborted(request)) {
LOG(("ScriptLoadRequest (%p): evaluation failed", aRequest));
// For a dynamic import, the promise is rejected. Otherwise an error is
// reported by AutoEntryScript.
rv = NS_ERROR_ABORT;
}

// ModuleEvaluate returns a promise unless the user cancels the execution in
Expand All @@ -1248,7 +1243,11 @@ nsresult ModuleLoaderBase::EvaluateModuleInContext(
}

if (request->IsDynamicImport()) {
FinishDynamicImport(aCx, request, NS_OK, evaluationPromise);
if (NS_FAILED(rv)) {
FinishDynamicImportAndReject(request, rv);
} else {
FinishDynamicImport(aCx, request, NS_OK, evaluationPromise);
}
} else {
// If this is not a dynamic import, and if the promise is rejected,
// the value is unwrapped from the promise value.
Expand Down
4 changes: 4 additions & 0 deletions js/loader/ModuleLoaderBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,10 @@ class ModuleLoaderBase : public nsISupports {
// Called when a module script has been loaded, including imports.
virtual void OnModuleLoadComplete(ModuleLoadRequest* aRequest) = 0;

virtual bool IsModuleEvaluationAborted(ModuleLoadRequest* aRequest) {
return false;
}

// Get the error message when resolving failed. The default is to call
// nsContentUtils::FormatLoalizedString. But currently
// nsContentUtils::FormatLoalizedString cannot be called on a worklet thread,
Expand Down

0 comments on commit 21fb3e9

Please sign in to comment.