Summary
The UI conversion cancel path still force-terminates the conversion worker if graceful cancellation takes more than two seconds. That is risky while the worker may be inside OIIO reads, FFmpeg writes, or output-file cleanup.
Current Evidence
Checked on origin/main at a900411.
src/renderkit/ui/main_window_logic.py contains:
self.worker.request_cancel()
# If it doesn't stop in 2 seconds, terminate
if not self.worker.wait(2000):
logger.warning("Worker did not stop gracefully, terminating...")
self.worker.terminate()
self.worker.wait()
This contradicts the safer preview-worker pattern in src/renderkit/ui/widgets.py, which explicitly avoids terminating threads busy with I/O.
Risk
QThread.terminate() can stop execution at an arbitrary point. During conversion this can leave:
- partially written or corrupt output files
- FFmpeg subprocesses/resources in an uncertain state
- OIIO/cache/file handles in an unsafe state
- UI state showing cancellation even if cleanup did not complete cleanly
Affected Code
src/renderkit/ui/main_window_logic.py:1784 (_cancel_conversion)
src/renderkit/ui/conversion_worker.py (ConversionWorker.request_cancel)
src/renderkit/core/converter.py progress callback cancellation path
Suggested Fix
Prefer cooperative cancellation end to end:
- Keep requesting cancellation through the progress callback.
- Disable repeated cancel clicks while cancellation is pending.
- Show a persistent "Cancelling..." state instead of killing the thread after a fixed timeout.
- Ensure converter/encoder cleanup runs through normal
finally blocks.
- Consider interrupting/closing FFmpeg through a controlled encoder API if immediate cancellation is needed.
Acceptance Criteria
- The conversion UI no longer calls
QThread.terminate() for normal cancellation.
- A test or Qt-focused smoke path verifies cancel state transitions.
- Output cleanup behavior is defined for cancelled conversions.
- The user still gets clear feedback if cancellation takes longer than two seconds.
Summary
The UI conversion cancel path still force-terminates the conversion worker if graceful cancellation takes more than two seconds. That is risky while the worker may be inside OIIO reads, FFmpeg writes, or output-file cleanup.
Current Evidence
Checked on
origin/mainata900411.src/renderkit/ui/main_window_logic.pycontains:This contradicts the safer preview-worker pattern in
src/renderkit/ui/widgets.py, which explicitly avoids terminating threads busy with I/O.Risk
QThread.terminate()can stop execution at an arbitrary point. During conversion this can leave:Affected Code
src/renderkit/ui/main_window_logic.py:1784(_cancel_conversion)src/renderkit/ui/conversion_worker.py(ConversionWorker.request_cancel)src/renderkit/core/converter.pyprogress callback cancellation pathSuggested Fix
Prefer cooperative cancellation end to end:
finallyblocks.Acceptance Criteria
QThread.terminate()for normal cancellation.