Skip to content

Asynchronous unmounting methods for BackgroundSession#688

Open
khanhtranngoccva wants to merge 1 commit into
cberner:masterfrom
khanhtranngoccva:async-unmount
Open

Asynchronous unmounting methods for BackgroundSession#688
khanhtranngoccva wants to merge 1 commit into
cberner:masterfrom
khanhtranngoccva:async-unmount

Conversation

@khanhtranngoccva

Copy link
Copy Markdown
Contributor

Two new methods are added:

  1. umount_no_join: Allows for unmounting a filesystem, then returning the session thread's join handle if it succeeds. This allows for batching join threads somewhere else (e.g. in a joining sink).
  2. umount_background: Similar to umount_no_join, but performs the unmount in a separate thread, and returns the session thread's join handle immediately. This allows for batching unmount threads, as well as giving an opportunity for session threads to join even if the unmounting process fails for arbitrary reasons.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f84367d879

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/session.rs

/// Unmount the filesystem from a separate thread and return the background thread for manual joining. This allows the main thread to continue doing useful work while reporting errors in the background, and for implementing structured concurrency by giving the filesystem threads an opportunity to be joined.
pub fn umount_background(mut self) -> BackgroundUnmount {
let unmount_thread = thread::spawn(move || {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Surface unmount thread spawn errors

In processes that have exhausted their thread quota or hit OS limits, std::thread::spawn panics on failure; unlike Session::spawn above, which uses thread::Builder::spawn(...)?, this makes the new cleanup path able to panic a daemon while it is trying to unmount. Please return an io::Result<BackgroundUnmount> or otherwise surface the spawn error so callers can handle this failure mode.

Useful? React with 👍 / 👎.

Comment thread src/session.rs

/// A set of relevant threads during an asynchronous unmounting process.
#[derive(Debug)]
pub struct BackgroundUnmount {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Re-export the background unmount type

Because this new public return type lives inside the private session module while src/lib.rs only re-exports BackgroundSession, downstream crates can receive the value by inference but cannot name it in their own function signatures, struct fields, or imports. Since umount_background is a public API intended for batching/structured concurrency, re-export BackgroundUnmount from the crate root alongside BackgroundSession.

Useful? React with 👍 / 👎.

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

Successfully merging this pull request may close these issues.

1 participant