-
Notifications
You must be signed in to change notification settings - Fork 353
Closed
Labels
bugSomething isn't workingSomething isn't working
Milestone
Description
Describe the bug
It looks like there is a memory leak of closeOnBeforeunload
listener in case it fails to connect to WebSocket and tries to connect again
Sorry for not sticking with the issue template but I'm going to fix it if you think that it is indeed a memory leak
So why the memory leak? I've noticed while debugging my application that the beforeunload
listener keeps adding up by socket io client
As we see here:
engine.io-client/lib/socket.ts
Lines 355 to 366 in dfee8de
addEventListener( | |
"beforeunload", | |
() => { | |
if (this.transport) { | |
// silently close the transport | |
this.transport.removeAllListeners(); | |
this.transport.close(); | |
} | |
}, | |
false | |
); | |
} |
But, in the onClose
we don't remove that listener:
engine.io-client/lib/socket.ts
Lines 894 to 932 in dfee8de
private onClose(reason: string, description?: CloseDetails | Error) { | |
if ( | |
"opening" === this.readyState || | |
"open" === this.readyState || | |
"closing" === this.readyState | |
) { | |
debug('socket close with reason: "%s"', reason); | |
// clear timers | |
this.clearTimeoutFn(this.pingTimeoutTimer); | |
// stop event from firing again for transport | |
this.transport.removeAllListeners("close"); | |
// ensure transport won't stay open | |
this.transport.close(); | |
// ignore further transport communication | |
this.transport.removeAllListeners(); | |
if (typeof removeEventListener === "function") { | |
removeEventListener("offline", this.offlineEventListener, false); | |
} | |
// set ready state | |
this.readyState = "closed"; | |
// clear session id | |
this.id = null; | |
// emit close event | |
this.emitReserved("close", reason, description); | |
// clean buffers after, so users can still | |
// grab the buffers on `close` event | |
this.writeBuffer = []; | |
this.prevBufferLen = 0; | |
} | |
} |
just making sure that I'm correct and if so I'll create a PR to fix that...
Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't working