ai-collab/bulk-image-organizer

Watch

Fork 0

[Bug]: App crashes after Confirm Delete / File Duplicate Bulk Actions send-to-trash on Windows (files deleted, no completion dialog) #170

New issue

Closed

opened 2026-08-21 01:18:51 +00:00 by Claude · 3 comments

Claude commented

2026-08-21 01:18:51 +00:00

Member

App version

Packaged Windows build (reported against a build around 0.6.1/v0.7.0-era).

Platform

Windows

Report (from user, in chat, not yet reproduced by the user on a second attempt)

in windows, when confirming the delete file bulk action the files were deleted but the application crashed before alerting the user or returning to the UI.

"Delete file bulk action" is the File Duplicate Bulk Actions dialog (_on_dupe_bulk_actions in ui/main_window.py) with "send to trash" selected — it skips the normal DeleteConfirmDialog and calls _start_confirm_delete(trash_paths, remove_empty_dirs=True) directly. The underlying DeleteWorker (core/delete_worker.py) ran to completion (files were actually sent to the recycle bin), but the app crashed before the finished signal's handler (_on_delete_finished) could show a summary or return control to the UI. No dialog, no apparent log output described.

Investigation (code-level, not yet confirmed against an actual Windows crash log)

This matches a failure shape this project has hit before: DR-027/DR-028 (issue #111) — a native crash inside a QThreadPool worker that bypasses Python's exception handling entirely (no traceback, no dialog), traced both times to a native/COM dependency being touched for the first time inside worker code.

DeleteWorker._run_delete() calls send2trash.send2trash() once per file, in a loop, entirely inside a QRunnable.run() executing on a QThreadPool worker thread. On Windows, send2trash auto-selects its backend (send2trash/win/__init__.py): if pywin32 is importable, it unconditionally uses send2trash/win/modern.py, which per-call does:

pythoncom.CoInitialize()
fileop = pythoncom.CoCreateInstance(shell.CLSID_FileOperation, ..., shell.IID_IFileOperation)
...
fileop.PerformOperations()
...
pythoncom.CoUninitialize()

i.e. it initializes a COM STA apartment on whatever OS thread QThreadPool handed it, creates a Shell IFileOperation COM object with an IFileOperationProgressSink callback, performs the operation, and tears the apartment down — repeated once per file. QThreadPool worker threads never pump a Windows message loop, which STA COM apartments and Explorer's shell-operation callbacks generally expect to have available. This is a well-known source of instability for COM shell operations invoked from bare worker threads, and unlike a normal Python exception (which DeleteWorker.run()'s except Exception already catches and reports via the error signal), a native/COM-level fault here would bypass that handling entirely — consistent with "crashed before alerting the user."

pywin32 is not a direct dependency of this project (pyproject.toml only lists send2trash>=1.8) — send2trash only requires it via its own optional [nativelib] extra, which we don't request — but nothing prevents it from being present transitively or in a given build environment, and send2trash's own fallback logic silently switches to the COM path whenever it's importable, with no app-level control over that choice today.

By contrast, send2trash/win/legacy.py (the fallback when pywin32 is absent) uses only ctypes + SHFileOperationW — no COM, no apartment/message-pump requirements, safe to call from an arbitrary thread, and any failure surfaces as a normal OSError/WindowsError that DeleteWorker's existing except OSError already handles correctly.

Proposed fix

Force DeleteWorker to always use send2trash.win.legacy.send2trash on sys.platform == "win32", instead of the package's own auto-detection, so the COM-based backend is never reachable from this app regardless of whether pywin32 happens to be present in a given build/environment. Non-Windows platforms are unaffected.

Acceptance criteria

DeleteWorker on Windows never invokes send2trash.win.modern (COM/IFileOperation) — always the ctypes/SHFileOperationW legacy backend.
macOS/Linux behavior unchanged.
A Decision Record documents the reasoning (this is unconfirmed against an actual Windows crash log/report, same as DR-027 was before its follow-up).
Regression test verifies the Windows import path resolves to the legacy backend.

Status

Not yet reproduced by the user on a retry; root cause identified via code inspection and precedent (DR-027/028), not confirmed via an actual crash log or dump from the user's machine. Proceeding with the fix per the same pattern DR-027 used (implement the well-evidenced fix, await confirmation). If this recurs after the fix ships, the next step is capturing %APPDATA%\bulk-image-organizer\logs and/or a nuitka-crash-report.xml from the crash for further diagnosis.

