Race condition in destroy method
- Dominant language
- JavaScript
- Stars
- 7.8k
- Forks
- 983
- PR merge metrics
- No merged PRs in 30d
Description
**What version of this package are you using?**
Version: Commit d1c0ebe
Platform: Windows 10, Chrome Version 94.0.4606.81 (Official Build) (64-bit)
**Problem:**
peer.destroy() method has potential race condition
**How (my interpretation)**
It unhooks `peer._channel.onclose`, to avoid infinite recursion (since closing data channel will invoke `peer.destroy()` method)
Then calls `_channel.close()` and `_pc.close()` sequentially.
But order of execution is not guaranteed to be the same as they were called
Occasionally peer connection is closed before data channel is closed, causing remote peer to throw "RTCError Transport channel closed".
**Expected:**
When peer.destroy() method is called, remote peer should not experience any error.
**Work around:**
call `peer._channel.close()` to destroy peer
**Writing pull request:**
I am reluctant to write a pull request since I do not have full picture of the library and I may introduce more bugs with my own changes.
I will only write pull request if I am sure it is bug free.
My intent is to submit this issue for bug confirmation.
**Reproducible code:**
```
// Run it few more times, error should occur randomly
peerA = new Peer({ initiator: true });
peerB = new Peer({ initiator: false });
peerA.on('signal', s => peerB.signal(s));
peerB.on('signal', s => peerA.signal(s));
peerA.on("error", console.warn);
peerB.on("error", console.warn);
setTimeout(peerA.destroy.bind(peerA), 100);
// Work around
// setTimeout(peerA._channel.close.bind(peerA._channel), 100)
```
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.