-
Notifications
You must be signed in to change notification settings - Fork 628
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
Create copy_buf_abortable, which enables to stop copying in the middle. #2507
Conversation
MotivationIf we read file with futures_io and make it abortable, we have to
Recap of recent discussion in #2497@taiki-e pointed out that
I also think that for more convenience, something like AsyncRead / AsyncWrite 'd abortable is necessary. I agree to you at this point. (also mentioned in #2156) Pros and consI think there exist 2 different options,
For first one, we perform copy with futures_io and make it abortable, we have to
But the second one,
So I think the second one has a thicker struct that perhaps lowers performance, and the first one is less complicated for programmers when it comes to copying one to another. Thus, I believe that implement abortable copy into a separate mod, like this current PR, is better off. |
@taiki-e After that, I found an opinion that we should implement the ability to stop in the middle for all IOs, not just the copy_buf(). This topic is very relevant to this pull request, but the original Issue is already outdated. Therefore, I created a new issue in #2509, and then suggested another solution. Please check it out. |
@taiki-e Sorry for the late response. I saw through your review and added another change in this PR. |
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.
Thanks, looks good to me aside from a nit.
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [futures](https://github.com/rust-lang/futures-rs) | dependencies | patch | `0.3.21` -> `0.3.23` | --- ### Release Notes <details> <summary>rust-lang/futures-rs</summary> ### [`v0.3.23`](https://github.com/rust-lang/futures-rs/blob/HEAD/CHANGELOG.md#​0323---2022-08-14) [Compare Source](rust-lang/futures-rs@0.3.22...0.3.23) - Work around MSRV increase due to a cargo bug. ### [`v0.3.22`](https://github.com/rust-lang/futures-rs/blob/HEAD/CHANGELOG.md#​0322---2022-08-14) [Compare Source](rust-lang/futures-rs@0.3.21...0.3.22) - Fix `Sync` impl of `BiLockGuard` ([#​2570](rust-lang/futures-rs#2570)) - Fix partial iteration in `FuturesUnordered` ([#​2574](rust-lang/futures-rs#2574)) - Fix false detection of inner panics in `Shared` ([#​2576](rust-lang/futures-rs#2576)) - Add `Mutex::lock_owned` and `Mutex::try_lock_owned` ([#​2571](rust-lang/futures-rs#2571)) - Add `io::copy_buf_abortable` ([#​2507](rust-lang/futures-rs#2507)) - Remove `Unpin` bound from `TryStreamExt::into_async_read` ([#​2599](rust-lang/futures-rs#2599)) - Make `run_until_stalled` handle self-waking futures ([#​2593](rust-lang/futures-rs#2593)) - Use `FuturesOrdered` in `try_join_all` ([#​2556](rust-lang/futures-rs#2556)) - Fix orderings in `LocalPool` waker ([#​2608](rust-lang/futures-rs#2608)) - Fix `stream::Chunk` adapters size hints ([#​2611](rust-lang/futures-rs#2611)) - Add `push_front` and `push_back` to `FuturesOrdered` ([#​2591](rust-lang/futures-rs#2591)) - Deprecate `FuturesOrdered::push` in favor of `FuturesOrdered::push_back` ([#​2591](rust-lang/futures-rs#2591)) - Performance improvements ([#​2583](rust-lang/futures-rs#2583), [#​2626](rust-lang/futures-rs#2626)) - Documentation improvements ([#​2579](rust-lang/futures-rs#2579), [#​2604](rust-lang/futures-rs#2604), [#​2613](rust-lang/futures-rs#2613)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzMi4xNTYuMSIsInVwZGF0ZWRJblZlciI6IjMyLjE1Ni4xIn0=--> Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1507 Reviewed-by: crapStone <crapstone@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
See #2497 .
Summary
When transferring data from one place to another using the copy_buf function, you may want to abort the operation in the middle of the transfer, for example, by SIGINT. Since the transfer is divided into small increments by internal buffers and probably must not be done at a time, the interruption logic can be implemented by checking for the aborting request flag at every poll_fill_buf().
This patch has