staltz / callbag-share

👜 Callbag operator that broadcasts a single source to multiple sinks
MIT License
22 stars 3 forks source link

Potential race condition where a sink might receive data before handshake #13

Open teohhanhui opened 2 years ago

teohhanhui commented 2 years ago

Given this test:

https://github.com/staltz/callbag-share/blob/d96748edec631800ec5e606018f519ccaeb8f766/test.js#L93-L203

If sinkA sends a pull asynchronously but immediately(*), then it can happen that sinkB has sent its handshake to share, but has not yet received back the reciprocal handshake when it receives data from share.

(*)setTimeout has an intrinsic delay, so this isn't triggered here:

https://github.com/staltz/callbag-share/blob/d96748edec631800ec5e606018f519ccaeb8f766/test.js#L166

I'm encountering this intermittent failure in my Rust port: https://github.com/teohhanhui/callbag-rs/runs/4752918190?check_suite_focus=true#step:6:627

I've tried to rule this out as just a problem with how the test is written, or an implementation bug in the Rust port, but it does seem to be an actual bug in the share logic.

teohhanhui commented 2 years ago

Hmm... Never mind, I think I was mistaken. Judging from the timestamp of the log entries, something weird is going on there...

EDIT: https://users.rust-lang.org/t/help-wanted-debugging-failing-async-tests-in-an-open-source-project/69837/15

teohhanhui commented 2 years ago

Actually it might be a real bug:

https://github.com/teohhanhui/callbag-rs/runs/4919701692?check_suite_focus=true#step:6:1796

staltz commented 2 years ago

Thanks. Could you submit a PR that reproduces the bug in the tests? (No need to submit a fix for the test)