fix: prevent crash when force-closing during PDF download (#59)#61
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a crash that occurs when users force-close the application window during a PDF download. The issue was caused by background download threads attempting to call Tkinter UI update methods after the main window was destroyed.
Changes:
- Enabled daemon threads to allow the process to exit cleanly even with running background threads
- Added
app_closingflag with guards in UI update calls to prevent post-destruction UI access - Replaced
sys.exit(0)withroot.destroy()for proper resource cleanup during window closure
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return | ||
|
|
||
| try: | ||
| root.after(0, lambda: (not app_closing) and func(*args, **kwargs)) |
There was a problem hiding this comment.
The lambda function uses (not app_closing) and func(*args, **kwargs) which will return the result of func(*args, **kwargs) when app_closing is False, but will return False when app_closing is True. This is correct for preventing execution, but the lambda will still be queued even if app_closing becomes True between line 706 and line 710. Additionally, this creates a redundant check since line 706-707 already guards against app_closing. Consider simplifying to root.after(0, lambda: func(*args, **kwargs)) since the early return on line 706-707 already handles the app_closing check.
| root.after(0, lambda: (not app_closing) and func(*args, **kwargs)) | |
| root.after(0, lambda: func(*args, **kwargs)) |
|
|
||
| session = requests.Session() # 初始化请求 | ||
| download_states = [] # 初始化下载状态 | ||
| app_closing = False |
There was a problem hiding this comment.
The global flag app_closing is accessed and modified across multiple threads without synchronization primitives (e.g., threading.Lock). In Python, while simple boolean operations are generally atomic due to the GIL, there's no guarantee that all threads will immediately see the updated value due to potential memory caching. This could lead to race conditions where background threads continue to call ui_call even after app_closing is set to True. Consider using threading.Event instead, which provides proper memory barriers and thread synchronization. For example: app_closing = threading.Event(), then use app_closing.is_set() to check and app_closing.set() to signal closure.
| except Exception: | ||
| # 主窗口销毁后,root.after 会抛错,直接忽略即可 | ||
| pass |
There was a problem hiding this comment.
Using a bare except Exception: here silently swallows all exceptions, which could hide legitimate errors beyond just the expected case where root has been destroyed. While this may work for the intended use case, it makes debugging more difficult. Consider being more specific about what exception you expect (e.g., except tk.TclError: for Tkinter-related errors when the window is destroyed), or at minimum, log the exception for debugging purposes.
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Using a bare except Exception: here silently swallows all exceptions during window destruction. While this provides robustness, it could hide legitimate errors. Consider being more specific (e.g., except tk.TclError:) or at minimum, log the exception using print_error(e) to aid debugging, consistent with error handling elsewhere in the codebase (e.g., lines 28-29, 383).
| except Exception: | |
| pass | |
| except Exception as e: | |
| print_error(e) |
Motivation
root.after)。(终止下载导致崩溃 #59)Description
t.daemon = True)以避免后台线程阻止进程退出;修改点在src/tchMaterial-parser.pyw中的thread_it。app_closing并在ui_call中先检查该标志以避免在关闭过程中调度 UI 更新,同时对root.after抛错做安全吞掉处理。on_closing,增加幂等性保护,先设置app_closing = True,结束子进程并尝试root.destroy(),替代直接调用sys.exit(0),从而安全完成资源清理。src/tchMaterial-parser.pyw。Testing
python -m py_compile src/tchMaterial-parser.pyw校验语法,结果成功。Codex Task