Skip to content

fix: make tracker.stop() idempotent - #1336

Open
davidberenstein1957 wants to merge 3 commits into
masterfrom
fix/stop-idempotent
Open

fix: make tracker.stop() idempotent#1336
davidberenstein1957 wants to merge 3 commits into
masterfrom
fix/stop-idempotent

Conversation

@davidberenstein1957

Copy link
Copy Markdown
Collaborator

Calling stop() twice wrote a second complete emissions row (two CSV rows, two API POSTs, two handler.exit() calls) for a single run, because nothing recorded that the tracker had stopped — _start_time stays set and both schedulers being None was only used to emit an advisory warning.

What changed

  • _initialize_runtime_state() now initializes _is_stopped, final_emissions and final_emissions_data.
  • stop() returns early on _is_stopped, returning the cached final_emissions.
  • The lock release moved after the guard, so a repeat stop() no longer retries os.remove on an already-removed lock file.
  • The misplaced else: logger.warning("Tracker already stopped !") (bound to the _scheduler_monitor_power check) is replaced by the real terminal-state guard.

Why

__exit__ calls stop() unconditionally, so a with block plus an explicit stop() — the usual way to get the return value — double counts the whole run.

Verification

tests/test_emissions_tracker.py::TestCarbonTracker::test_offline_tracker_stop_is_idempotent asserts one CSV row and an identical return value after a double stop. It fails on master (2 rows) and passes with this change. Full file: 32 passed.

Closes #1307

🤖 Generated with Claude Code

A second stop() ran a full measurement and wrote a duplicate row through
every output handler. Guard on an explicit _is_stopped flag and return the
cached final_emissions instead. The lock release now happens after the
guard, so a repeat stop no longer retries os.remove.

Closes #1307

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.54%. Comparing base (065d0e6) to head (045655e).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1336      +/-   ##
==========================================
+ Coverage   91.39%   91.54%   +0.15%     
==========================================
  Files          49       49              
  Lines        5056     5063       +7     
==========================================
+ Hits         4621     4635      +14     
+ Misses        435      428       -7     

☔ 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.

davidberenstein1957 and others added 2 commits August 12, 2026 17:47
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Lock.release() left _has_created_lock True, so a second release()
deleted a lock file that may by then belong to another process.

Rename the stop guard flag to _stopped_at (a timestamp) so start/stop
share one piece of state instead of two.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@davidberenstein1957
davidberenstein1957 marked this pull request as ready for review August 12, 2026 19:14
@davidberenstein1957
davidberenstein1957 requested a review from a team as a code owner August 12, 2026 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tracker.stop() is not idempotent: a second call writes a duplicate emissions row

1 participant