-
Notifications
You must be signed in to change notification settings - Fork 538
fix(realtime): terminate web worker on disconnect to prevent memory leak #1907
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
fix(realtime): terminate web worker on disconnect to prevent memory leak #1907
Conversation
Web Workers were created in _startWorkerHeartbeat() but never terminated in _teardownConnection(), causing memory accumulation with each disconnect/reconnect cycle. This fix adds a _terminateWorker() private method that properly terminates the Web Worker and clears the reference, then calls it during connection teardown. Fixes supabase#1902
|
Hi @filipecabaco, I wanted to ask regarding the Label Issues and PRs check. I am not completely sure why is it failing. Is it something i need to fix from my side? Please let me know. Thank you 🙏 |
|
@tanmaysharma2001 don't worry about this. I think sometimes it fails when you do not accept edits from maintainers, but don't worry about it. maybe i will remove this workflow in the future, if it fails consistently. |
mandarini
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great fix! This correctly addresses the memory leak by terminating workers on disconnect.
One related issue: the error handler in _startWorkerHeartbeat() has the same problem. If a worker errors out, workerRef isn't cleared, so on reconnect no new worker gets created. Could you update the error handler to use your new method?
this.workerRef.onerror = (error) => {
this.log('worker', 'worker error', (error as ErrorEvent).message)
this._terminateWorker()
}
…eat function Previously, the worker error handler in _startWorkerHeartbeat() directly calledworkerRef.terminate() without clearing the workerRef reference. This caused amemory leak on worker errors because on reconnection, the guard check for workerRef would prevent creating a new worker, leaving the connection withouta heartbeat mechanism.Now uses the centralized _terminateWorker() method which both terminates theworker and clears the reference, ensuring proper cleanup and allowing newworkers to be created on reconnection.This complements the fix in _teardownConnection() to fully prevent workermemory leaks across all disconnect/error scenarios.
|
Hi @mandarini, i have updated the said changes. Can you have a look please? |
🔍 Description
This PR fixes a memory leak in RealtimeClient when using the
worker: trueoption. Web Workers were being created during connection but never properly cleaned up during disconnection, leading to memory accumulation over time.What changed?
Added
_terminateWorker()private methodworkerRefreference for garbage collectionUpdated
_teardownConnection()method_terminateWorker()during connection teardownAdded test coverage
workerRefis cleared after terminationWhy was this change needed?
When using RealtimeClient with
worker: true, each disconnect/reconnect cycle would create a new Web Worker without terminating the previous one. This caused:The fix ensures proper worker lifecycle management by terminating workers during connection teardown.
Closes #1902
📸 Screenshots/Examples
Before: Workers were created but never terminated
🔄 Breaking changes
📋 Checklist
<type>(<scope>): <description>npx nx formatto ensure consistent code formatting📝 Additional notes