Skip to content

add context cancellation to disk windows#1943

Merged
shirou merged 2 commits intoshirou:masterfrom
StefanoBalzarottiNozomi:add-context-disk-windows
Nov 26, 2025
Merged

add context cancellation to disk windows#1943
shirou merged 2 commits intoshirou:masterfrom
StefanoBalzarottiNozomi:add-context-disk-windows

Conversation

@StefanoBalzarottiNozomi
Copy link
Copy Markdown
Contributor

I readded context cancellation. context is cancelled as soon as possible, but there are no leaking goroutines, if a system call blocks, code returns as soon as the system call returns.

@shirou
Copy link
Copy Markdown
Owner

shirou commented Nov 19, 2025

Sorry for late reponse. Great work on adding context cancellation! However, I noticed the context checking pattern is inconsistent across the loop implementations:

  • processLogicalDrives (line 195): uses select statement
  • processVolumeLoop (line 145): uses if ctx.Err() != nil
  • processMountsForVolume (line 178): uses if ctx.Err() != nil

For consistency and following Go idioms, I'd recommend using select for all loop-based context checks. The select statement is the more conventional pattern when checking context cancellation in loops. How about to use select statement in processVolumeLoop and processMountsForVolume ?

@StefanoBalzarottiNozomi
Copy link
Copy Markdown
Contributor Author

Sorry for late reponse. Great work on adding context cancellation! However, I noticed the context checking pattern is inconsistent across the loop implementations:

* `processLogicalDrives` (line 195): uses `select` statement

* `processVolumeLoop` (line 145): uses `if ctx.Err() != nil`

* `processMountsForVolume` (line 178): uses `if ctx.Err() != nil`

For consistency and following Go idioms, I'd recommend using select for all loop-based context checks. The select statement is the more conventional pattern when checking context cancellation in loops. How about to use select statement in processVolumeLoop and processMountsForVolume ?

done, now loops uses select with context

Copy link
Copy Markdown
Owner

@shirou shirou left a comment

Choose a reason for hiding this comment

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

Thank you for updating the PR. The context cancellation implementation looks good - it properly checks for cancellation at each loop iteration without leaking goroutines. LGTM!

@shirou shirou merged commit 93ca345 into shirou:master Nov 26, 2025
52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants