supabase-js icon indicating copy to clipboard operation
supabase-js copied to clipboard

bug: BroadcastChannel event listener never removed causing memory leak

Open IdellHeaney opened this issue 1 month ago • 2 comments

Bug Report

Describe the bug

In GoTrueClient, a BroadcastChannel is created and an event listener is attached in the constructor, but there's no mechanism to remove the listener or close the channel. This causes memory leaks as the listener holds a reference to the client instance via closure.

Expected behavior

The class should provide a dispose() method to properly clean up:

Remove the BroadcastChannel event listener Close the BroadcastChannel Stop auto refresh timers Remove visibility change callback Clear state change emitters Note: The class already implements _removeVisibilityChangedCallback() for similar cleanup, but no equivalent exists for BroadcastChannel.

Impact

Memory leaks in SPAs where clients are created/destroyed Potential duplicate event handling with multiple client instances Resources held unnecessarily after client is no longer needed

IdellHeaney avatar Jan 14 '26 16:01 IdellHeaney

Thanks for flagging this @uwuchanis73783! 💚 You're right, the cleanup pattern exists for visibilityChangedCallback but was missed for BroadcastChannel.

Opening a PR to add a dispose() method that properly cleans up the channel.

7ttp avatar Jan 14 '26 16:01 7ttp

Thank you for raising this issue @IdellHeaney and thanks @7ttp for the proposed solution!

After internal discussion, we believe that adding an explicit dispose() method to clean up the BroadcastChannel listener in GoTrueClient doesn't provide enough real-world value and might actually create more confusion.

Here's why:

  • In practice, the GoTrueClient instance is meant to live as long as the web app/tab does. The expected usage is a single, long-lived client—not creating and destroying client instances repeatedly.
  • When the tab is closed (the typical end-of-life for a client), the browser automatically releases all resources, including the BroadcastChannel and its listeners—so manual cleanup isn't necessary.
  • Introducing a public API for disposal (dispose()) makes lifecycle management more complex, and could lead to unexpected issues if used incorrectly (e.g., a developer calls dispose() and the client stops working mid-session, resulting in confusing bugs).
  • If the application design ever truly needs short-lived/temporary client instances, implementing custom cleanup or using a different architecture is recommended.

While the leak is technically correct in theory, in practical terms there's no actual impact for typical usage. We prefer to keep the API surface minimal and focused on common scenarios.

We're therefore closing this issue for now. If we ever introduce more complex resource management (e.g., factories or pooled clients), we'll revisit cleanup mechanisms with clear usage patterns.

Thank you for your contribution and for helping us clarify our rationale! You're helping us make Supabase better! 💚

mandarini avatar Feb 04 '26 13:02 mandarini