Reset cleanup flag when cleanup fails
With this flag set before app.cleanup() returns, any normal Exception from EBEAMSystemDashboard.cleanup() marks shutdown as complete even if cleanup aborted partway through. In the window-close/Ctrl-Q path finish_shutdown() runs once from quit_app and then again from the root.mainloop() finally, so a partial cleanup failure causes the second pass to return immediately and skip retrying the remaining COM-port/update cancellations before the root and logger are torn down. Set the flag only after cleanup succeeds, or clear it in the except Exception path.
Originally posted by @chatgpt-codex-connector[bot] in #162 (comment)
Description:
The dashboard shutdown path can incorrectly mark cleanup as complete before cleanup actually succeeds.
In main.py, run_cleanup() sets cleanup_complete = True before calling app.cleanup(). If EBEAMSystemDashboard.cleanup() raises a normal Exception, run_cleanup() logs the error but leaves cleanup_complete set to True. During the normal window-close/Ctrl-Q shutdown path, finish_shutdown() can run once from quit_app() and then again from the root.mainloop() finally block. Because the flag is already set, the second pass returns immediately and cannot retry cleanup work that may have been skipped after the first exception.
dashboard.py catches many expected per-subsystem cleanup errors internally, so this is probably not triggered by ordinary COM-port close or update-cancel failures. However, any unexpected exception escaping from EBEAMSystemDashboard.cleanup() can still cause partial cleanup to be treated as successful. That could leave scheduled callbacks, subsystem update loops, or serial cleanup attempts unhandled before the Tk root and logger are torn down.
Suggested fix:
- Only set
cleanup_complete = True after app.cleanup() returns successfully, or explicitly reset cleanup_complete = False in the except Exception path.
- Preserve the existing
BaseException behavior of resetting the flag and re-raising.
- Add a regression check where cleanup raises once and then succeeds on a second
finish_shutdown() call, confirming the retry is not skipped.
This is a small, low-risk shutdown hardening fix.
With this flag set before
app.cleanup()returns, any normalExceptionfromEBEAMSystemDashboard.cleanup()marks shutdown as complete even if cleanup aborted partway through. In the window-close/Ctrl-Q pathfinish_shutdown()runs once fromquit_appand then again from theroot.mainloop()finally, so a partial cleanup failure causes the second pass to return immediately and skip retrying the remaining COM-port/update cancellations before the root and logger are torn down. Set the flag only after cleanup succeeds, or clear it in theexcept Exceptionpath.Originally posted by @chatgpt-codex-connector[bot] in #162 (comment)
Description:
The dashboard shutdown path can incorrectly mark cleanup as complete before cleanup actually succeeds.
In
main.py,run_cleanup()setscleanup_complete = Truebefore callingapp.cleanup(). IfEBEAMSystemDashboard.cleanup()raises a normalException,run_cleanup()logs the error but leavescleanup_completeset toTrue. During the normal window-close/Ctrl-Q shutdown path,finish_shutdown()can run once fromquit_app()and then again from theroot.mainloop()finallyblock. Because the flag is already set, the second pass returns immediately and cannot retry cleanup work that may have been skipped after the first exception.dashboard.pycatches many expected per-subsystem cleanup errors internally, so this is probably not triggered by ordinary COM-port close or update-cancel failures. However, any unexpected exception escaping fromEBEAMSystemDashboard.cleanup()can still cause partial cleanup to be treated as successful. That could leave scheduled callbacks, subsystem update loops, or serial cleanup attempts unhandled before the Tk root and logger are torn down.Suggested fix:
cleanup_complete = Trueafterapp.cleanup()returns successfully, or explicitly resetcleanup_complete = Falsein theexcept Exceptionpath.BaseExceptionbehavior of resetting the flag and re-raising.finish_shutdown()call, confirming the retry is not skipped.This is a small, low-risk shutdown hardening fix.