Diff
Modified: branches/safari-612-branch/Source/_javascript_Core/ChangeLog (287649 => 287650)
--- branches/safari-612-branch/Source/_javascript_Core/ChangeLog 2022-01-05 21:36:32 UTC (rev 287649)
+++ branches/safari-612-branch/Source/_javascript_Core/ChangeLog 2022-01-05 21:44:38 UTC (rev 287650)
@@ -1,3 +1,101 @@
+2022-01-05 Mark Lam <mark....@apple.com>
+
+ Cherry-pick r287421. rdar://problem/84260429
+
+ 2021-12-23 Mark Lam <mark....@apple.com>
+
+ Make DeferredWorkTimer::addPendingWork() return a Ticket.
+ https://bugs.webkit.org/show_bug.cgi?id=234628
+ rdar://84260429
+
+ Reviewed by Yusuke Suzuki.
+
+ 1. Make Ticket a unique token instead of the JSObject* target object.
+ The Ticket is now a pointer to the TicketData in the pending work list.
+
+ 2. Instead of taking a Ticket argument, DeferredWorkTimer::addPendingWork() now
+ takes a JSObject* `target` argument explicitly, and returns the Ticket for the
+ added TicketData instead.
+
+ All the relevant DeferredWorkTimer APIS already take a Ticket as an argument.
+ This ensures that addPendingWork() is called before we start doing work with
+ these APIs (especially scheduleWorkSoon()).
+
+ 3. Previously, addPendingWork() will only save one instance of TicketData for
+ a given JSObject* key. With this patch, we'll register a new TicketData
+ instance for every call to addPendingWork(), and return a unique Ticket for it.
+
+ This is needed because it may be possible for 2 different clients to call
+ addPendingWork() and scheduleWorkSoon() with the same target JSObject* but with
+ different sets of dependencies.
+
+ Secondly, even is the both sets of dependencies are identical, a client may
+ call addPendingWork() and scheduleWorkSoon() with the same JSObject* target
+ more than once because it intended to schedule more than 1 task to run.
+
+ Note that DeferredWorkTimer::doWork() consumes the corresponding TicketData
+ (i.e. removes it from the m_pendingTickets list) for each task as it is run.
+ To ensure that the dependencies for each task is protected, we'll either need
+ to ref count the TicketData for the same target object (and hold off on removing
+ it from the list), or we'll need to register a different TicketData instance
+ for each task. Ref counting can solve the second issue above, but does not
+ solve the first. So, this patch goes with the more generic solution to allow
+ each task to have its own TicketData instance (and, its own unique Ticket).
+
+ 4. Previously, if the client cancels pending work, we would remove the TicketData
+ immediately from the m_pendingTickets list. This opens up an opportunity for
+ the same TicketData memory to be re-allocated by another client. This, in turn,
+ would make the Ticket token not unique and potentially allow a cancelled ticket
+ to be reused before DeferredWorkTimer::doWork() is called.
+
+ This patch changes DeferredWorkTimer::cancelPendingWork() to only clear the
+ contents of the TicketData instead. TicketData::scriptExecutionOwner being
+ null is used as an indication that the ticket has been cancelled. Since the
+ TicketData itself is not "freed" yet, all TicketData will remain unique until
+ DeferredWorkTimer::doWork().
+
+ Consequently, DeferredWorkTimer::doWork() will now check for cancelled tickets
+ and remove them from the m_pendingTickets list.
+
+ 5. JSFinalizationRegistry was previously calling DeferredWorkTimer::hasPendingWork()
+ to check if it has already scheduled a task, so as not to reschedule again until
+ after the previously scheduled task has been run. This does not play nice
+ with the new Ticket API, because this hasPendingWork() check needs to be done
+ before calling addPendingWork(), and hence, the Ticket is not available yet.
+
+ Fortunately, JSFinalizationRegistry should know if it has already scheduled
+ a task itself. This patch adds a m_hasAlreadyScheduledWork flag to
+ JSFinalizationRegistry that can be used for this check instead.
+
+ * jsc.cpp:
+ (JSC_DEFINE_HOST_FUNCTION):
+ * runtime/DeferredWorkTimer.cpp:
+ (JSC::DeferredWorkTimer::TicketData::TicketData):
+ (JSC::DeferredWorkTimer::TicketData::vm):
+ (JSC::DeferredWorkTimer::TicketData::cancel):
+ (JSC::DeferredWorkTimer::doWork):
+ (JSC::DeferredWorkTimer::addPendingWork):
+ (JSC::DeferredWorkTimer::hasPendingWork):
+ (JSC::DeferredWorkTimer::hasDependancyInPendingWork):
+ (JSC::DeferredWorkTimer::cancelPendingWork):
+ * runtime/DeferredWorkTimer.h:
+ (JSC::DeferredWorkTimer::TicketData::target):
+ * runtime/JSFinalizationRegistry.cpp:
+ (JSC::JSFinalizationRegistry::finalizeUnconditionally):
+ * runtime/JSFinalizationRegistry.h:
+ * wasm/WasmStreamingCompiler.cpp:
+ (JSC::Wasm::StreamingCompiler::StreamingCompiler):
+ (JSC::Wasm::StreamingCompiler::~StreamingCompiler):
+ (JSC::Wasm::StreamingCompiler::didComplete):
+ (JSC::Wasm::StreamingCompiler::fail):
+ (JSC::Wasm::StreamingCompiler::cancel):
+ * wasm/WasmStreamingCompiler.h:
+ * wasm/js/JSWebAssembly.cpp:
+ (JSC::JSWebAssembly::webAssemblyModuleValidateAsync):
+ (JSC::instantiate):
+ (JSC::compileAndInstantiate):
+ (JSC::JSWebAssembly::webAssemblyModuleInstantinateAsync):
+
2022-01-05 Russell Epstein <repst...@apple.com>
Cherry-pick r286283. rdar://problem/87125362
Modified: branches/safari-612-branch/Source/_javascript_Core/jsc.cpp (287649 => 287650)
--- branches/safari-612-branch/Source/_javascript_Core/jsc.cpp 2022-01-05 21:36:32 UTC (rev 287649)
+++ branches/safari-612-branch/Source/_javascript_Core/jsc.cpp 2022-01-05 21:44:38 UTC (rev 287650)
@@ -2377,8 +2377,8 @@
return throwVMTypeError(globalObject, scope, "First argument is not a JS function"_s);
// FIXME: We don't look at the timeout parameter because we don't have a schedule work later API.
- vm.deferredWorkTimer->addPendingWork(vm, callback, { });
- vm.deferredWorkTimer->scheduleWorkSoon(callback, [callback](DeferredWorkTimer::Ticket, DeferredWorkTimer::TicketData&&) {
+ auto ticket = vm.deferredWorkTimer->addPendingWork(vm, callback, { });
+ vm.deferredWorkTimer->scheduleWorkSoon(ticket, [callback](DeferredWorkTimer::Ticket) {
JSGlobalObject* globalObject = callback->globalObject();
MarkedArgumentBuffer args;
call(globalObject, callback, jsUndefined(), args, "You shouldn't see this...");
Modified: branches/safari-612-branch/Source/_javascript_Core/runtime/DeferredWorkTimer.cpp (287649 => 287650)
--- branches/safari-612-branch/Source/_javascript_Core/runtime/DeferredWorkTimer.cpp 2022-01-05 21:36:32 UTC (rev 287649)
+++ branches/safari-612-branch/Source/_javascript_Core/runtime/DeferredWorkTimer.cpp 2022-01-05 21:44:38 UTC (rev 287650)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2017-2020 Apple Inc. All rights reserved.
+ * Copyright (C) 2017-2021 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -37,6 +37,25 @@
static const bool verbose = false;
}
+inline DeferredWorkTimer::TicketData::TicketData(VM& vm, JSObject* scriptExecutionOwner, Vector<Strong<JSCell>>&& dependencies)
+ : dependencies(WTFMove(dependencies))
+ , scriptExecutionOwner(vm, scriptExecutionOwner)
+{
+}
+
+inline VM& DeferredWorkTimer::TicketData::vm()
+{
+ ASSERT(!isCancelled());
+ return target()->vm();
+}
+
+inline void DeferredWorkTimer::TicketData::cancel()
+{
+ scriptExecutionOwner.clear();
+ dependencies.clear();
+}
+
+
DeferredWorkTimer::DeferredWorkTimer(VM& vm)
: Base(vm)
{
@@ -54,7 +73,6 @@
while (!m_tasks.isEmpty()) {
auto [ticket, task] = m_tasks.takeFirst();
- auto globalObject = ticket->structure(vm)->globalObject();
dataLogLnIf(DeferredWorkTimerInternal::verbose, "Doing work on: ", RawPointer(ticket));
auto pendingTicket = m_pendingTickets.find(ticket);
@@ -61,8 +79,17 @@
// We may have already canceled this task or its owner may have been canceled.
if (pendingTicket == m_pendingTickets.end())
continue;
+ ASSERT(ticket == pendingTicket->get());
- switch (globalObject->globalObjectMethodTable()->scriptExecutionStatus(globalObject, pendingTicket->value.scriptExecutionOwner.get())) {
+ if (ticket->isCancelled()) {
+ m_pendingTickets.remove(pendingTicket);
+ continue;
+ }
+
+ // We shouldn't access the TicketData to get this globalObject until
+ // after we confirm that the ticket is still valid (which we did above).
+ auto globalObject = ticket->target()->structure(vm)->globalObject();
+ switch (globalObject->globalObjectMethodTable()->scriptExecutionStatus(globalObject, ticket->scriptExecutionOwner.get())) {
case ScriptExecutionStatus::Suspended:
suspendedTasks.append(std::make_tuple(ticket, WTFMove(task)));
continue;
@@ -75,8 +102,7 @@
// Remove ticket from m_pendingTickets since we are going to run it.
// But we want to keep ticketData while running task since it ensures dependencies are strongly held.
- auto ticketData = WTFMove(pendingTicket->value);
- m_pendingTickets.remove(pendingTicket);
+ std::unique_ptr<TicketData> ticketData = m_pendingTickets.take(pendingTicket);
// Allow tasks we are about to run to schedule work.
m_currentlyRunningTask = true;
@@ -87,9 +113,9 @@
vm.finalizeSynchronousJSExecution();
auto scope = DECLARE_CATCH_SCOPE(vm);
- task(ticket, WTFMove(ticketData));
+ task(ticket);
+ ticketData = nullptr;
if (Exception* exception = scope.exception()) {
- auto* globalObject = ticket->globalObject();
scope.clearException();
globalObject->globalObjectMethodTable()->reportUncaughtExceptionAtEventLoop(globalObject, exception);
}
@@ -103,6 +129,13 @@
while (!suspendedTasks.isEmpty())
m_tasks.prepend(suspendedTasks.takeLast());
+ // It is theoretically possible that a client may cancel a pending ticket and
+ // never call scheduleWorkSoon() on it. As such, it would not be found when
+ // we iterated m_tasks above. We'll need to make sure to purge them here.
+ m_pendingTickets.removeIf([] (auto& ticket) {
+ return ticket->isCancelled();
+ });
+
if (m_pendingTickets.isEmpty() && m_shouldStopRunLoopWhenAllTicketsFinish) {
ASSERT(m_tasks.isEmpty());
RunLoop::current().stop();
@@ -118,38 +151,42 @@
RunLoop::run();
}
-void DeferredWorkTimer::addPendingWork(VM& vm, Ticket ticket, Vector<Strong<JSCell>>&& dependencies)
+DeferredWorkTimer::Ticket DeferredWorkTimer::addPendingWork(VM& vm, JSObject* target, Vector<Strong<JSCell>>&& dependencies)
{
ASSERT(vm.currentThreadIsHoldingAPILock() || (Thread::mayBeGCThread() && vm.heap.worldIsStopped()));
for (unsigned i = 0; i < dependencies.size(); ++i)
- ASSERT(dependencies[i].get() != ticket);
+ ASSERT(dependencies[i].get() != target);
- auto globalObject = ticket->globalObject();
- auto result = m_pendingTickets.ensure(ticket, [&] {
- dataLogLnIf(DeferredWorkTimerInternal::verbose, "Adding new pending ticket: ", RawPointer(ticket));
- JSObject* scriptExecutionOwner = globalObject->globalObjectMethodTable()->currentScriptExecutionOwner(globalObject);
- dependencies.append(Strong<JSCell>(vm, ticket));
- return TicketData { WTFMove(dependencies), Strong<JSObject>(vm, scriptExecutionOwner) };
- });
- if (!result.isNewEntry) {
- dataLogLnIf(DeferredWorkTimerInternal::verbose, "Adding new dependencies for ticket: ", RawPointer(ticket));
- result.iterator->value.dependencies.appendVector(WTFMove(dependencies));
- }
+ auto* globalObject = target->globalObject();
+ JSObject* scriptExecutionOwner = globalObject->globalObjectMethodTable()->currentScriptExecutionOwner(globalObject);
+ dependencies.append(Strong<JSCell>(vm, target));
+
+ auto ticketData = makeUnique<TicketData>(vm, scriptExecutionOwner, WTFMove(dependencies));
+ Ticket ticket = ticketData.get();
+
+ dataLogLnIf(DeferredWorkTimerInternal::verbose, "Adding new pending ticket: ", RawPointer(ticket));
+ auto result = m_pendingTickets.add(WTFMove(ticketData));
+ RELEASE_ASSERT(result.isNewEntry);
+
+ return ticket;
}
bool DeferredWorkTimer::hasPendingWork(Ticket ticket)
{
+ auto result = m_pendingTickets.find(ticket);
+ if (result == m_pendingTickets.end() || ticket->isCancelled())
+ return false;
ASSERT(ticket->vm().currentThreadIsHoldingAPILock() || (Thread::mayBeGCThread() && ticket->vm().heap.worldIsStopped()));
- return m_pendingTickets.contains(ticket);
+ return true;
}
bool DeferredWorkTimer::hasDependancyInPendingWork(Ticket ticket, JSCell* dependency)
{
+ auto result = m_pendingTickets.find(ticket);
+ if (result == m_pendingTickets.end() || ticket->isCancelled())
+ return false;
ASSERT(ticket->vm().currentThreadIsHoldingAPILock() || (Thread::mayBeGCThread() && ticket->vm().heap.worldIsStopped()));
- ASSERT(m_pendingTickets.contains(ticket));
-
- auto result = m_pendingTickets.get(ticket);
- return result.dependencies.contains(dependency);
+ return (*result)->dependencies.contains(dependency);
}
void DeferredWorkTimer::scheduleWorkSoon(Ticket ticket, Task&& task)
@@ -162,11 +199,15 @@
bool DeferredWorkTimer::cancelPendingWork(Ticket ticket)
{
- ASSERT(ticket->vm().currentThreadIsHoldingAPILock() || (Thread::mayBeGCThread() && ticket->vm().heap.worldIsStopped()));
- bool result = m_pendingTickets.remove(ticket);
+ ASSERT(m_pendingTickets.contains(ticket));
+ ASSERT(ticket->isCancelled() || ticket->vm().currentThreadIsHoldingAPILock() || (Thread::mayBeGCThread() && ticket->vm().heap.worldIsStopped()));
- if (result)
+ bool result = false;
+ if (!ticket->isCancelled()) {
dataLogLnIf(DeferredWorkTimerInternal::verbose, "Canceling ticket: ", RawPointer(ticket));
+ ticket->cancel();
+ result = true;
+ }
return result;
}
Modified: branches/safari-612-branch/Source/_javascript_Core/runtime/DeferredWorkTimer.h (287649 => 287650)
--- branches/safari-612-branch/Source/_javascript_Core/runtime/DeferredWorkTimer.h 2022-01-05 21:36:32 UTC (rev 287649)
+++ branches/safari-612-branch/Source/_javascript_Core/runtime/DeferredWorkTimer.h 2022-01-05 21:44:38 UTC (rev 287650)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2017-2020 Apple Inc. All rights reserved.
+ * Copyright (C) 2017-2021 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -29,7 +29,7 @@
#include "Strong.h"
#include <wtf/Deque.h>
-#include <wtf/HashMap.h>
+#include <wtf/HashSet.h>
#include <wtf/Lock.h>
#include <wtf/Vector.h>
@@ -44,14 +44,26 @@
using Base = JSRunLoopTimer;
struct TicketData {
+ private:
+ WTF_MAKE_FAST_ALLOCATED;
+ public:
+ TicketData(VM&, JSObject* scriptExecutionOwner, Vector<Strong<JSCell>>&& dependencies);
+
+ VM& vm();
+ JSObject* target();
+
+ void cancel();
+ bool isCancelled() const { return !scriptExecutionOwner.get(); }
+
Vector<Strong<JSCell>> dependencies;
Strong<JSObject> scriptExecutionOwner;
};
+ using Ticket = TicketData*;
+
void doWork(VM&) final;
- using Ticket = JSObject*;
- void addPendingWork(VM&, Ticket, Vector<Strong<JSCell>>&& dependencies);
+ Ticket addPendingWork(VM&, JSObject* target, Vector<Strong<JSCell>>&& dependencies);
bool hasPendingWork(Ticket);
bool hasDependancyInPendingWork(Ticket, JSCell* dependency);
bool cancelPendingWork(Ticket);
@@ -61,7 +73,7 @@
// to make sure your memory ownership model won't leak memory when
// this occurs. The easiest way is to make sure everything is either owned
// by a GC'd value in dependencies or by the Task lambda.
- using Task = Function<void(Ticket, TicketData&&)>;
+ using Task = Function<void(Ticket)>;
void scheduleWorkSoon(Ticket, Task&&);
void didResumeScriptExecutionOwner();
@@ -77,7 +89,13 @@
bool m_shouldStopRunLoopWhenAllTicketsFinish { false };
bool m_currentlyRunningTask { false };
Deque<std::tuple<Ticket, Task>> m_tasks WTF_GUARDED_BY_LOCK(m_taskLock);
- HashMap<Ticket, TicketData> m_pendingTickets;
+ HashSet<std::unique_ptr<TicketData>> m_pendingTickets;
};
+inline JSObject* DeferredWorkTimer::TicketData::target()
+{
+ ASSERT(!isCancelled());
+ return jsCast<JSObject*>(dependencies.last().get());
+}
+
} // namespace JSC
Modified: branches/safari-612-branch/Source/_javascript_Core/runtime/JSFinalizationRegistry.cpp (287649 => 287650)
--- branches/safari-612-branch/Source/_javascript_Core/runtime/JSFinalizationRegistry.cpp 2022-01-05 21:36:32 UTC (rev 287649)
+++ branches/safari-612-branch/Source/_javascript_Core/runtime/JSFinalizationRegistry.cpp 2022-01-05 21:44:38 UTC (rev 287650)
@@ -149,13 +149,15 @@
return !bucket.value.size();
});
- if (!vm.deferredWorkTimer->hasPendingWork(this) && (readiedCell || deadCount(locker))) {
- vm.deferredWorkTimer->addPendingWork(vm, this, { });
- ASSERT(vm.deferredWorkTimer->hasPendingWork(this));
- vm.deferredWorkTimer->scheduleWorkSoon(this, [this](DeferredWorkTimer::Ticket, DeferredWorkTimer::TicketData&&) {
+ if (!m_hasAlreadyScheduledWork && (readiedCell || deadCount(locker))) {
+ auto ticket = vm.deferredWorkTimer->addPendingWork(vm, this, { });
+ ASSERT(vm.deferredWorkTimer->hasPendingWork(ticket));
+ vm.deferredWorkTimer->scheduleWorkSoon(ticket, [this](DeferredWorkTimer::Ticket) {
JSGlobalObject* globalObject = this->globalObject();
+ this->m_hasAlreadyScheduledWork = false;
this->runFinalizationCleanup(globalObject);
});
+ m_hasAlreadyScheduledWork = true;
}
}
Modified: branches/safari-612-branch/Source/_javascript_Core/runtime/JSFinalizationRegistry.h (287649 => 287650)
--- branches/safari-612-branch/Source/_javascript_Core/runtime/JSFinalizationRegistry.h 2022-01-05 21:36:32 UTC (rev 287649)
+++ branches/safari-612-branch/Source/_javascript_Core/runtime/JSFinalizationRegistry.h 2022-01-05 21:44:38 UTC (rev 287650)
@@ -106,6 +106,7 @@
// We use a separate list for no unregister values instead of a special key in the tables above because the HashMap has a tendency to reallocate under us when iterating...
LiveRegistrations m_noUnregistrationLive;
DeadRegistrations m_noUnregistrationDead;
+ bool m_hasAlreadyScheduledWork { false };
};
} // namespace JSC
Modified: branches/safari-612-branch/Source/_javascript_Core/wasm/WasmStreamingCompiler.cpp (287649 => 287650)
--- branches/safari-612-branch/Source/_javascript_Core/wasm/WasmStreamingCompiler.cpp 2022-01-05 21:36:32 UTC (rev 287649)
+++ branches/safari-612-branch/Source/_javascript_Core/wasm/WasmStreamingCompiler.cpp 2022-01-05 21:44:38 UTC (rev 287650)
@@ -26,7 +26,6 @@
#include "config.h"
#include "WasmStreamingCompiler.h"
-#include "DeferredWorkTimer.h"
#include "JSWebAssembly.h"
#include "JSWebAssemblyCompileError.h"
#include "JSWebAssemblyHelpers.h"
@@ -44,7 +43,6 @@
StreamingCompiler::StreamingCompiler(VM& vm, CompilerMode compilerMode, JSGlobalObject* globalObject, JSPromise* promise, JSObject* importObject)
: m_vm(vm)
, m_compilerMode(compilerMode)
- , m_promise(promise)
, m_info(Wasm::ModuleInformation::create())
, m_parser(m_info.get(), *this)
{
@@ -52,17 +50,17 @@
dependencies.append(Strong<JSCell>(vm, globalObject));
if (importObject)
dependencies.append(Strong<JSCell>(vm, importObject));
- vm.deferredWorkTimer->addPendingWork(vm, promise, WTFMove(dependencies));
- ASSERT(vm.deferredWorkTimer->hasPendingWork(promise));
- ASSERT(vm.deferredWorkTimer->hasDependancyInPendingWork(promise, globalObject));
- ASSERT(!importObject || vm.deferredWorkTimer->hasDependancyInPendingWork(promise, importObject));
+ m_ticket = vm.deferredWorkTimer->addPendingWork(vm, promise, WTFMove(dependencies));
+ ASSERT(vm.deferredWorkTimer->hasPendingWork(m_ticket));
+ ASSERT(vm.deferredWorkTimer->hasDependancyInPendingWork(m_ticket, globalObject));
+ ASSERT(!importObject || vm.deferredWorkTimer->hasDependancyInPendingWork(m_ticket, importObject));
}
StreamingCompiler::~StreamingCompiler()
{
- if (m_promise) {
- auto* promise = std::exchange(m_promise, nullptr);
- m_vm.deferredWorkTimer->scheduleWorkSoon(promise, [](DeferredWorkTimer::Ticket, DeferredWorkTimer::TicketData&&) mutable { });
+ if (m_ticket) {
+ auto ticket = std::exchange(m_ticket, nullptr);
+ m_vm.deferredWorkTimer->scheduleWorkSoon(ticket, [](DeferredWorkTimer::Ticket) mutable { });
}
}
@@ -137,12 +135,12 @@
};
auto result = makeValidationResult(*m_plan);
- auto* promise = std::exchange(m_promise, nullptr);
+ auto ticket = std::exchange(m_ticket, nullptr);
switch (m_compilerMode) {
case CompilerMode::Validation: {
- m_vm.deferredWorkTimer->scheduleWorkSoon(promise, [result = WTFMove(result)](DeferredWorkTimer::Ticket ticket, DeferredWorkTimer::TicketData&& ticketData) mutable {
- JSPromise* promise = jsCast<JSPromise*>(ticket);
- JSGlobalObject* globalObject = jsCast<JSGlobalObject*>(ticketData.dependencies[0].get());
+ m_vm.deferredWorkTimer->scheduleWorkSoon(ticket, [result = WTFMove(result)](DeferredWorkTimer::Ticket ticket) mutable {
+ JSPromise* promise = jsCast<JSPromise*>(ticket->target());
+ JSGlobalObject* globalObject = jsCast<JSGlobalObject*>(ticket->dependencies[0].get());
VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
@@ -159,10 +157,10 @@
}
case CompilerMode::FullCompile: {
- m_vm.deferredWorkTimer->scheduleWorkSoon(promise, [result = WTFMove(result)](DeferredWorkTimer::Ticket ticket, DeferredWorkTimer::TicketData&& ticketData) mutable {
- JSPromise* promise = jsCast<JSPromise*>(ticket);
- JSGlobalObject* globalObject = jsCast<JSGlobalObject*>(ticketData.dependencies[0].get());
- JSObject* importObject = jsCast<JSObject*>(ticketData.dependencies[1].get());
+ m_vm.deferredWorkTimer->scheduleWorkSoon(ticket, [result = WTFMove(result)](DeferredWorkTimer::Ticket ticket) mutable {
+ JSPromise* promise = jsCast<JSPromise*>(ticket->target());
+ JSGlobalObject* globalObject = jsCast<JSGlobalObject*>(ticket->dependencies[0].get());
+ JSObject* importObject = jsCast<JSObject*>(ticket->dependencies[1].get());
VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);
@@ -206,8 +204,14 @@
return;
m_eagerFailed = true;
}
- auto* promise = std::exchange(m_promise, nullptr);
- m_vm.deferredWorkTimer->cancelPendingWork(promise);
+ auto ticket = std::exchange(m_ticket, nullptr);
+ JSPromise* promise = jsCast<JSPromise*>(ticket->target());
+ // The pending work TicketData was keeping the promise alive. We need to
+ // make sure it is reachable from the stack before we remove it from the
+ // pending work list. Note: m_ticket stores it as a PackedPtr, which is not
+ // scannable by the GC.
+ WTF::compilerFence();
+ m_vm.deferredWorkTimer->cancelPendingWork(ticket);
promise->reject(globalObject, error);
}
@@ -220,8 +224,8 @@
return;
m_eagerFailed = true;
}
- auto* promise = std::exchange(m_promise, nullptr);
- m_vm.deferredWorkTimer->cancelPendingWork(promise);
+ auto ticket = std::exchange(m_ticket, nullptr);
+ m_vm.deferredWorkTimer->cancelPendingWork(ticket);
}
Modified: branches/safari-612-branch/Source/_javascript_Core/wasm/WasmStreamingCompiler.h (287649 => 287650)
--- branches/safari-612-branch/Source/_javascript_Core/wasm/WasmStreamingCompiler.h 2022-01-05 21:36:32 UTC (rev 287649)
+++ branches/safari-612-branch/Source/_javascript_Core/wasm/WasmStreamingCompiler.h 2022-01-05 21:44:38 UTC (rev 287650)
@@ -29,6 +29,7 @@
#if ENABLE(WEBASSEMBLY)
+#include "DeferredWorkTimer.h"
#include "JSCJSValue.h"
namespace JSC {
@@ -72,7 +73,7 @@
bool m_threadedCompilationStarted { false };
Lock m_lock;
unsigned m_remainingCompilationRequests { 0 };
- JSPromise* m_promise; // Raw pointer, but held by DeferredWorkTimer.
+ DeferredWorkTimer::Ticket m_ticket;
Ref<Wasm::ModuleInformation> m_info;
StreamingParser m_parser;
RefPtr<LLIntPlan> m_plan;
Modified: branches/safari-612-branch/Source/_javascript_Core/wasm/js/JSWebAssembly.cpp (287649 => 287650)
--- branches/safari-612-branch/Source/_javascript_Core/wasm/js/JSWebAssembly.cpp 2022-01-05 21:36:32 UTC (rev 287649)
+++ branches/safari-612-branch/Source/_javascript_Core/wasm/js/JSWebAssembly.cpp 2022-01-05 21:44:38 UTC (rev 287650)
@@ -172,9 +172,9 @@
Vector<Strong<JSCell>> dependencies;
dependencies.append(Strong<JSCell>(vm, globalObject));
- vm.deferredWorkTimer->addPendingWork(vm, promise, WTFMove(dependencies));
- Wasm::Module::validateAsync(&vm.wasmContext, WTFMove(source), createSharedTask<Wasm::Module::CallbackType>([promise, globalObject, &vm] (Wasm::Module::ValidationResult&& result) mutable {
- vm.deferredWorkTimer->scheduleWorkSoon(promise, [promise, globalObject, result = WTFMove(result), &vm](DeferredWorkTimer::Ticket, DeferredWorkTimer::TicketData&&) mutable {
+ auto ticket = vm.deferredWorkTimer->addPendingWork(vm, promise, WTFMove(dependencies));
+ Wasm::Module::validateAsync(&vm.wasmContext, WTFMove(source), createSharedTask<Wasm::Module::CallbackType>([ticket, promise, globalObject, &vm] (Wasm::Module::ValidationResult&& result) mutable {
+ vm.deferredWorkTimer->scheduleWorkSoon(ticket, [promise, globalObject, result = WTFMove(result), &vm](DeferredWorkTimer::Ticket) mutable {
auto scope = DECLARE_THROW_SCOPE(vm);
JSValue module = JSWebAssemblyModule::createStub(vm, globalObject, globalObject->webAssemblyModuleStructure(), WTFMove(result));
if (UNLIKELY(scope.exception())) {
@@ -203,11 +203,11 @@
dependencies.append(Strong<JSCell>(vm, promise));
dependencies.append(Strong<JSCell>(vm, importObject));
- vm.deferredWorkTimer->addPendingWork(vm, instance, WTFMove(dependencies));
+ auto ticket = vm.deferredWorkTimer->addPendingWork(vm, instance, WTFMove(dependencies));
// Note: This completion task may or may not get called immediately.
- module->module().compileAsync(&vm.wasmContext, instance->memoryMode(), createSharedTask<Wasm::CodeBlock::CallbackType>([promise, instance, module, importObject, resolveKind, creationMode, &vm] (Ref<Wasm::CodeBlock>&& refCodeBlock) mutable {
+ module->module().compileAsync(&vm.wasmContext, instance->memoryMode(), createSharedTask<Wasm::CodeBlock::CallbackType>([ticket, promise, instance, module, importObject, resolveKind, creationMode, &vm] (Ref<Wasm::CodeBlock>&& refCodeBlock) mutable {
RefPtr<Wasm::CodeBlock> codeBlock = WTFMove(refCodeBlock);
- vm.deferredWorkTimer->scheduleWorkSoon(instance, [promise, instance, module, importObject, resolveKind, creationMode, &vm, codeBlock = WTFMove(codeBlock)](DeferredWorkTimer::Ticket, DeferredWorkTimer::TicketData&&) mutable {
+ vm.deferredWorkTimer->scheduleWorkSoon(ticket, [promise, instance, module, importObject, resolveKind, creationMode, &vm, codeBlock = WTFMove(codeBlock)](DeferredWorkTimer::Ticket) mutable {
JSGlobalObject* globalObject = instance->globalObject();
resolve(vm, globalObject, promise, instance, module, importObject, codeBlock.releaseNonNull(), resolveKind, creationMode);
});
@@ -228,9 +228,9 @@
Vector<Strong<JSCell>> dependencies;
dependencies.append(Strong<JSCell>(vm, importObject));
dependencies.append(Strong<JSCell>(vm, moduleKeyCell));
- vm.deferredWorkTimer->addPendingWork(vm, promise, WTFMove(dependencies));
- Wasm::Module::validateAsync(&vm.wasmContext, WTFMove(source), createSharedTask<Wasm::Module::CallbackType>([promise, importObject, moduleKeyCell, globalObject, resolveKind, creationMode, &vm] (Wasm::Module::ValidationResult&& result) mutable {
- vm.deferredWorkTimer->scheduleWorkSoon(promise, [promise, importObject, moduleKeyCell, globalObject, result = WTFMove(result), resolveKind, creationMode, &vm](DeferredWorkTimer::Ticket, DeferredWorkTimer::TicketData&&) mutable {
+ auto ticket = vm.deferredWorkTimer->addPendingWork(vm, promise, WTFMove(dependencies));
+ Wasm::Module::validateAsync(&vm.wasmContext, WTFMove(source), createSharedTask<Wasm::Module::CallbackType>([ticket, promise, importObject, moduleKeyCell, globalObject, resolveKind, creationMode, &vm] (Wasm::Module::ValidationResult&& result) mutable {
+ vm.deferredWorkTimer->scheduleWorkSoon(ticket, [promise, importObject, moduleKeyCell, globalObject, result = WTFMove(result), resolveKind, creationMode, &vm](DeferredWorkTimer::Ticket) mutable {
auto scope = DECLARE_THROW_SCOPE(vm);
JSWebAssemblyModule* module = JSWebAssemblyModule::createStub(vm, globalObject, globalObject->webAssemblyModuleStructure(), WTFMove(result));
if (UNLIKELY(scope.exception())) {
@@ -272,9 +272,9 @@
Vector<Strong<JSCell>> dependencies;
dependencies.append(Strong<JSCell>(vm, importObject));
dependencies.append(Strong<JSCell>(vm, globalObject));
- vm.deferredWorkTimer->addPendingWork(vm, promise, WTFMove(dependencies));
- Wasm::Module::validateAsync(&vm.wasmContext, WTFMove(source), createSharedTask<Wasm::Module::CallbackType>([promise, importObject, globalObject, &vm] (Wasm::Module::ValidationResult&& result) mutable {
- vm.deferredWorkTimer->scheduleWorkSoon(promise, [promise, importObject, globalObject, result = WTFMove(result), &vm](DeferredWorkTimer::Ticket, DeferredWorkTimer::TicketData&&) mutable {
+ auto ticket = vm.deferredWorkTimer->addPendingWork(vm, promise, WTFMove(dependencies));
+ Wasm::Module::validateAsync(&vm.wasmContext, WTFMove(source), createSharedTask<Wasm::Module::CallbackType>([ticket, promise, importObject, globalObject, &vm] (Wasm::Module::ValidationResult&& result) mutable {
+ vm.deferredWorkTimer->scheduleWorkSoon(ticket, [promise, importObject, globalObject, result = WTFMove(result), &vm](DeferredWorkTimer::Ticket) mutable {
auto scope = DECLARE_THROW_SCOPE(vm);
JSWebAssemblyModule* module = JSWebAssemblyModule::createStub(vm, globalObject, globalObject->webAssemblyModuleStructure(), WTFMove(result));
if (UNLIKELY(scope.exception())) {
