From c1218b250d7ebbe039315763662473d8aaa34b46 Mon Sep 17 00:00:00 2001 From: 190n Date: Wed, 15 Jan 2025 04:06:52 -0800 Subject: [PATCH] Bump WebKit and re-enable IPInt (#16227) Co-authored-by: Jarred Sumner Co-authored-by: Kai Tamkun --- cmake/tools/SetupWebKit.cmake | 2 +- scripts/runner.node.mjs | 9 +- src/bun.js/bindings/BunDebugger.cpp | 4 +- src/bun.js/bindings/BunPlugin.h | 2 +- src/bun.js/bindings/DOMWrapperWorld-class.h | 2 +- .../bindings/InspectorLifecycleAgent.cpp | 3 + src/bun.js/bindings/InspectorLifecycleAgent.h | 1 + .../bindings/InspectorTestReporterAgent.cpp | 3 + .../bindings/InspectorTestReporterAgent.h | 1 + src/bun.js/bindings/JSCTaskScheduler.h | 4 +- .../bindings/ScriptExecutionContext.cpp | 78 ++++++++-- src/bun.js/bindings/ScriptExecutionContext.h | 44 +++--- src/bun.js/bindings/ZigGlobalObject.cpp | 10 +- src/bun.js/bindings/ZigGlobalObject.h | 6 +- src/bun.js/bindings/root.h | 2 - .../bindings/webcore/BroadcastChannel.cpp | 9 +- .../webcore/ContextDestructionObserver.cpp | 8 +- .../webcore/ContextDestructionObserver.h | 7 +- src/bun.js/bindings/webcore/MessagePort.cpp | 137 +++++++++--------- src/bun.js/bindings/webcore/MessagePort.h | 25 +++- .../bindings/webcore/MessagePortChannel.h | 5 +- .../webcore/MessagePortChannelProvider.h | 13 +- .../MessagePortChannelProviderImpl.cpp | 8 +- .../webcore/MessagePortChannelProviderImpl.h | 1 + .../webcore/MessagePortChannelRegistry.cpp | 33 ++--- .../webcore/MessagePortChannelRegistry.h | 9 +- src/bun.js/bindings/webcore/Worker.cpp | 16 +- src/bun.js/javascript.zig | 4 +- src/bun.js/test/jest.zig | 2 +- src/js_ast.zig | 1 + .../worker_threads/worker_threads.test.ts | 6 +- .../broadcast-channel.test.ts | 48 ++++-- 32 files changed, 312 insertions(+), 191 deletions(-) diff --git a/cmake/tools/SetupWebKit.cmake b/cmake/tools/SetupWebKit.cmake index ba34a517ab0aae..d00d3a3e96eecb 100644 --- a/cmake/tools/SetupWebKit.cmake +++ b/cmake/tools/SetupWebKit.cmake @@ -2,7 +2,7 @@ option(WEBKIT_VERSION "The version of WebKit to use") option(WEBKIT_LOCAL "If a local version of WebKit should be used instead of downloading") if(NOT WEBKIT_VERSION) - set(WEBKIT_VERSION e1a802a2287edfe7f4046a9dd8307c8b59f5d816) + set(WEBKIT_VERSION 9e3b60e4a6438d20ee6f8aa5bec6b71d2b7d213f) endif() string(SUBSTRING ${WEBKIT_VERSION} 0 16 WEBKIT_VERSION_PREFIX) diff --git a/scripts/runner.node.mjs b/scripts/runner.node.mjs index c66d8b60195472..cc0ebfcb86e966 100755 --- a/scripts/runner.node.mjs +++ b/scripts/runner.node.mjs @@ -199,7 +199,7 @@ async function runTests() { failure ||= result; flaky ||= true; - if (attempt >= maxAttempts) { + if (attempt >= maxAttempts || isAlwaysFailure(error)) { flaky = false; failedResults.push(failure); } @@ -1552,6 +1552,13 @@ function getExitCode(outcome) { return 1; } +// A flaky segfault, sigtrap, or sigill must never be ignored. +// If it happens in CI, it will happen to our users. +function isAlwaysFailure(error) { + error = ((error || "") + "").toLowerCase().trim(); + return error.includes("segmentation fault") || error.includes("sigill") || error.includes("sigtrap"); +} + /** * @param {string} signal */ diff --git a/src/bun.js/bindings/BunDebugger.cpp b/src/bun.js/bindings/BunDebugger.cpp index 9b9759591779a9..c0aeba45eb5447 100644 --- a/src/bun.js/bindings/BunDebugger.cpp +++ b/src/bun.js/bindings/BunDebugger.cpp @@ -26,7 +26,7 @@ class BunInspectorConnection; static WebCore::ScriptExecutionContext* debuggerScriptExecutionContext = nullptr; static WTF::Lock inspectorConnectionsLock = WTF::Lock(); -static WTF::HashMap>* inspectorConnections = nullptr; +static WTF::UncheckedKeyHashMap>* inspectorConnections = nullptr; static bool waitingForConnection = false; extern "C" void Debugger__didConnect(); @@ -487,7 +487,7 @@ extern "C" unsigned int Bun__createJSDebugger(Zig::GlobalObject* globalObject) { Locker locker(inspectorConnectionsLock); if (inspectorConnections == nullptr) { - inspectorConnections = new WTF::HashMap>(); + inspectorConnections = new WTF::UncheckedKeyHashMap>(); } inspectorConnections->add(globalObject->scriptExecutionContext()->identifier(), Vector()); diff --git a/src/bun.js/bindings/BunPlugin.h b/src/bun.js/bindings/BunPlugin.h index 09562f90f59680..ed54f2ff42e2eb 100644 --- a/src/bun.js/bindings/BunPlugin.h +++ b/src/bun.js/bindings/BunPlugin.h @@ -15,7 +15,7 @@ using namespace JSC; class BunPlugin { public: - using VirtualModuleMap = WTF::HashMap>; + using VirtualModuleMap = WTF::UncheckedKeyHashMap>; // This is a list of pairs of regexps and functions to match against class Group { diff --git a/src/bun.js/bindings/DOMWrapperWorld-class.h b/src/bun.js/bindings/DOMWrapperWorld-class.h index 75d14bfaf9969d..29b10884793f9b 100644 --- a/src/bun.js/bindings/DOMWrapperWorld-class.h +++ b/src/bun.js/bindings/DOMWrapperWorld-class.h @@ -50,7 +50,7 @@ class DOMWrapperWorld : public RefCounted { private: JSC::VM& m_vm; - HashSet m_jsWindowProxies; + UncheckedKeyHashSet m_jsWindowProxies; DOMObjectWrapperMap m_wrappers; String m_name; diff --git a/src/bun.js/bindings/InspectorLifecycleAgent.cpp b/src/bun.js/bindings/InspectorLifecycleAgent.cpp index f65b1ffcd4f473..090f9f59d7f0b4 100644 --- a/src/bun.js/bindings/InspectorLifecycleAgent.cpp +++ b/src/bun.js/bindings/InspectorLifecycleAgent.cpp @@ -11,9 +11,12 @@ #include #include #include "ConsoleObject.h" +#include namespace Inspector { +WTF_MAKE_TZONE_ALLOCATED_IMPL(InspectorLifecycleAgent); + // Zig bindings implementation extern "C" { diff --git a/src/bun.js/bindings/InspectorLifecycleAgent.h b/src/bun.js/bindings/InspectorLifecycleAgent.h index 5990b833f60840..8f2d9ebc0a9d22 100644 --- a/src/bun.js/bindings/InspectorLifecycleAgent.h +++ b/src/bun.js/bindings/InspectorLifecycleAgent.h @@ -18,6 +18,7 @@ enum class DisconnectReason; class InspectorLifecycleAgent final : public InspectorAgentBase, public Inspector::LifecycleReporterBackendDispatcherHandler { WTF_MAKE_NONCOPYABLE(InspectorLifecycleAgent); + WTF_MAKE_TZONE_ALLOCATED(InspectorLifecycleAgent); public: InspectorLifecycleAgent(JSC::JSGlobalObject&); diff --git a/src/bun.js/bindings/InspectorTestReporterAgent.cpp b/src/bun.js/bindings/InspectorTestReporterAgent.cpp index 00d8bbc7daa37b..f93097cc46318b 100644 --- a/src/bun.js/bindings/InspectorTestReporterAgent.cpp +++ b/src/bun.js/bindings/InspectorTestReporterAgent.cpp @@ -11,9 +11,12 @@ #include "ZigGlobalObject.h" #include "ModuleLoader.h" +#include namespace Inspector { +WTF_MAKE_TZONE_ALLOCATED_IMPL(InspectorTestReporterAgent); + // Zig bindings implementation extern "C" { diff --git a/src/bun.js/bindings/InspectorTestReporterAgent.h b/src/bun.js/bindings/InspectorTestReporterAgent.h index 7c6afcf2e0ec26..374553edd0976b 100644 --- a/src/bun.js/bindings/InspectorTestReporterAgent.h +++ b/src/bun.js/bindings/InspectorTestReporterAgent.h @@ -18,6 +18,7 @@ enum class DisconnectReason; class InspectorTestReporterAgent final : public InspectorAgentBase, public Inspector::TestReporterBackendDispatcherHandler { WTF_MAKE_NONCOPYABLE(InspectorTestReporterAgent); + WTF_MAKE_TZONE_ALLOCATED(InspectorTestReporterAgent); public: InspectorTestReporterAgent(JSC::JSGlobalObject&); diff --git a/src/bun.js/bindings/JSCTaskScheduler.h b/src/bun.js/bindings/JSCTaskScheduler.h index 29660c406d2bea..24e8eb56e3f6b0 100644 --- a/src/bun.js/bindings/JSCTaskScheduler.h +++ b/src/bun.js/bindings/JSCTaskScheduler.h @@ -22,8 +22,8 @@ class JSCTaskScheduler { public: Lock m_lock; - HashSet> m_pendingTicketsKeepingEventLoopAlive; - HashSet> m_pendingTicketsOther; + UncheckedKeyHashSet> m_pendingTicketsKeepingEventLoopAlive; + UncheckedKeyHashSet> m_pendingTicketsOther; }; } diff --git a/src/bun.js/bindings/ScriptExecutionContext.cpp b/src/bun.js/bindings/ScriptExecutionContext.cpp index 34534d6369b048..326aa556ba7949 100644 --- a/src/bun.js/bindings/ScriptExecutionContext.cpp +++ b/src/bun.js/bindings/ScriptExecutionContext.cpp @@ -7,15 +7,61 @@ #include "_libusockets.h" #include "BunClientData.h" #include "EventLoopTask.h" - +#include "BunBroadcastChannelRegistry.h" +#include extern "C" void Bun__startLoop(us_loop_t* loop); namespace WebCore { +static constexpr ScriptExecutionContextIdentifier INITIAL_IDENTIFIER_INTERNAL = 1; -static std::atomic lastUniqueIdentifier = 0; +static std::atomic lastUniqueIdentifier = INITIAL_IDENTIFIER_INTERNAL; + +#if ASSERT_ENABLED +static ScriptExecutionContextIdentifier initialIdentifier() +{ + static bool hasCalledInitialIdentifier = false; + ASSERT_WITH_MESSAGE(!hasCalledInitialIdentifier, "ScriptExecutionContext::initialIdentifier() cannot be called more than once. Use generateIdentifier() instead."); + hasCalledInitialIdentifier = true; + return INITIAL_IDENTIFIER_INTERNAL; +} +#else +static ScriptExecutionContextIdentifier initialIdentifier() +{ + return INITIAL_IDENTIFIER_INTERNAL; +} +#endif + +DEFINE_ALLOCATOR_WITH_HEAP_IDENTIFIER(ScriptExecutionContext); + +ScriptExecutionContext::ScriptExecutionContext(JSC::VM* vm, JSC::JSGlobalObject* globalObject) + : m_vm(vm) + , m_globalObject(globalObject) + , m_identifier(initialIdentifier()) + , m_broadcastChannelRegistry([](auto& owner, auto& lazyRef) { + lazyRef.set(BunBroadcastChannelRegistry::create()); + }) +{ + relaxAdoptionRequirement(); + addToContextsMap(); +} + +ScriptExecutionContext::ScriptExecutionContext(JSC::VM* vm, JSC::JSGlobalObject* globalObject, ScriptExecutionContextIdentifier identifier) + : m_vm(vm) + , m_globalObject(globalObject) + , m_identifier(identifier == std::numeric_limits::max() ? ++lastUniqueIdentifier : identifier) + , m_broadcastChannelRegistry([](auto& owner, auto& lazyRef) { + lazyRef.set(BunBroadcastChannelRegistry::create()); + }) +{ + relaxAdoptionRequirement(); + addToContextsMap(); +} WTF_MAKE_ISO_ALLOCATED_IMPL(EventLoopTask); + +#if !ENABLE(MALLOC_BREAKDOWN) WTF_MAKE_ISO_ALLOCATED_IMPL(ScriptExecutionContext); +#endif static Lock allScriptExecutionContextsMapLock; static HashMap& allScriptExecutionContextsMap() WTF_REQUIRES_LOCK(allScriptExecutionContextsMapLock) @@ -84,10 +130,13 @@ ScriptExecutionContext::~ScriptExecutionContext() { checkConsistency(); +#if ASSERT_ENABLED { Locker locker { allScriptExecutionContextsMapLock }; ASSERT_WITH_MESSAGE(!allScriptExecutionContextsMap().contains(m_identifier), "A ScriptExecutionContext subclass instance implementing postTask should have already removed itself from the map"); } + m_inScriptExecutionContextDestructor = true; +#endif // ASSERT_ENABLED auto postMessageCompletionHandlers = WTFMove(m_processMessageWithMessagePortsSoonHandlers); for (auto& completionHandler : postMessageCompletionHandlers) @@ -95,6 +144,10 @@ ScriptExecutionContext::~ScriptExecutionContext() while (auto* destructionObserver = m_destructionObservers.takeAny()) destructionObserver->contextDestroyed(); + +#if ASSERT_ENABLED + m_inScriptExecutionContextDestructor = false; +#endif // ASSERT_ENABLED } bool ScriptExecutionContext::postTaskTo(ScriptExecutionContextIdentifier identifier, Function&& task) @@ -111,12 +164,17 @@ bool ScriptExecutionContext::postTaskTo(ScriptExecutionContextIdentifier identif void ScriptExecutionContext::didCreateDestructionObserver(ContextDestructionObserver& observer) { - // ASSERT(!m_inScriptExecutionContextDestructor); +#if ASSERT_ENABLED + ASSERT(!m_inScriptExecutionContextDestructor); +#endif // ASSERT_ENABLED m_destructionObservers.add(&observer); } void ScriptExecutionContext::willDestroyDestructionObserver(ContextDestructionObserver& observer) { +#if ASSERT_ENABLED + ASSERT(!m_inScriptExecutionContextDestructor); +#endif // ASSERT_ENABLED m_destructionObservers.remove(&observer); } @@ -212,16 +270,14 @@ void ScriptExecutionContext::dispatchMessagePortEvents() void ScriptExecutionContext::checkConsistency() const { - // for (auto* messagePort : m_messagePorts) - // ASSERT(messagePort->scriptExecutionContext() == this); +#if ASSERT_ENABLED + for (auto* messagePort : m_messagePorts) + ASSERT(messagePort->scriptExecutionContext() == this); - // for (auto* destructionObserver : m_destructionObservers) - // ASSERT(destructionObserver->scriptExecutionContext() == this); + for (auto* destructionObserver : m_destructionObservers) + ASSERT(destructionObserver->scriptExecutionContext() == this); - // for (auto* activeDOMObject : m_activeDOMObjects) { - // ASSERT(activeDOMObject->scriptExecutionContext() == this); - // activeDOMObject->assertSuspendIfNeededWasCalled(); - // } +#endif // ASSERT_ENABLED } void ScriptExecutionContext::createdMessagePort(MessagePort& messagePort) diff --git a/src/bun.js/bindings/ScriptExecutionContext.h b/src/bun.js/bindings/ScriptExecutionContext.h index 23bca74153877b..6122948614cdbb 100644 --- a/src/bun.js/bindings/ScriptExecutionContext.h +++ b/src/bun.js/bindings/ScriptExecutionContext.h @@ -3,7 +3,6 @@ #include "root.h" #include "ActiveDOMObject.h" #include "ContextDestructionObserver.h" -#include "BunBroadcastChannelRegistry.h" #include #include #include @@ -12,7 +11,9 @@ #include #include #include "CachedScript.h" +#include "wtf/ThreadSafeWeakPtr.h" #include +#include namespace uWS { template @@ -26,34 +27,25 @@ struct us_loop_t; namespace WebCore { class WebSocket; +class BunBroadcastChannelRegistry; class MessagePort; class ScriptExecutionContext; class EventLoopTask; using ScriptExecutionContextIdentifier = uint32_t; +DECLARE_ALLOCATOR_WITH_HEAP_IDENTIFIER(ScriptExecutionContext); -class ScriptExecutionContext : public CanMakeWeakPtr { +class ScriptExecutionContext : public CanMakeWeakPtr, public RefCounted { +#if ENABLE(MALLOC_BREAKDOWN) + WTF_MAKE_FAST_ALLOCATED_WITH_HEAP_IDENTIFIER(ScriptExecutionContext); +#else WTF_MAKE_ISO_ALLOCATED(ScriptExecutionContext); +#endif public: - ScriptExecutionContext(JSC::VM* vm, JSC::JSGlobalObject* globalObject) - : m_vm(vm) - , m_globalObject(globalObject) - , m_identifier(0) - , m_broadcastChannelRegistry(BunBroadcastChannelRegistry::create()) - { - regenerateIdentifier(); - } - - ScriptExecutionContext(JSC::VM* vm, JSC::JSGlobalObject* globalObject, ScriptExecutionContextIdentifier identifier) - : m_vm(vm) - , m_globalObject(globalObject) - , m_identifier(identifier) - , m_broadcastChannelRegistry(BunBroadcastChannelRegistry::create()) - { - addToContextsMap(); - } + ScriptExecutionContext(JSC::VM* vm, JSC::JSGlobalObject* globalObject); + ScriptExecutionContext(JSC::VM* vm, JSC::JSGlobalObject* globalObject, ScriptExecutionContextIdentifier identifier); ~ScriptExecutionContext(); @@ -77,6 +69,8 @@ class ScriptExecutionContext : public CanMakeWeakPtr { static ScriptExecutionContext* getScriptExecutionContext(ScriptExecutionContextIdentifier identifier); void refEventLoop(); void unrefEventLoop(); + using RefCounted::deref; + using RefCounted::ref; const WTF::URL& url() const { @@ -156,7 +150,7 @@ class ScriptExecutionContext : public CanMakeWeakPtr { m_vm = &globalObject->vm(); } - BunBroadcastChannelRegistry& broadcastChannelRegistry() { return m_broadcastChannelRegistry; } + BunBroadcastChannelRegistry& broadcastChannelRegistry() { return m_broadcastChannelRegistry.get(*this); } static ScriptExecutionContext* getMainThreadScriptExecutionContext(); @@ -166,10 +160,10 @@ class ScriptExecutionContext : public CanMakeWeakPtr { WTF::URL m_url = WTF::URL(); ScriptExecutionContextIdentifier m_identifier; - HashSet m_messagePorts; - HashSet m_destructionObservers; + UncheckedKeyHashSet m_messagePorts; + UncheckedKeyHashSet m_destructionObservers; Vector> m_processMessageWithMessagePortsSoonHandlers; - Ref m_broadcastChannelRegistry; + LazyRef m_broadcastChannelRegistry; bool m_willProcessMessageWithMessagePortsSoon { false }; @@ -202,6 +196,10 @@ class ScriptExecutionContext : public CanMakeWeakPtr { return m_connected_client_websockets_ctx; } } + +#if ASSERT_ENABLED + bool m_inScriptExecutionContextDestructor = false; +#endif }; ScriptExecutionContext* executionContext(JSC::JSGlobalObject*); diff --git a/src/bun.js/bindings/ZigGlobalObject.cpp b/src/bun.js/bindings/ZigGlobalObject.cpp index 3cb729fd1391ca..afd4d0f32f59f8 100644 --- a/src/bun.js/bindings/ZigGlobalObject.cpp +++ b/src/bun.js/bindings/ZigGlobalObject.cpp @@ -1,7 +1,6 @@ #include "root.h" -#include "JavaScriptCore/PropertySlot.h" #include "ZigGlobalObject.h" #include "helpers.h" #include "JavaScriptCore/ArgList.h" @@ -244,9 +243,6 @@ extern "C" void JSCInitialize(const char* envp[], size_t envc, void (*onCrash)(c JSC::Options::useConcurrentJIT() = true; // JSC::Options::useSigillCrashAnalyzer() = true; JSC::Options::useWasm() = true; - // Disable IPInt, the in-place WASM interpreter, by default until it is more stable - // (it breaks pglite as of 2025-01-06) - JSC::Options::useWasmIPInt() = false; JSC::Options::useSourceProviderCache() = true; // JSC::Options::useUnlinkedCodeBlockJettisoning() = false; JSC::Options::exposeInternalModuleLoader() = true; @@ -859,6 +855,9 @@ void Zig::GlobalObject::resetOnEachMicrotaskTick() } } +// executionContextId: -1 for main thread +// executionContextId: maxInt32 for macros +// executionContextId: >-1 for workers extern "C" JSC__JSGlobalObject* Zig__GlobalObject__create(void* console_client, int32_t executionContextId, bool miniMode, bool evalMode, void* worker_ptr) { auto heapSize = miniMode ? JSC::HeapType::Small : JSC::HeapType::Large; @@ -875,7 +874,7 @@ extern "C" JSC__JSGlobalObject* Zig__GlobalObject__create(void* console_client, WebCore::JSVMClientData::create(&vm, Bun__getVM()); const auto createGlobalObject = [&]() -> Zig::GlobalObject* { - if (UNLIKELY(executionContextId > -1)) { + if (UNLIKELY(executionContextId == std::numeric_limits::max() || executionContextId > -1)) { auto* structure = Zig::GlobalObject::createStructure(vm); if (UNLIKELY(!structure)) { return nullptr; @@ -1214,6 +1213,7 @@ GlobalObject::~GlobalObject() if (auto* ctx = scriptExecutionContext()) { ctx->removeFromContextsMap(); + ctx->deref(); } } diff --git a/src/bun.js/bindings/ZigGlobalObject.h b/src/bun.js/bindings/ZigGlobalObject.h index e3fdb82956a737..322de6b989c216 100644 --- a/src/bun.js/bindings/ZigGlobalObject.h +++ b/src/bun.js/bindings/ZigGlobalObject.h @@ -73,8 +73,8 @@ namespace Zig { class JSCStackTrace; -using JSDOMStructureMap = HashMap>; -using DOMGuardedObjectSet = HashSet; +using JSDOMStructureMap = UncheckedKeyHashMap>; +using DOMGuardedObjectSet = UncheckedKeyHashSet; #define ZIG_GLOBAL_OBJECT_DEFINED @@ -451,7 +451,7 @@ class GlobalObject : public Bun::GlobalScope { // This increases the cache hit rate for JSC::VM's SourceProvider cache // It also avoids an extra allocation for the SourceProvider // The key is a pointer to the source code - WTF::HashMap> sourceProviderMap; + WTF::UncheckedKeyHashMap> sourceProviderMap; size_t reloadCount = 0; void reload(); diff --git a/src/bun.js/bindings/root.h b/src/bun.js/bindings/root.h index 0fb85524cab446..22615ad374a9e5 100644 --- a/src/bun.js/bindings/root.h +++ b/src/bun.js/bindings/root.h @@ -59,8 +59,6 @@ #include #endif -#include - /* Disabling warning C4206: nonstandard extension used: translation unit is empty. By design, we rely on #define flags to make some translation units empty. Make sure this warning does not turn into an error. diff --git a/src/bun.js/bindings/webcore/BroadcastChannel.cpp b/src/bun.js/bindings/webcore/BroadcastChannel.cpp index dbd4df5ff41ae7..d52911bb789e59 100644 --- a/src/bun.js/bindings/webcore/BroadcastChannel.cpp +++ b/src/bun.js/bindings/webcore/BroadcastChannel.cpp @@ -28,6 +28,7 @@ #include "BunClientData.h" #include "BroadcastChannelRegistry.h" +#include "BunBroadcastChannelRegistry.h" #include "EventNames.h" #include "EventTarget.h" #include "MessageEvent.h" @@ -53,16 +54,16 @@ namespace WebCore { WTF_MAKE_ISO_ALLOCATED_IMPL(BroadcastChannel); static Lock allBroadcastChannelsLock; -static HashMap& allBroadcastChannels() WTF_REQUIRES_LOCK(allBroadcastChannelsLock) +static UncheckedKeyHashMap& allBroadcastChannels() WTF_REQUIRES_LOCK(allBroadcastChannelsLock) { - static NeverDestroyed> map; + static NeverDestroyed> map; return map; } static Lock channelToContextIdentifierLock; -static HashMap& channelToContextIdentifier() +static UncheckedKeyHashMap& channelToContextIdentifier() { - static NeverDestroyed> map; + static NeverDestroyed> map; return map; } diff --git a/src/bun.js/bindings/webcore/ContextDestructionObserver.cpp b/src/bun.js/bindings/webcore/ContextDestructionObserver.cpp index 72e6a85c497529..1de690c45a3d7f 100644 --- a/src/bun.js/bindings/webcore/ContextDestructionObserver.cpp +++ b/src/bun.js/bindings/webcore/ContextDestructionObserver.cpp @@ -26,6 +26,7 @@ #include "config.h" #include "ContextDestructionObserver.h" +#include #include "ScriptExecutionContext.h" @@ -42,6 +43,11 @@ ContextDestructionObserver::~ContextDestructionObserver() observeContext(nullptr); } +RefPtr ContextDestructionObserver::protectedScriptExecutionContext() const +{ + return m_context.get(); +} + void ContextDestructionObserver::observeContext(ScriptExecutionContext* scriptExecutionContext) { if (m_context) { @@ -49,7 +55,7 @@ void ContextDestructionObserver::observeContext(ScriptExecutionContext* scriptEx m_context->willDestroyDestructionObserver(*this); } - m_context = scriptExecutionContext; + m_context = WeakPtr { scriptExecutionContext, EnableWeakPtrThreadingAssertions::No }; if (m_context) { ASSERT(m_context->isContextThread()); diff --git a/src/bun.js/bindings/webcore/ContextDestructionObserver.h b/src/bun.js/bindings/webcore/ContextDestructionObserver.h index 7408b5f2728680..f206ad80354e16 100644 --- a/src/bun.js/bindings/webcore/ContextDestructionObserver.h +++ b/src/bun.js/bindings/webcore/ContextDestructionObserver.h @@ -13,7 +13,8 @@ class ContextDestructionObserver { public: WEBCORE_EXPORT virtual void contextDestroyed(); - ScriptExecutionContext* scriptExecutionContext() const { return m_context; } + ScriptExecutionContext* scriptExecutionContext() const { return m_context.get(); } + RefPtr protectedScriptExecutionContext() const; protected: WEBCORE_EXPORT ContextDestructionObserver(ScriptExecutionContext*); @@ -21,7 +22,7 @@ class ContextDestructionObserver { void observeContext(ScriptExecutionContext*); private: - ScriptExecutionContext* m_context; + WeakPtr m_context; }; -} \ No newline at end of file +} diff --git a/src/bun.js/bindings/webcore/MessagePort.cpp b/src/bun.js/bindings/webcore/MessagePort.cpp index aa05ebbbbb11d0..3acb2416868f23 100644 --- a/src/bun.js/bindings/webcore/MessagePort.cpp +++ b/src/bun.js/bindings/webcore/MessagePort.cpp @@ -49,50 +49,36 @@ extern "C" void Bun__eventLoop__incrementRefConcurrently(void* bunVM, int delta) namespace WebCore { +#if ENABLE(MALLOC_BREAKDOWN) +DEFINE_ALLOCATOR_WITH_HEAP_IDENTIFIER(MessagePort); +#else WTF_MAKE_ISO_ALLOCATED_IMPL(MessagePort); +#endif static Lock allMessagePortsLock; -static HashMap& allMessagePorts() WTF_REQUIRES_LOCK(allMessagePortsLock) +static UncheckedKeyHashMap>& allMessagePorts() WTF_REQUIRES_LOCK(allMessagePortsLock) { - static NeverDestroyed> map; + static NeverDestroyed>> map; return map; } -static HashMap& portToContextIdentifier() WTF_REQUIRES_LOCK(allMessagePortsLock) +static UncheckedKeyHashMap& portToContextIdentifier() WTF_REQUIRES_LOCK(allMessagePortsLock) { - static NeverDestroyed> map; + static NeverDestroyed> map; return map; } -void MessagePort::ref() const -{ - ++m_refCount; -} - -void MessagePort::deref() const +bool MessagePort::hasPendingActivity() const { - // This custom deref() function ensures that as long as the lock to allMessagePortsLock is taken, no MessagePort will be destroyed. - // This allows notifyMessageAvailable to easily query the map and manipulate MessagePort instances. - - if (!--m_refCount) { - Locker locker { allMessagePortsLock }; + // If the ScriptExecutionContext has been shut down on this object close()'ed, we can GC. + if (!scriptExecutionContext() || m_isDetached) + return false; - if (m_refCount) - return; + // If this MessagePort has no message event handler then there is no point in keeping it alive. + if (!m_hasMessageEventListener) + return false; - auto iterator = allMessagePorts().find(m_identifier); - if (iterator != allMessagePorts().end() && iterator->value == this) { - allMessagePorts().remove(iterator); - portToContextIdentifier().remove(m_identifier); - } - - delete this; - } -} - -bool MessagePort::hasPendingActivity() const -{ - return m_refCount > 0; + return m_entangled; } bool MessagePort::isMessagePortAliveForTesting(const MessagePortIdentifier& identifier) @@ -101,29 +87,20 @@ bool MessagePort::isMessagePortAliveForTesting(const MessagePortIdentifier& iden return allMessagePorts().contains(identifier); } -ScriptExecutionContextIdentifier MessagePort::contextIdForMessagePortId(MessagePortIdentifier messagePortId) -{ - Locker locker { allMessagePortsLock }; - return portToContextIdentifier().get(messagePortId); -} - void MessagePort::notifyMessageAvailable(const MessagePortIdentifier& identifier) { - ScriptExecutionContextIdentifier scriptExecutionContextIdentifier; + std::optional scriptExecutionContextIdentifier; + ThreadSafeWeakPtr weakPort; { Locker locker { allMessagePortsLock }; - scriptExecutionContextIdentifier = portToContextIdentifier().get(identifier); + scriptExecutionContextIdentifier = portToContextIdentifier().getOptional(identifier); + weakPort = allMessagePorts().get(identifier); } if (!scriptExecutionContextIdentifier) return; - ScriptExecutionContext::ensureOnContextThread(scriptExecutionContextIdentifier, [identifier](auto&) { - RefPtr port; - { - Locker locker { allMessagePortsLock }; - port = allMessagePorts().get(identifier); - } - if (port) + ScriptExecutionContext::ensureOnContextThread(*scriptExecutionContextIdentifier, [weakPort = WTFMove(weakPort)](auto&) { + if (RefPtr port = weakPort.get()) port->messageAvailable(); }); } @@ -157,15 +134,23 @@ MessagePort::MessagePort(ScriptExecutionContext& scriptExecutionContext, const M MessagePort::~MessagePort() { - // LOG(MessagePorts, "Destroyed MessagePort %s (%p) in process %" PRIu64, m_identifier.logString().utf8().data(), this, WebCore::Process::identifier().toUInt64()); - ASSERT(allMessagePortsLock.isLocked()); + Locker locker { allMessagePortsLock }; + + auto iterator = allMessagePorts().find(m_identifier); + if (iterator != allMessagePorts().end()) { + // ThreadSafeWeakPtr::get() returns null as soon as the object has started destruction. + if (RefPtr messagePort = iterator->value.get(); !messagePort) { + allMessagePorts().remove(iterator); + portToContextIdentifier().remove(m_identifier); + } + } if (m_entangled) close(); - if (auto contextId = portToContextIdentifier().get(m_identifier)) - ScriptExecutionContext::getScriptExecutionContext(contextId)->destroyedMessagePort(*this); + if (auto* context = scriptExecutionContext()) + context->destroyedMessagePort(*this); } void MessagePort::entangle() @@ -191,7 +176,7 @@ ExceptionOr MessagePort::postMessage(JSC::JSGlobalObject& state, JSC::JSVa if (!ports.isEmpty()) { for (auto& port : ports) { if (port->identifier() == m_identifier || port->identifier() == m_remoteIdentifier) - return Exception { DataCloneError }; + return Exception { ExceptionCode::DataCloneError }; } auto disentangleResult = MessagePort::disentanglePorts(WTFMove(ports)); @@ -202,12 +187,7 @@ ExceptionOr MessagePort::postMessage(JSC::JSGlobalObject& state, JSC::JSVa MessageWithMessagePorts message { messageData.releaseReturnValue(), WTFMove(transferredPorts) }; - // LOG(MessagePorts, "Actually posting message to port %s (to be received by port %s)", m_identifier.logString().utf8().data(), m_remoteIdentifier.logString().utf8().data()); - - ScriptExecutionContextIdentifier contextId = contextIdForMessagePortId(m_remoteIdentifier); - - MessagePortChannelProvider::fromContext(*ScriptExecutionContext::getScriptExecutionContext(contextId)).postMessageToRemote(WTFMove(message), m_remoteIdentifier); - + MessagePortChannelProvider::fromContext(*protectedScriptExecutionContext()).postMessageToRemote(WTFMove(message), m_remoteIdentifier); return {}; } @@ -273,7 +253,7 @@ void MessagePort::dispatchMessages() // The HTML5 spec specifies that any messages sent to a document that is not fully active should be dropped, so this behavior is OK. ASSERT(started()); - auto* context = scriptExecutionContext(); + RefPtr context = scriptExecutionContext(); if (!context || context->activeDOMObjectsAreSuspended() || !isEntangled()) return; @@ -282,26 +262,33 @@ void MessagePort::dispatchMessages() // LOG(MessagePorts, "MessagePort %s (%p) dispatching %zu messages", m_identifier.logString().utf8().data(), this, messages.size()); - auto* context = scriptExecutionContext(); - if (!context || !context->jsGlobalObject()) + RefPtr context = scriptExecutionContext(); + if (!context || !context->globalObject()) return; - // ASSERT(context->isContextThread()); + ASSERT(context->isContextThread()); + auto* globalObject = defaultGlobalObject(context->globalObject()); + Ref vm = globalObject->vm(); + auto scope = DECLARE_CATCH_SCOPE(vm); - // bool contextIsWorker = is(*context); for (auto& message : messages) { // close() in Worker onmessage handler should prevent next message from dispatching. - // if (contextIsWorker && downcast(*context).isClosing()) - // return; + if (Zig::GlobalObject::scriptExecutionStatus(globalObject, globalObject) != ScriptExecutionStatus::Running) + return; auto ports = MessagePort::entanglePorts(*context, WTFMove(message.transferredPorts)); + if (UNLIKELY(scope.exception())) { + // Currently, we assume that the only way we can get here is if we have a termination. + RELEASE_ASSERT(vm->hasPendingTerminationException()); + return; + } // Per specification, each MessagePort object has a task source called the port message queue. // queueTaskKeepingObjectAlive(context, *this, TaskSource::PostedMessageQueue, [this, event = WTFMove(event)] { // dispatchEvent(event.event); // }); - ScriptExecutionContext::postTaskTo(contextIdForMessagePortId(m_identifier), [protectedThis = Ref { *this }, ports = WTFMove(ports), message = WTFMove(message)](ScriptExecutionContext& context) mutable { + ScriptExecutionContext::postTaskTo(context->identifier(), [protectedThis = Ref { *this }, ports = WTFMove(ports), message = WTFMove(message)](ScriptExecutionContext& context) mutable { auto event = MessageEvent::create(*context.jsGlobalObject(), message.message.releaseNonNull(), {}, {}, {}, WTFMove(ports)); protectedThis->dispatchEvent(event.event); }); @@ -392,6 +379,14 @@ Vector> MessagePort::entanglePorts(ScriptExecutionContext& c }); } +void MessagePort::contextDestroyed() +{ + ASSERT(scriptExecutionContext()); + + close(); + // ActiveDOMObject::contextDestroyed(); +} + void MessagePort::onDidChangeListenerImpl(EventTarget& self, const AtomString& eventType, OnDidChangeListenerKind kind) { if (eventType == eventNames().messageEvent) { @@ -399,19 +394,25 @@ void MessagePort::onDidChangeListenerImpl(EventTarget& self, const AtomString& e switch (kind) { case Add: if (port.m_messageEventCount == 0) { - port.scriptExecutionContext()->refEventLoop(); + auto* context = port.scriptExecutionContext(); + if (context) + context->refEventLoop(); } port.m_messageEventCount++; break; case Remove: port.m_messageEventCount--; if (port.m_messageEventCount == 0) { - port.scriptExecutionContext()->unrefEventLoop(); + auto* context = port.scriptExecutionContext(); + if (context) + context->unrefEventLoop(); } break; case Clear: if (port.m_messageEventCount > 0) { - port.scriptExecutionContext()->unrefEventLoop(); + auto* context = port.scriptExecutionContext(); + if (context) + context->unrefEventLoop(); } port.m_messageEventCount = 0; break; @@ -461,6 +462,7 @@ void MessagePort::jsRef(JSGlobalObject* lexicalGlobalObject) { if (!m_hasRef) { m_hasRef = true; + ref(); Bun__eventLoop__incrementRefConcurrently(WebCore::clientData(lexicalGlobalObject->vm())->bunVM, 1); } } @@ -469,6 +471,7 @@ void MessagePort::jsUnref(JSGlobalObject* lexicalGlobalObject) { if (m_hasRef) { m_hasRef = false; + deref(); Bun__eventLoop__incrementRefConcurrently(WebCore::clientData(lexicalGlobalObject->vm())->bunVM, -1); } } diff --git a/src/bun.js/bindings/webcore/MessagePort.h b/src/bun.js/bindings/webcore/MessagePort.h index d4532433fe4ed3..aa386fb27a8fd1 100644 --- a/src/bun.js/bindings/webcore/MessagePort.h +++ b/src/bun.js/bindings/webcore/MessagePort.h @@ -47,9 +47,15 @@ class WebCoreOpaqueRoot; struct StructuredSerializeOptions; -class MessagePort final : /* public ActiveDOMObject, */ public ContextDestructionObserver, public EventTarget { +DECLARE_ALLOCATOR_WITH_HEAP_IDENTIFIER(MessagePort); + +class MessagePort final : /* public ActiveDOMObject, */ public ContextDestructionObserver, public EventTarget, public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr { WTF_MAKE_NONCOPYABLE(MessagePort); +#if ENABLE(MALLOC_BREAKDOWN) + WTF_MAKE_FAST_ALLOCATED_WITH_HEAP_IDENTIFIER(MessagePort); +#else WTF_MAKE_ISO_ALLOCATED(MessagePort); +#endif public: static Ref create(ScriptExecutionContext&, const MessagePortIdentifier& local, const MessagePortIdentifier& remote); @@ -82,15 +88,21 @@ class MessagePort final : /* public ActiveDOMObject, */ public ContextDestructio const MessagePortIdentifier& identifier() const { return m_identifier; } const MessagePortIdentifier& remoteIdentifier() const { return m_remoteIdentifier; } - WEBCORE_EXPORT void ref() const; - WEBCORE_EXPORT void deref() const; + void ref() const + { + ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr::ref(); + } + void deref() const + { + ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr::deref(); + } // EventTarget. EventTargetInterface eventTargetInterface() const final { return MessagePortEventTargetInterfaceType; } - ScriptExecutionContext* scriptExecutionContext() const final { return ScriptExecutionContext::getScriptExecutionContext(contextIdForMessagePortId(m_identifier)); } + ScriptExecutionContext* scriptExecutionContext() const final { return this->ContextDestructionObserver::scriptExecutionContext(); } void refEventTarget() final { ref(); } void derefEventTarget() final { deref(); } @@ -103,8 +115,6 @@ class MessagePort final : /* public ActiveDOMObject, */ public ContextDestructio bool hasPendingActivity() const; - static ScriptExecutionContextIdentifier contextIdForMessagePortId(MessagePortIdentifier); - void jsRef(JSGlobalObject*); void jsUnref(JSGlobalObject*); bool jsHasRef() { return m_hasRef; } @@ -117,8 +127,7 @@ class MessagePort final : /* public ActiveDOMObject, */ public ContextDestructio // ActiveDOMObject // const char* activeDOMObjectName() const final; - // void contextDestroyed() final; - // void stop() final { close(); } + void contextDestroyed() final; // bool virtualHasPendingActivity() const final; EventTargetData* eventTargetData() final { return &m_eventTargetData; } diff --git a/src/bun.js/bindings/webcore/MessagePortChannel.h b/src/bun.js/bindings/webcore/MessagePortChannel.h index d50105badb3cce..1f3e408b61e38a 100644 --- a/src/bun.js/bindings/webcore/MessagePortChannel.h +++ b/src/bun.js/bindings/webcore/MessagePortChannel.h @@ -32,12 +32,13 @@ #include #include #include +#include namespace WebCore { class MessagePortChannelRegistry; -class MessagePortChannel : public RefCounted { +class MessagePortChannel : public RefCountedAndCanMakeWeakPtr { public: static Ref create(MessagePortChannelRegistry&, const MessagePortIdentifier& port1, const MessagePortIdentifier& port2); @@ -75,7 +76,7 @@ class MessagePortChannel : public RefCounted { std::optional m_processes[2]; RefPtr m_entangledToProcessProtectors[2]; Vector m_pendingMessages[2]; - HashSet> m_pendingMessagePortTransfers[2]; + UncheckedKeyHashSet> m_pendingMessagePortTransfers[2]; RefPtr m_pendingMessageProtectors[2]; uint64_t m_messageBatchesInFlight { 0 }; diff --git a/src/bun.js/bindings/webcore/MessagePortChannelProvider.h b/src/bun.js/bindings/webcore/MessagePortChannelProvider.h index 26ddfd460c1db3..843171df627f64 100644 --- a/src/bun.js/bindings/webcore/MessagePortChannelProvider.h +++ b/src/bun.js/bindings/webcore/MessagePortChannelProvider.h @@ -31,17 +31,25 @@ #include #include +namespace WebCore { +class MessagePortChannelProvider; +} + +namespace WTF { +template struct IsDeprecatedWeakRefSmartPointerException; +template<> struct IsDeprecatedWeakRefSmartPointerException : std::true_type {}; +} + namespace WebCore { class ScriptExecutionContext; struct MessagePortIdentifier; struct MessageWithMessagePorts; -class MessagePortChannelProvider { +class MessagePortChannelProvider : public CanMakeWeakPtr { public: static MessagePortChannelProvider& fromContext(ScriptExecutionContext&); static MessagePortChannelProvider& singleton(); - // WEBCORE_EXPORT static void setSharedProvider(MessagePortChannelProvider&); virtual ~MessagePortChannelProvider() {} @@ -53,7 +61,6 @@ class MessagePortChannelProvider { virtual void takeAllMessagesForPort(const MessagePortIdentifier&, CompletionHandler&&, CompletionHandler&&)>&&) = 0; virtual std::optional tryTakeMessageForPort(const MessagePortIdentifier&) = 0; - virtual void postMessageToRemote(MessageWithMessagePorts&&, const MessagePortIdentifier& remoteTarget) = 0; }; diff --git a/src/bun.js/bindings/webcore/MessagePortChannelProviderImpl.cpp b/src/bun.js/bindings/webcore/MessagePortChannelProviderImpl.cpp index b569c0d18de232..62b9818bbbe1ea 100644 --- a/src/bun.js/bindings/webcore/MessagePortChannelProviderImpl.cpp +++ b/src/bun.js/bindings/webcore/MessagePortChannelProviderImpl.cpp @@ -22,7 +22,7 @@ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF * THE POSSIBILITY OF SUCH DAMAGE. */ - +#include "root.h" #include "config.h" #include "MessagePortChannelProviderImpl.h" @@ -32,9 +32,7 @@ namespace WebCore { -MessagePortChannelProviderImpl::MessagePortChannelProviderImpl() -{ -} +MessagePortChannelProviderImpl::MessagePortChannelProviderImpl() = default; MessagePortChannelProviderImpl::~MessagePortChannelProviderImpl() { @@ -48,7 +46,7 @@ void MessagePortChannelProviderImpl::createNewMessagePortChannel(const MessagePo void MessagePortChannelProviderImpl::entangleLocalPortInThisProcessToRemote(const MessagePortIdentifier& local, const MessagePortIdentifier& remote) { - m_registry.didEntangleLocalToRemote(local, remote, WebCore::Process::identifier()); + m_registry.didEntangleLocalToRemote(local, remote, Process::identifier()); } void MessagePortChannelProviderImpl::messagePortDisentangled(const MessagePortIdentifier& local) diff --git a/src/bun.js/bindings/webcore/MessagePortChannelProviderImpl.h b/src/bun.js/bindings/webcore/MessagePortChannelProviderImpl.h index 17429b0d5e6ffb..fd029672c84616 100644 --- a/src/bun.js/bindings/webcore/MessagePortChannelProviderImpl.h +++ b/src/bun.js/bindings/webcore/MessagePortChannelProviderImpl.h @@ -25,6 +25,7 @@ #pragma once +#include #include "MessagePortChannelProvider.h" #include "MessagePortChannelRegistry.h" #include "MessageWithMessagePorts.h" diff --git a/src/bun.js/bindings/webcore/MessagePortChannelRegistry.cpp b/src/bun.js/bindings/webcore/MessagePortChannelRegistry.cpp index a92262f6301717..e4e9b82bd5f9a5 100644 --- a/src/bun.js/bindings/webcore/MessagePortChannelRegistry.cpp +++ b/src/bun.js/bindings/webcore/MessagePortChannelRegistry.cpp @@ -24,6 +24,8 @@ */ #include "config.h" + +#include #include "MessagePortChannelRegistry.h" // #include "Logging.h" @@ -35,6 +37,8 @@ namespace WebCore { +WTF_MAKE_ISO_ALLOCATED_IMPL(MessagePortChannelRegistry); + MessagePortChannelRegistry::MessagePortChannelRegistry() = default; MessagePortChannelRegistry::~MessagePortChannelRegistry() @@ -54,15 +58,11 @@ void MessagePortChannelRegistry::messagePortChannelCreated(MessagePortChannel& c { // ASSERT(isMainThread()); - auto result = m_openChannels.ensure(channel.port1(), [channel = &channel] { - return channel; - }); - ASSERT(result.isNewEntry); + auto result = m_openChannels.add(channel.port1(), channel); + ASSERT_UNUSED(result, result.isNewEntry); - result = m_openChannels.ensure(channel.port2(), [channel = &channel] { - return channel; - }); - ASSERT(result.isNewEntry); + result = m_openChannels.add(channel.port2(), channel); + ASSERT_UNUSED(result, result.isNewEntry); } void MessagePortChannelRegistry::messagePortChannelDestroyed(MessagePortChannel& channel) @@ -83,7 +83,7 @@ void MessagePortChannelRegistry::didEntangleLocalToRemote(const MessagePortIdent // ASSERT(isMainThread()); // The channel might be gone if the remote side was closed. - auto* channel = m_openChannels.get(local); + RefPtr channel = m_openChannels.get(local); if (!channel) return; @@ -97,11 +97,8 @@ void MessagePortChannelRegistry::didDisentangleMessagePort(const MessagePortIden // ASSERT(isMainThread()); // The channel might be gone if the remote side was closed. - auto* channel = m_openChannels.get(port); - if (!channel) - return; - - channel->disentanglePort(port); + if (RefPtr channel = m_openChannels.get(port)) + channel->disentanglePort(port); } void MessagePortChannelRegistry::didCloseMessagePort(const MessagePortIdentifier& port) @@ -110,7 +107,7 @@ void MessagePortChannelRegistry::didCloseMessagePort(const MessagePortIdentifier // LOG(MessagePorts, "Registry: MessagePort %s closed in registry", port.logString().utf8().data()); - auto* channel = m_openChannels.get(port); + RefPtr channel = m_openChannels.get(port); if (!channel) return; @@ -132,7 +129,7 @@ bool MessagePortChannelRegistry::didPostMessageToRemote(MessageWithMessagePorts& // LOG(MessagePorts, "Registry: Posting message to MessagePort %s in registry", remoteTarget.logString().utf8().data()); // The channel might be gone if the remote side was closed. - auto* channel = m_openChannels.get(remoteTarget); + RefPtr channel = m_openChannels.get(remoteTarget); if (!channel) { // LOG(MessagePorts, "Registry: Could not find MessagePortChannel for port %s; It was probably closed. Message will be dropped.", remoteTarget.logString().utf8().data()); return false; @@ -145,10 +142,8 @@ void MessagePortChannelRegistry::takeAllMessagesForPort(const MessagePortIdentif { // ASSERT(isMainThread()); - // LOG(MessagePorts, "Registry: Taking all messages for MessagePort %s", port.logString().utf8().data()); - // The channel might be gone if the remote side was closed. - auto* channel = m_openChannels.get(port); + RefPtr channel = m_openChannels.get(port); if (!channel) { callback({}, [] {}); return; diff --git a/src/bun.js/bindings/webcore/MessagePortChannelRegistry.h b/src/bun.js/bindings/webcore/MessagePortChannelRegistry.h index 43880ea913e4f5..1bbf560339f4fd 100644 --- a/src/bun.js/bindings/webcore/MessagePortChannelRegistry.h +++ b/src/bun.js/bindings/webcore/MessagePortChannelRegistry.h @@ -30,10 +30,15 @@ #include "MessagePortIdentifier.h" #include "ProcessIdentifier.h" #include +#include +#include namespace WebCore { -class MessagePortChannelRegistry { +class MessagePortChannelRegistry final : public CanMakeWeakPtr, public CanMakeCheckedPtr { + WTF_MAKE_ISO_ALLOCATED(MessagePortChannelRegistry); + WTF_OVERRIDE_DELETE_FOR_CHECKED_PTR(MessagePortChannelRegistry); + public: WEBCORE_EXPORT MessagePortChannelRegistry(); @@ -53,7 +58,7 @@ class MessagePortChannelRegistry { WEBCORE_EXPORT void messagePortChannelDestroyed(MessagePortChannel&); private: - HashMap m_openChannels; + UncheckedKeyHashMap> m_openChannels; }; } // namespace WebCore diff --git a/src/bun.js/bindings/webcore/Worker.cpp b/src/bun.js/bindings/webcore/Worker.cpp index a9f8fdae5d2c07..3b5d322b0bf64e 100644 --- a/src/bun.js/bindings/webcore/Worker.cpp +++ b/src/bun.js/bindings/webcore/Worker.cpp @@ -418,18 +418,26 @@ void Worker::forEachWorker(const Functionref(); worker->dispatchExit(exitCode); + worker->deref(); if (globalObject) { JSC::VM& vm = globalObject->vm(); vm.setHasTerminationRequest(); - // clang-tidy is smart enough to realize that deref() leads to freeing - // but it's not smart enough to realize that `hasOneRef()` ensures its safety - while (!vm.hasOneRef()) // NOLINT - vm.derefSuppressingSaferCPPChecking(); // NOLINT + { + globalObject->esmRegistryMap()->clear(globalObject); + globalObject->requireMap()->clear(globalObject); + vm.deleteAllCode(JSC::DeleteAllCodeEffort::PreventCollectionAndDeleteAllCode); + gcUnprotect(globalObject); + globalObject = nullptr; + } + + vm.heap.collectNow(JSC::Sync, JSC::CollectionScope::Full); vm.derefSuppressingSaferCPPChecking(); // NOLINT + vm.derefSuppressingSaferCPPChecking(); // NOLINT } } extern "C" void WebWorker__dispatchOnline(Worker* worker, Zig::GlobalObject* globalObject) diff --git a/src/bun.js/javascript.zig b/src/bun.js/javascript.zig index b56003968a8f8e..020d11f5b49a2f 100644 --- a/src/bun.js/javascript.zig +++ b/src/bun.js/javascript.zig @@ -1953,7 +1953,7 @@ pub const VirtualMachine = struct { } vm.global = ZigGlobalObject.create( vm.console, - -1, + if (opts.is_main_thread) -1 else std.math.maxInt(i32), false, false, null, @@ -2064,7 +2064,7 @@ pub const VirtualMachine = struct { vm.global = ZigGlobalObject.create( vm.console, - -1, + if (opts.is_main_thread) -1 else std.math.maxInt(i32), opts.smol, opts.eval, null, diff --git a/src/bun.js/test/jest.zig b/src/bun.js/test/jest.zig index 4be73143aa7ef1..ab6c6163ebd6eb 100644 --- a/src/bun.js/test/jest.zig +++ b/src/bun.js/test/jest.zig @@ -1931,7 +1931,7 @@ fn formatLabel(globalThis: *JSGlobalObject, label: string, function_args: []JSVa .quote_strings = true, }; const value_fmt = current_arg.toFmt(&formatter); - const test_index_str = std.fmt.allocPrint(allocator, "{any}", .{value_fmt}) catch bun.outOfMemory(); + const test_index_str = std.fmt.allocPrint(allocator, "{}", .{value_fmt}) catch bun.outOfMemory(); defer allocator.free(test_index_str); list.appendSlice(allocator, test_index_str) catch bun.outOfMemory(); idx += 1; diff --git a/src/js_ast.zig b/src/js_ast.zig index 6b4fd31d3283d4..04d6466e7b7039 100644 --- a/src/js_ast.zig +++ b/src/js_ast.zig @@ -8090,6 +8090,7 @@ pub const Macro = struct { .allocator = default_allocator, .args = resolver.opts.transform_options, .log = log, + .is_main_thread = false, .env_loader = env, }); diff --git a/test/js/node/worker_threads/worker_threads.test.ts b/test/js/node/worker_threads/worker_threads.test.ts index 6ed8af8acee368..38857a83fda884 100644 --- a/test/js/node/worker_threads/worker_threads.test.ts +++ b/test/js/node/worker_threads/worker_threads.test.ts @@ -151,12 +151,12 @@ test("receiveMessageOnPort works across threads", async () => { let sharedBufferView = new Int32Array(sharedBuffer); let msg = { sharedBuffer }; worker.postMessage(msg); - expect(Atomics.wait(sharedBufferView, 0, 0)).toBe("ok"); + expect(await Atomics.waitAsync(sharedBufferView, 0, 0).value).toBe("ok"); const message = receiveMessageOnPort(port1); expect(message).toBeDefined(); expect(message!.message).toBe("done!"); await worker.terminate(); -}); +}, 9999999); test("receiveMessageOnPort works as FIFO", () => { const { port1, port2 } = new MessageChannel(); @@ -188,7 +188,7 @@ test("receiveMessageOnPort works as FIFO", () => { receiveMessageOnPort(value); }).toThrow(); } -}); +}, 9999999); test("you can override globalThis.postMessage", async () => { const worker = new Worker(new URL("./worker-override-postMessage.js", import.meta.url).href); diff --git a/test/js/web/broadcastchannel/broadcast-channel.test.ts b/test/js/web/broadcastchannel/broadcast-channel.test.ts index 918bbd36b3d504..cd17faf5a01ee6 100644 --- a/test/js/web/broadcastchannel/broadcast-channel.test.ts +++ b/test/js/web/broadcastchannel/broadcast-channel.test.ts @@ -178,20 +178,38 @@ test("Closing a channel in onmessage prevents already queued tasks from firing o c1.postMessage("done"); }); -test("broadcast channel used with workers", done => { - var bc = new BroadcastChannel("hello test"); - var count = 0; - var workersCount = 100; - bc.onmessage = (e: MessageEvent) => { - expect(e).toBeInstanceOf(MessageEvent); - expect(e.target).toBe(bc); - expect(e.data).toBe("hello from worker"); - if (++count == workersCount) { - bc.close(); - done(); +test("broadcast channel used with workers", async () => { + const batchSize = 1; + for (let total = 0; total < 100; total++) { + let bc = new BroadcastChannel("hello test"); + + let promises: Promise[] = []; + let resolveFns = []; + + for (var i = 0; i < batchSize; i++) { + const { promise, resolve } = Promise.withResolvers(); + promises.push(promise); + resolveFns.push(resolve); } - }; - for (var i = 0; i < workersCount; i++) { - new Worker(new URL("./broadcast-channel-worker.ts", import.meta.url).href); + bc.onmessage = (e: MessageEvent) => { + expect(e).toBeInstanceOf(MessageEvent); + expect(e.target).toBe(bc); + expect(e.data).toBe("hello from worker"); + const resolve = resolveFns.shift(); + if (resolve) { + resolve(); + } else { + console.warn("resolve fn not found"); + } + console.count("resolve fn called"); + }; + + for (let i = 0; i < batchSize; i++) { + new Worker(new URL("./broadcast-channel-worker.ts", import.meta.url).href); + } + + await Promise.all(promises); + bc.close(); + console.count("Batch complete"); } -}); +}, 99999);