### App version Packaged Windows build (reported against a build around 0.6.1/v0.7.0-era). ### Platform Windows ### Report (from user, in chat, not yet reproduced by the user on a second attempt) > in windows, when confirming the delete file bulk action the files were deleted but the application crashed before alerting the user or returning to the UI. "Delete file bulk action" is the **File Duplicate Bulk Actions** dialog (`_on_dupe_bulk_actions` in `ui/main_window.py`) with "send to trash" selected — it skips the normal `DeleteConfirmDialog` and calls `_start_confirm_delete(trash_paths, remove_empty_dirs=True)` directly. The underlying `DeleteWorker` (`core/delete_worker.py`) ran to completion (files were actually sent to the recycle bin), but the app crashed before the `finished` signal's handler (`_on_delete_finished`) could show a summary or return control to the UI. No dialog, no apparent log output described. ### Investigation (code-level, not yet confirmed against an actual Windows crash log) This matches a failure shape this project has hit before: **DR-027/DR-028** (issue #111) — a native crash inside a `QThreadPool` worker that bypasses Python's exception handling entirely (no traceback, no dialog), traced both times to a native/COM dependency being touched for the first time inside worker code. `DeleteWorker._run_delete()` calls `send2trash.send2trash()` once per file, in a loop, entirely inside a `QRunnable.run()` executing on a `QThreadPool` worker thread. On Windows, `send2trash` auto-selects its backend (`send2trash/win/__init__.py`): if `pywin32` is importable, it unconditionally uses `send2trash/win/modern.py`, which per-call does: ```python pythoncom.CoInitialize() fileop = pythoncom.CoCreateInstance(shell.CLSID_FileOperation, ..., shell.IID_IFileOperation) ... fileop.PerformOperations() ... pythoncom.CoUninitialize() ``` i.e. it initializes a COM STA apartment on whatever OS thread `QThreadPool` handed it, creates a Shell `IFileOperation` COM object with an `IFileOperationProgressSink` callback, performs the operation, and tears the apartment down — repeated once per file. `QThreadPool` worker threads never pump a Windows message loop, which STA COM apartments and Explorer's shell-operation callbacks generally expect to have available. This is a well-known source of instability for COM shell operations invoked from bare worker threads, and unlike a normal Python exception (which `DeleteWorker.run()`'s `except Exception` already catches and reports via the `error` signal), a native/COM-level fault here would bypass that handling entirely — consistent with "crashed before alerting the user." `pywin32` is not a direct dependency of this project (`pyproject.toml` only lists `send2trash>=1.8`) — `send2trash` only requires it via its own optional `[nativelib]` extra, which we don't request — but nothing prevents it from being present transitively or in a given build environment, and `send2trash`'s own fallback logic silently switches to the COM path whenever it's importable, with no app-level control over that choice today. By contrast, `send2trash/win/legacy.py` (the fallback when `pywin32` is absent) uses only `ctypes` + `SHFileOperationW` — no COM, no apartment/message-pump requirements, safe to call from an arbitrary thread, and any failure surfaces as a normal `OSError`/`WindowsError` that `DeleteWorker`'s existing `except OSError` already handles correctly. ### Proposed fix Force `DeleteWorker` to always use `send2trash.win.legacy.send2trash` on `sys.platform == "win32"`, instead of the package's own auto-detection, so the COM-based backend is never reachable from this app regardless of whether `pywin32` happens to be present in a given build/environment. Non-Windows platforms are unaffected. ### Acceptance criteria - [ ] `DeleteWorker` on Windows never invokes `send2trash.win.modern` (COM/`IFileOperation`) — always the `ctypes`/`SHFileOperationW` legacy backend. - [ ] macOS/Linux behavior unchanged. - [ ] A Decision Record documents the reasoning (this is unconfirmed against an actual Windows crash log/report, same as DR-027 was before its follow-up). - [ ] Regression test verifies the Windows import path resolves to the legacy backend. ### Status Not yet reproduced by the user on a retry; root cause identified via code inspection and precedent (DR-027/028), not confirmed via an actual crash log or dump from the user's machine. Proceeding with the fix per the same pattern DR-027 used (implement the well-evidenced fix, await confirmation). If this recurs after the fix ships, the next step is capturing `%APPDATA%\bulk-image-organizer\logs` and/or a `nuitka-crash-report.xml` from the crash for further diagnosis.

Claude added the

Kind/Bug

