Skip to content
This repository has been archived by the owner on Jun 8, 2021. It is now read-only.

Implement an AsyncBufRead adapter for InputStream #296

Closed
sdroege opened this issue May 6, 2020 · 9 comments
Closed

Implement an AsyncBufRead adapter for InputStream #296

sdroege opened this issue May 6, 2020 · 9 comments

Comments

@sdroege
Copy link
Member

sdroege commented May 6, 2020

This would work by managing an internal buffer itself. The size should be configurable. On completion the AsyncBufRead would be ready and provide access to the filled part of the buffer.

@sdroege sdroege mentioned this issue May 6, 2020
40 tasks
@gdesmott
Copy link
Contributor

I can give this a shot. Can you please explain a bit how this should be implemented and the API you are looking for? I'm not yet familiar with the async traits.

@gdesmott
Copy link
Contributor

Shouldn't AsyncBufRead be implemented on PollableInputStream instead as it's the one implementing AsyncRead?

@sdroege
Copy link
Member Author

sdroege commented May 15, 2020

No, and that one also only has an adapter that you can get from it to have an AsyncRead. We can't implement it directly on our types :)

AsyncBufRead (unlike AsyncRead) can be implemented for the normal input streams. AsyncRead requires an API where the buffers are owned by the caller while AsyncBufRead allows the buffer to be owned by the callee. You could have an adapter that contains a vec and then hands out immutable slices to that vec whenever a read has finished (because g_input_stream_read() requires ownership of the buffer it is impossible to implement AsyncRead on it).

@gdesmott
Copy link
Contributor

it is impossible to implement AsyncRead on it

But AsyncBufRead has a trait bound on AsyncRead so the adapter will have to implement both traits. How will this work then?

@sdroege
Copy link
Member Author

sdroege commented May 15, 2020

Ah I missed that. You would implement AsyncRead indirectly then, with a copy from the internal buffer into the caller-provided buffer.

@gdesmott
Copy link
Contributor

Here is what I have so far for AsyncRead: gdesmott@be9f0ad

Problem is the test is calling poll_read() once which is returning Pending but then seems stuck.
Isn't the Poll from read_async_future supposed to wake up cx once its ready which would trigger a new poll from the executioner?

First time I'm actually writing futures code so I may have miss something. :)

@sdroege
Copy link
Member Author

sdroege commented May 18, 2020

Answered there in the commit :)

@gdesmott
Copy link
Contributor

poll_read() receives a Pin<&mut Self> but I'm not able to get a &mut InputStreamAsyncBufRead out of it as my type doesn't implement Unpin. Is that normal?

As a result, I'm trying to workaround this immutability by wrapping the state in a Refcell. I kinda works except when performing state switches as I need to deconstruct the state enum while it's borrowed, see gdesmott@84bd74d#diff-463b53a4eb9782d0f3c7dfb895572516R393

error[E0507]: cannot move out of dereference of `std::cell::RefMut<'_, input_stream::State>`
   --> src/input_stream.rs:393:15
    |
393 |         match *state {
    |               ^^^^^^ help: consider borrowing here: `&*state`
394 |             State::Waiting { buffer: buffer } => {
    |                                      ------
    |                                      |
    |                                      data moved here
    |                                      move occurs because `buffer` has type `std::vec::Vec<u8>`, which does not implement the `Copy` trait

Is there a better pattern to handle all this? Or should I just implement Unpin on InputStreamAsyncBufRead? I'm still not very clear on its exact semantic.

@sdroege
Copy link
Member Author

sdroege commented May 19, 2020

You might want to use the pin_project crate for this, or require Unpin. Various existing future implementations in the futures crate can probably be taken here as examples. There are IIRC a few that also have multiple states, for example and_then :)

gdesmott pushed a commit to gdesmott/gio that referenced this issue May 22, 2020
gdesmott pushed a commit to gdesmott/gio that referenced this issue May 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants