Skip to content

Fix advance_time re-entrancy guard leaking on early return/exception#220

Open
gomezzz wants to merge 2 commits into
ci/uv-migrationfrom
fix/advance-time-reentrancy
Open

Fix advance_time re-entrancy guard leaking on early return/exception#220
gomezzz wants to merge 2 commits into
ci/uv-migrationfrom
fix/advance-time-reentrancy

Conversation

@gomezzz

@gomezzz gomezzz commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

PASEOS.advance_time set its re-entrancy guard _is_advancing_time = True before validating arguments and only cleared it on the normal return path. Any early return (radiation restart/death) or exception therefore left the guard stuck True, so every subsequent call failed the assert not self._is_advancing_time ("advance_time is already running").

In real-time activity mode the measured interval between processor updates can round to zero; advance_time(0) then tripped the positive-time assert, poisoned the guard, crashed the background ActivityProcessor, and deadlocked wait_for_activity.

  • Release the guard in a finally block so it is always cleared.
  • Treat a zero-length interval as a no-op (return before touching the guard).
  • Skip zero-length processor updates in ActivityProcessor._update instead of asserting.

No behavioural change on the normal path; the fix only affects the leak/cleanup and zero-length edge cases.

Test plan

  • Full suite green on the uv/Python 3.8 CI (36/36).

Stacked on the uv PR (#221) so it inherits the green uv CI; independent of the tooling changes and can be re-parented onto main if you'd rather merge it on its own.

@gomezzz gomezzz mentioned this pull request Jul 24, 2026
2 tasks
advance_time set _is_advancing_time=True before validating its arguments
and only cleared it on the normal return path, so any early return
(radiation events) or exception left the guard stuck True, poisoning
every subsequent call with a spurious 'advance_time is already running'
assertion. In real-time activity mode the measured interval can round to
zero, tripping the positive-time assert, which crashed the background
ActivityProcessor and deadlocked wait_for_activity.

Release the guard in a finally block, treat a zero interval as a no-op,
and skip zero-length processor updates instead of asserting.
@gomezzz
gomezzz force-pushed the fix/advance-time-reentrancy branch from 5e48f0f to f9fc9df Compare July 24, 2026 09:33
@gomezzz
gomezzz changed the base branch from ci/ruff-lint-format to ci/uv-migration July 24, 2026 09:33
@gomezzz gomezzz mentioned this pull request Jul 24, 2026
2 tasks
@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown

Overall Coverage

Coverage Report
FileStmtsMissCoverMissing
__init__.py33197%48
paseos.py1471292%67–68, 153–154, 214, 229, 247, 256, 274, 305, 313, 316
activities
   activity_manager.py43393%46, 74, 156
   activity_processor.py59198%91
   activity_runner.py621084%80–81, 84, 104, 108–109, 112–113, 122, 125
actors
   actor_builder.py1853283%24–25, 27, 30, 33, 223–225, 256–258, 312, 316–317, 320–321, 342, 344–345, 348–349, 488, 545, 573, 585–586, 588–589, 596–597, 604–605
   base_actor.py1281886%82, 104, 120, 167, 207–209, 230, 239, 256, 277, 283–286, 292, 307, 362
   ground_station_actor.py15380%47, 51, 53
   spacecraft_actor.py60198%122
central_body
   central_body.py65592%68, 177–178, 186–187
   is_in_line_of_sight.py581279%107–108, 110, 112–117, 124, 168, 188
   mesh_between_points.py35294%64, 72
   sphere_between_points.py26869%35, 37–38, 48, 50–51, 63–64
communication
   find_next_window.py200100% 
   get_communication_window.py23291%38, 64
geometric_model
   geometric_model.py281643%32, 34–38, 47, 49, 61, 76, 78–79, 92–93, 101–102
power
   charge_model.py16194%50
   discharge_model.py7186%34
   power_device_type.py40100% 
radiation
   radiation_model.py320100% 
tests
   activity_test.py56395%92, 95–96
   actor_builder_test.py630100% 
   advance_time_test.py170100% 
   communication_window_test.py48198%177
   custom_propagator_test.py250100% 
   custom_property_test.py230100% 
   default_cfg_test.py90100% 
   eclipse_test.py10190%20
   event_based_test.py230100% 
   import_test.py6183%13
   init_test.py8188%16
   line_of_sight_test.py62494%143–146
   mesh_test.py119397%119, 137, 145
   multiple_instance_test.py120100% 
   operations_monitor_test.py310100% 
   power_test.py290100% 
   radiation_test.py590100% 
   test_utils.py170100% 
   thermal_model_test.py30197%70
   time_multiplier_test.py260100% 
   visualization_test.py18194%30
thermal
   thermal_model.py750100% 
utils
   check_cfg.py611870%31, 40, 46, 60–62, 67, 70–72, 75–77, 80–82, 98, 103
   load_default_cfg.py90100% 
   operations_monitor.py71396%127–128, 130
   reference_frame.py40100% 
   set_log_level.py60100% 
visualization
   animation.py18667%28–31, 36, 45
   plot.py9367%29–30, 32
   space_animation.py2152389%109–110, 125, 148–149, 183, 207, 214, 220, 330, 353–355, 360, 369–370, 374, 410, 416–417, 448, 462, 473
TOTAL220519791% 

Tests Skipped Failures Errors Time
38 0 💤 0 ❌ 0 🔥 3m 55s ⏱️

Covers the two failure modes fixed here: a zero-length interval is a
no-op that never engages the guard, and an exception mid-advance_time
still releases it so a subsequent call succeeds (previously deadlocked).
Also corrects the ActivityProcessor._update comment now that advance_time
handles zero intervals itself.
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.

1 participant