Skip to content

fix: don't deadlock the TUI against an in-flight import - #2

Open
plbarrio wants to merge 1 commit into
subhashraveendran:mainfrom
plbarrio:fix/import-deadlock
Open

plbarrio wants to merge 1 commit into
subhashraveendran:mainfrom
plbarrio:fix/import-deadlock

Conversation

@plbarrio

Copy link
Copy Markdown

Progress events were sent on a blocking channel from inside the PTP data phase, i.e. with the client transaction mutex held. Once the 32-slot event channel filled -- roughly five files in, at ~8 events per 23 MB NEF -- the importer blocked there while still holding mu, and the renderer blocked in View() -> Connected() waiting for that same mu. Neither side could proceed, so the whole TUI froze, keyboard included.

Two changes, either of which breaks the cycle:

  • Progress sends are now non-blocking (trySend). Progress is a lossy signal by nature: the next tick carries the same cumulative totals. The lifecycle events the UI must not miss (FileStarted/FileDone/FileSkipped/AllDone/ Error) are still sent normally, and none of those are emitted under mu.

  • Connected() reads an atomic mirror of cmdConn instead of taking mu, so the render path can never block behind an in-flight transfer.

Verified on a Nikon D5300 over Wi-Fi: 20 files / 320 MB imported back to back with no stall, longest gap between files 11s. Before, imports never got past 6 files without freezing.

Fixes #1

Progress events were sent on a blocking channel from inside the PTP data
phase, i.e. with the client transaction mutex held. Once the 32-slot event
channel filled -- roughly five files in, at ~8 events per 23 MB NEF -- the
importer blocked there while still holding mu, and the renderer blocked in
View() -> Connected() waiting for that same mu. Neither side could proceed,
so the whole TUI froze, keyboard included.

Two changes, either of which breaks the cycle:

- Progress sends are now non-blocking (trySend). Progress is a lossy signal
  by nature: the next tick carries the same cumulative totals. The lifecycle
  events the UI must not miss (FileStarted/FileDone/FileSkipped/AllDone/
  Error) are still sent normally, and none of those are emitted under mu.

- Connected() reads an atomic mirror of cmdConn instead of taking mu, so the
  render path can never block behind an in-flight transfer.

Verified on a Nikon D5300 over Wi-Fi: 20 files / 320 MB imported back to back
with no stall, longest gap between files 11s. Before, imports never got past
6 files without freezing.
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.

Import deadlocks after ~5 files (D5300): progress send blocks while holding the PTP mutex

1 participant