webtorrent icon indicating copy to clipboard operation
webtorrent copied to clipboard

`add` and `remove` are not async safe

Open uriva opened this issue 10 months ago • 6 comments

What version of this package are you using? ^2.1.27

What operating system, Node.js, and npm version?

ubuntu 23.4 v19.0.0 8.19.2

What happened? It's possible to get add and remove to throw if calling them in parallel, even with different hash ids.

What did you expect to happen?

remove should not throw if it can't find the torrent.

add should not throw if the torrent is already added.

Are you willing to submit a pull request to fix this bug? yes

uriva avatar Oct 17 '23 13:10 uriva

those are async functions, you simply need to run

await client.add()

and

await client.remove()

ThaUnknown avatar Oct 17 '23 13:10 ThaUnknown

consider the case where add is called from different places in the code. If there is an sync context switch inside add, then we can be inside it twice.

await is not equivalent to a lock.

uriva avatar Oct 17 '23 14:10 uriva

fair

ThaUnknown avatar Oct 17 '23 14:10 ThaUnknown

while we're at it, these functions should probably not have a cb parameter and be async at the same time.

uriva avatar Oct 17 '23 14:10 uriva

they should be, but not as a part of this issue

this was planned for ages: https://github.com/orgs/webtorrent/projects/1#card-60702582

they became async because other issues were being blocked by it, and using .then would be useless if it was later re-written to await anyways, and dropping callback based calls for webtorrent, means doing it inside close to 20 packages, which requries a lot of testing and rewriting

ThaUnknown avatar Oct 17 '23 14:10 ThaUnknown

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

github-actions[bot] avatar Dec 17 '23 12:12 github-actions[bot]