Skip to content

Fix Windows crash on GUI success logs - #123

Open
RenanGBarreto wants to merge 4 commits into
mainfrom
fix/windows-unicode-logging
Open

Fix Windows crash on GUI success logs#123
RenanGBarreto wants to merge 4 commits into
mainfrom
fix/windows-unicode-logging

Conversation

@RenanGBarreto

Copy link
Copy Markdown
Contributor

Fix Windows crash on GUI success logs

🤔 Why?

On Windows, the GUI runs the CLI as a subprocess and pipes its stdout. Those pipes often use the system ANSI code page (e.g., cp1252). When a success message includes a non‑ASCII glyph (like an emoji), print() can raise UnicodeEncodeError, causing the GUI to report a failure even though the image was written successfully. See #122.

🔧 What changed

  • Catch UnicodeEncodeError in logging.log() and retry with an ASCII‑only fallback (sanitized text and ASCII icon when provided).
  • Ensure the GUI subprocess entry point reconfigures stdout/stderr to UTF‑8 with errors="replace" when available (guarded with contextlib.suppress).
  • Add unit tests that simulate Windows pipes rejecting non‑ASCII both in the message body and in the icon path.

🧪 How to test

  • Run unit tests: pytest tests/mkpfs/test_logging.py.
  • Manual (Windows): pack a single file to FFPFS via the GUI. Expect a success message without a crash even on non‑UTF‑8 systems; message will use ASCII when needed.

Closes #122.

On Windows, multiprocessing.Pool uses 'spawn' which re-imports
__main__ in each child. When this happens from a non-main thread
inside a --windowed PyInstaller binary, the spawn handshake
blocks the Windows message pump, making the window unresponsive.

This replaces the in-process cli_mkpfs_main() call with a
subprocess.Popen child, keeping the GUI thread free. The
subprocess handle is stored for a future stop/cancel button.

Output is streamed line-by-line to the log pane. A --gui-subprocess
router in the entry point allows the frozen binary to serve both
GUI and CLI roles. Windows console-window flash is suppressed via
CREATE_NO_WINDOW. Overwrite prompts are auto-confirmed via stdin.

Closes #120.
Address code review feedback on PR #121: the bare pass in the
BrokenPipeError/OSError handler was flagged. Added a comment
explaining why it is safe to ignore (child crashed before reading
stdin; the streaming loop will surface the error).
@RenanGBarreto RenanGBarreto added the bug Something isn't working label Aug 31, 2026
@github-code-quality

github-code-quality Bot commented Aug 31, 2026

Copy link
Copy Markdown

Code Coverage Overview

Languages: Python

Python / code-coverage/pytest

The overall line coverage in commit 9a6520e in the fix/windows-unicode-... branch remains at 72%, unchanged from commit 2abb9cd in the main branch.

Show a line coverage summary of the most impacted files.
File main 2abb9cd fix/windows-unicode-... 9a6520e +/-
logging.py 82% 84% +2%

Updated August 31, 2026 15:00 UTC

Comment thread mkpfs/gui/panels/base.py Fixed
Comment thread mkpfs/gui/panels/base.py
try:
proc.stdin.write("y\n")
proc.stdin.close()
except (BrokenPipeError, OSError):
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] UnicodeEncodeError on Windows when packing a single file via the GUI

1 participant