-
Notifications
You must be signed in to change notification settings - Fork 411
Drop dep tokio
's io-util
feat as it broke MSRV and isn't useful
#2537
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Drop dep tokio
's io-util
feat as it broke MSRV and isn't useful
#2537
Conversation
Oops, also had to test-only pin |
CI still unhappy 😭
|
1046f33
to
97b6c6f
Compare
In any case, IMO we should still land this while we work through |
Feel free to squash, LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
We use `tokio`'s `io-util` feature to provide the `Async{Read,Write}Ext` traits, which allow us to simply launch a read future or `poll_write` directly as well as `split` the `TcpStream` into a read/write half. However, these traits aren't actually doing much for us - they are really just wrapping the `readable` future (which we can trivially use ourselves) and `poll_write` isn't doing anything for us that `poll_write_ready` can't. Similarly, the split logic is actually just `Arc`ing the `TcpStream` and busy-waiting when an operation is busy to prevent concurrent reads/writes. However, there's no reason to prevent concurrent access at the stream level - we aren't ever concurrently writing or reading (though we may concurrently read and write, which is fine). Worse, the `io-util` feature broke MSRV (though they're likely to fix this upstream) and carries two additional dependencies (only one on the latest upstream tokio). Thus, we simply drop the dependency here. Fixes lightningdevkit#2527.
97b6c6f
to
afc5a02
Compare
Squashed with the one extra space removed:
|
We use
tokio
'sio-util
feature to provide theAsync{Read,Write}Ext
traits, which allow us to simply launch a read future orpoll_write
directly as well assplit
theTcpStream
into a read/write half. However, these traits aren't actually doing much for us - they are really just wrapping thereadable
future (which we can trivially use ourselves) andpoll_write
isn't doing anything for us thatpoll_write_ready
can't.Similarly, the split logic is actually just
Arc
ing theTcpStream
and busy-waiting when an operation is busy to prevent concurrent reads/writes. However, there's no reason to prevent concurrent access at the stream level - we aren't ever concurrently writing or reading (though we may concurrently read and write, which is fine).Worse, the
io-util
feature broke MSRV (though they're likely to fix this upstream) and carries two additional dependencies (only one on the latest upstream tokio).Thus, we simply drop the dependency here.
Fixes #2527.