Suggestion to remove Clone bound from reader
- Dominant language
- Rust
- Stars
- 166
- Forks
- 49
- PR merge metrics
- No merged PRs in 30d
Description
AIUI, the clone bound is required in the case where multiple requests are accepted over the same AsyncRead and to avoid having a lifetime bound on the `Request` type.
I think there a couple of problems with this:
- Many streams are not cloneable, which can make integrations difficult, eg. #109.
- Even for a TcpStream, it doesn't *really* make sense to clone it, as concurrent acess will just corrupt the state.
- There's nothing that inherently prevents accidental concurrent access: the crate just relies on the caller doing the right thing.
I think a good workaround would be to use a `oneshot::channel`. Specifically, create an `AsyncRead` type like:
```rust
struct LeasedReader {
return_to_sender: oneshot::Sender,
inner: Option,
}
```
This would have a `finish()` method on that returns the inner reader to the sender. The advantage of this is that the sender loses access to the stream whilst is it on lease, the stream does not require any internal synchronization, and if there is a problem whilst reading from the stream (such as a panic or error) it will not be returned to the sender. The sender will just see the oneshot channel be canceled, and can handle that case cleanly.
The downside to this is that it would require changing the signature of the `accept` function: I see two possibilities. Either the sender side can get its own wrapper that implements `AsyncRead` and hides the fact that the inner reader may be leased out. Alternatively, the `accept` function can return an additional `impl Future` (the oneshot::Receiver) for the caller to recover the reader.
Contributor guide
Assessment
This issue has not been assessed yet.