fix: restore signal handlers on lock release

[davidberenstein1957]

Description

codecarbon/lock.py now saves the signal handlers replaced by signal.signal() in self._previous_handlers (main thread only, once the lock file is acquired) instead of discarding them. _handle_exit releases the lock and then delegates to the handler it replaced: it calls a previous Python handler, does nothing for SIG_IGN, and for the default disposition raises SystemExit(128 + signum) or KeyboardInterrupt for SIGINT, so finally / __exit__ / @track_emissions still write the final emissions. release() removes the lock file first, then restores the saved handlers, but only when the currently installed handler is still the one it set, so an application that registered its own handler afterward is not clobbered; restoring is skipped off the main thread, where signal.signal() raises, so stop() from a worker thread still writes its CSV and removes the lock file. The internal thread lock is now an RLock, since _handle_exit can call release() on a thread already holding it.

Related Issue

Fixes #1310

Motivation and Context

Lock is constructed whenever allow_multiple_runs=False. Because it overwrote the process signal disposition permanently, restoring the default SIGINT handler never happened, so KeyboardInterrupt stopped being raised at all — every except KeyboardInterrupt: in an embedding application became dead code, including CodeCarbon's own in codecarbon/cli/monitor.py. Applications that had registered a graceful-shutdown SIGTERM handler also lost it silently.

Behavior change: previously any SIGINT/SIGTERM ended in SystemExit(1). Now the process does whatever the application asked for; with no application handler, SIGTERM exits with the conventional code 143 (after cleanup) and SIGINT raises KeyboardInterrupt.

How Has This Been Tested?

tests/test_lock.py (TestLockSignalHandlers) covers: handlers restored on release, forwarding to a previous Python handler, SystemExit(143) on SIGTERM with the default disposition, SIG_IGN kept, release from a worker thread, a None previous handler, no deadlock on re-entrant release, and a failed acquire() leaving handlers alone.

Screenshots (if appropriate):

N/A

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

AI Usage Disclosure

• 🟥 AI-vibecoded
• 🟠 AI-generated
• ⭐ AI-assisted
• ♻️ No AI used

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the docs/how-to/contributing.md document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[codecov]

Codecov Report

❌ Patch coverage is 89.28571% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.73%. Comparing base (e5e46ab) to head (f132178).

Files with missing lines	Patch %	Lines
codecarbon/lock.py	89.28%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1329      +/-   ##
==========================================
+ Coverage   91.70%   91.73%   +0.03%     
==========================================
  Files          49       49              
  Lines        5157     5178      +21     
==========================================
+ Hits         4729     4750      +21     
  Misses        428      428              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[benoit-cty]

🤖 This review comment was written and posted by Claude Opus 5.5 (AI assistant), at the request of @benoit-cty. Findings were checked by reading the code and running tests locally (merged with current master where relevant), but please double-check before acting on them.

Verdict: 🔧 Request changes

The bug is real. Lock permanently took over SIGINT/SIGTERM and always raised SystemExit(1). The fix saves the previous handlers, restores them on release, forwards the signal to them, and uses an RLock.

• Nested trackers are fine: a second lock never acquires, so it installs no handlers.
• Handlers the user sets after acquire() are kept (the getsignal check).

Must fix:

1. Calling release() off the main thread loses the final emissions and leaves the lock file behind (codecarbon/lock.py L71-75).
   • signal.signal() only works on the main thread, and release() now calls it with no thread check, so it raises ValueError.
   • stop() calls _lock.release() before saving anything. @suppress swallows the exception, so the rest of stop() never runs.
   • I verified this: with OfflineEmissionsTracker(allow_multiple_runs=False) and stop() called from a worker thread, master writes the CSV and removes the lock. With this PR, stop() returns None, no CSV is written, and the lock file stays until the process exits.
   • Stopping from a callback or worker thread is a common pattern.
   • Fix: only restore handlers when threading.current_thread() is threading.main_thread(). Remove the lock file first, and wrap the restore in try/except. Please add a test that stops from a worker thread.
2. SIGTERM now kills the process with no final write (lock.py L42-43).
   • When the previous SIGTERM handler was SIG_DFL, the PR restores it and re-sends the signal with os.kill. The process then dies immediately: no finally, no __exit__, no tracker.stop().
   • On master, SystemExit(1) let with / @track_emissions write the final emissions. I verified this: master exits with code 1 and finally runs; the PR exits with 143 and finally does not run.
   • On SLURM/Kubernetes shutdowns this loses the final row that users get today.
   • Suggested fix: forward only to callable previous handlers. When the previous handler is SIG_DFL, raise SystemExit(128 + signum) (or KeyboardInterrupt for SIGINT), so cleanup runs and the exit code stays conventional.

Nits:

• If the previous handler was installed from C code, signal.getsignal() returns None. release() then calls signal.signal(sig, None), which raises TypeError, and _handle_exit swallows the signal. Treat None as SIG_DFL.
• The description refers to an _atexit_hook that isn't in the code, and the ordering note about stop idempotency is already covered by fix: make tracker.stop() idempotent #1408. Please update it.

[davidberenstein1957]

Made the changes in f132178: worker-thread release and SIGTERM cleanup fixed.

• release() removes the lock file first and only restores handlers on the main thread (new worker-thread test).
• A default previous handler now raises SystemExit(128 + signum) / KeyboardInterrupt instead of os.kill, so finally and the final CSV row still happen (checked: exit 143, row written).
• A None previous handler is restored as SIG_DFL. Updated the description too.