Priority/High

labels

2026-08-21 01:19:01 +00:00

Claude referenced this issue from a commit

2026-08-21 01:26:52 +00:00

fix(core): force send2trash's legacy backend on Windows, never COM IFileOperation

Claude commented

2026-08-21 01:26:56 +00:00

Author

Member

Implemented per the proposed fix, documented as DR-037 in docs/DECISIONS.md. core/delete_worker.py now forces send2trash.win.legacy (ctypes/SHFileOperationW, no COM) on sys.platform == "win32" instead of letting send2trash auto-select its COM-based IFileOperation backend whenever pywin32 happens to be importable. macOS/Linux unaffected.

Regression test tests/test_delete_worker_windows_backend.py verifies the module resolves to the legacy backend on a simulated win32 platform (stubbing send2trash.win.legacy/.modern and reloading the module) and never reaches the modern/COM backend on other platforms. Full suite (705 passed) and ruff pass; UI exit smoke test clean.

Committed to 0.6.1/v0.7.0 (the same branch #163/#167/#168 are on) rather than a separate branch, per direction. Still unconfirmed against an actual Windows crash — no crash log or dump was available, and the root cause was identified by reading the installed send2trash source and this project's DR-027/DR-028 precedent, not by reproducing the crash. If it recurs after this ships, next step is capturing %APPDATA%\bulk-image-organizer\logs and/or a Nuitka crash report.

Implemented per the proposed fix, documented as DR-037 in `docs/DECISIONS.md`. `core/delete_worker.py` now forces `send2trash.win.legacy` (ctypes/`SHFileOperationW`, no COM) on `sys.platform == "win32"` instead of letting `send2trash` auto-select its COM-based `IFileOperation` backend whenever `pywin32` happens to be importable. macOS/Linux unaffected. Regression test `tests/test_delete_worker_windows_backend.py` verifies the module resolves to the legacy backend on a simulated `win32` platform (stubbing `send2trash.win.legacy`/`.modern` and reloading the module) and never reaches the modern/COM backend on other platforms. Full suite (705 passed) and ruff pass; UI exit smoke test clean. Committed to `0.6.1/v0.7.0` (the same branch #163/#167/#168 are on) rather than a separate branch, per direction. **Still unconfirmed against an actual Windows crash** — no crash log or dump was available, and the root cause was identified by reading the installed `send2trash` source and this project's DR-027/DR-028 precedent, not by reproducing the crash. If it recurs after this ships, next step is capturing `%APPDATA%\bulk-image-organizer\logs` and/or a Nuitka crash report.

Claude referenced this issue from a commit

2026-08-21 05:26:20 +00:00

Revert "fix(core): force send2trash's legacy backend on Windows, never COM IFileOperation"

Claude commented

2026-08-21 05:26:22 +00:00

Author

Member

Reverting the send2trash backend change (commit 3ccdad0 on 0.6.1/v0.7.0), including DR-037 and its test — per direction, since:

No code in this branch (including #168's status-bar changes) was found to touch the delete-completion path at all — I diffed it specifically and confirmed _start_confirm_delete/_on_delete_progress/_on_delete_finished/_on_delete_error are untouched by any commit here.
The crash hasn't recurred or been reproduced.

Treating this as a one-time issue on the reporter's system rather than an application bug for now. Leaving this issue open as a placeholder with no code change — reopen investigation here if it happens again, ideally with the app log (%APPDATA%\bulk-image-organizer\logs\bulk-image-organizer.log) from that session.

Reverting the send2trash backend change (commit `3ccdad0` on `0.6.1/v0.7.0`), including DR-037 and its test — per direction, since: - No code in this branch (including #168's status-bar changes) was found to touch the delete-completion path at all — I diffed it specifically and confirmed `_start_confirm_delete`/`_on_delete_progress`/`_on_delete_finished`/`_on_delete_error` are untouched by any commit here. - The crash hasn't recurred or been reproduced. Treating this as a one-time issue on the reporter's system rather than an application bug for now. Leaving this issue open as a placeholder with no code change — reopen investigation here if it happens again, ideally with the app log (`%APPDATA%\bulk-image-organizer\logs\bulk-image-organizer.log`) from that session.

Claude added

and removed

labels

2026-08-21 05:26:28 +00:00

Claude referenced this issue

2026-08-22 04:35:05 +00:00

Fix viewer stuck image, onefile cache prune, status bar scope (0.6.2) #172