Simple-peer: Chrome m76 simple-peer doesn't destroy after disconnection (nor emits 'close')

[perkio]

this is due to a breaking change in chrome: https://bugs.chromium.org/p/chromium/issues/detail?id=982793

[paulalbertdev]

So, how do you advise to handle disconnections with simple-peer now ?

[paulalbertdev]

Since you've labelled this as a bug, I presume you're going need to write a workaround...
Thanks, and keep up the good work !

[t-mullen]

It seems checking for the broader condition pc.connectionState === 'failed' on a connectionstatechange event would work. We should probably destroy the peer if this happens anyways.

[pshah123]

Hey -- is there any update on this issue? I've been noticing this lately and it streams chrome is also doing something funky with the streams when making a second call.

Observed behaviour:

1. Peers call each other. Exchange video stream.
2. Peers close tab.
3. Peers call each other again. Exchange video stream.
4. Memory/CPU usage suddenly spikes on this call and the size of the memory usage significantly increases.
5. Repeat 5-6x and the memory usage is now 10-20x larger than before and most computers crash the tab. Behaviour persists until closing out Chrome completely.

By persisting the streams we were able to notice that the streams are never actually destroyed. Even if you close the tab and open it again multiple times, all the streams still exist and are in disconnected state. Moreover, the streams still hold data (i.e. video streams show the last received frame).

I'm wondering if there's a workaround or such we can use!

[pshah123]

Looking at the changed spec and the recommended steps there seems to be a few things that can be done:

• As @t-mullen and this comment suggested, a check on the connectionState property might still work here; since it seems more robust to use the iceConnectionState everywhere else, I've added this as a fallbackConnectionState over here and added the check in the original conditional for iceConnectionState == 'failed' over here.

• As recommended by another comment and apparently by the official spec, simply checking if the state has remained disconnected for a couple seconds seems to suffice in place of "failure." I don't think this is robust _at all_ and it seems likely to run into issues in the long run but I've added this alongside the broader check above as well, as a separate conditional here.

• Lastly the same comment also recommends just using Plan B to revert and get rid of unintended consequences; it seems that it's not very forward moving to just abandon the Unified Plan, so for now I've added this commented out in the Peer Config here. If needed this can be used by simply commenting out the line above, uncommenting this line, and another npm run build.

After using it in our live for a few trials we've noticed many of our prior problems mitigated, but only time will tell if this actually fixed the bug. I'll open a pull request once I can confirm that it worked! If anyone else wants to test the fork is here.

Edit: it seems to have worked in most situations, however there is a rare case where occasionally on Windows Chrome 76, refreshing the page does not seem to destroy the streams and they are kept in memory. After 10-11 refreshes, the buffer is ridiculously large and the CPU usage is too high. I haven't been able to reliably replicate this, but when it rarely happens, even a hard refresh is unable to destroy the connection. The only thing I can do here is close out the tab and open a new tab.

[t-mullen]

Options 1 seems to be the correct solution. We should be destroying the peer on the terminal connectionState == 'failed' irregardless of this issue.

Reverting to Plan B will almost certainly cause interop issues when using multiple media streams. You might have more severe issues with Safari since Plan B support there is very poor. This may have all been fixed (we don't test Plan B), but I doubt it.

I should have time to PR the connectionState check and write a few tests this weekend - sorry for the delay 👍