Skip to content
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

Reader reads free memory #5

Open
geigerzaehler opened this issue Jul 1, 2020 · 3 comments
Open

Reader reads free memory #5

geigerzaehler opened this issue Jul 1, 2020 · 3 comments

Comments

@geigerzaehler
Copy link
Contributor

It is possible for the reader to read freed memory.

The following example will panic at the assertion. The reader will read from the memory location previously occupied by write_buf which is now occupied by read_buf.

let (mut writer, mut reader) = pipe();
let mut write_buf = vec![0u8; 1024];
let _ = futures::poll!(writer.write_all(&mut write_buf));
drop(write_buf);

// Fill the space previously used by `write_buf`
let write_buf_overwrite = vec![1u8; 1024];
let mut read_buf = vec![0u8; 1024];
reader.read_exact(&mut read_buf).await.unwrap();
assert_eq!(read_buf, vec![0u8; 1024]);
drop(write_buf_overwrite);

To address this I’d suggest to copy the the write buffer into state.data instead of using *const u8.

@rousan
Copy link
Member

rousan commented Jul 1, 2020

We are using *const u8 to gain performance, if we have copied every buffer in-between, there will be a lot of memory allocation and de-allocation.

The internal architecture is such that, until the reader reads the chunk data, the writer's write method will block (asynchronously off-course).

let (mut writer, mut reader) = pipe();
let mut write_buf = vec![0u8; 1024];
let _ = futures::poll!(writer.write_all(&mut write_buf)); //---> This statement should block until the reader reads the data.
drop(write_buf);

@rousan
Copy link
Member

rousan commented Jul 1, 2020

I will check this once.

geigerzaehler pushed a commit to geigerzaehler/async-pipe-rs that referenced this issue Jul 2, 2020
This change fixes panics (routerify#6) and unsafe code (routerify#5). This comes at the
cost of an additional copy of the data send through the pipe and having
a buffer in the state.

Moreover all unsafe code is removed and the need for a custom `Drop`
implementation which makes the code overall easier.

We also add tests.
geigerzaehler pushed a commit to geigerzaehler/async-pipe-rs that referenced this issue Jul 2, 2020
This change fixes panics (routerify#6) and unsafe code (routerify#5). This comes at the
cost of an additional copy of the data send through the pipe and having
a buffer in the state.

Moreover all unsafe code is removed and the need for a custom `Drop`
implementation which makes the code overall easier.

We also add tests.
ttiurani pushed a commit to ttiurani/async-pipe-rs that referenced this issue Oct 25, 2020
This change fixes panics (routerify#6) and unsafe code (routerify#5). This comes at the
cost of an additional copy of the data send through the pipe and having
a buffer in the state.

Moreover all unsafe code is removed and the need for a custom `Drop`
implementation which makes the code overall easier.

We also add tests.
@seanpianka
Copy link
Member

The latest version of the AsyncRead trait now uses ReadBuf to handle potentially uninitialized memory. Here's the issue with the proposal and background.

If we can prioritize #8, we can make swifter headway on this issue. 🙂

geigerzaehler pushed a commit to geigerzaehler/async-pipe-rs that referenced this issue Jul 26, 2021
This change fixes panics (routerify#6) and unsafe code (routerify#5). This comes at the
cost of an additional copy of the data send through the pipe and having
a buffer in the state. All unsafe code is removed and the need for a
custom `Drop` implementation which makes the code overall easier.

We also provide an implementation for traits from `futures` which is
behind a feature flag.

We also add tests.
geigerzaehler pushed a commit to geigerzaehler/async-pipe-rs that referenced this issue Jul 28, 2021
This change fixes panics (routerify#6) and unsafe code (routerify#5). This comes at the
cost of an additional copy of the data send through the pipe and having
a buffer in the state. All unsafe code is removed and the need for a
custom `Drop` implementation which makes the code overall easier.

We also provide an implementation for traits from `futures` which is
behind a feature flag.

We also add tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants