Skip to content

Reduce likelihood of timeout/signal related race condition - #476

Open
LudovicoRighi wants to merge 4 commits into
mainfrom
no-process-error
Open

Reduce likelihood of timeout/signal related race condition#476
LudovicoRighi wants to merge 4 commits into
mainfrom
no-process-error

Conversation

@LudovicoRighi

@LudovicoRighi LudovicoRighi commented Jul 8, 2025

Copy link
Copy Markdown
Collaborator

Summary: there is a race condition (i.e. trying to send a signal to a process which doesn't exist anymore) in Procstar; this PR helps to make that race less likely, but won't fix it completely. More details below.


The race condition

  • To me, it looks like there is a race condition in Procstar, i.e. we may try to send a signal to a process that don't exist anymore, which results in a AgentMessageError: agent error response to message: no process..
    • see the kill function

      • ESRCH=The target process or process group does not exist. Note that an existing process might be a zombie, a process that has terminated execution, but has not yet been wait(2)ed for. No such process from man (from kill man)
    • I think this race condition could be fixed by using a locking mechanism between:

    • Ideally, rather than blindly trying to send the signal and then erroring with no process, we should rather ensure the process is still around before sending the signal and only log something like process already terminated: not sending the signal.
      (Not sure if there's also the risk that we kill another process which may have started with the same PID, haven't double checked yet).


The change

The idea of this change is to cancel the timeout task as soon as the update regarding the process termination is received.
This should help to reduce the likelihood of the issue explained above:

  • in particular, this helps in the case in which a run has already terminated during a disconnection period
    • as soon as Apsis and Procstar reconnect, Apsis would receive a process update that we can use to decide whether to cancel the timeout or not.

Issue reproduction:

Notice that running the test I've added against master branch helps to reproduce the AgentMessageError consistently.

> pytest test/int/procstar/test_timeout.py::test_timeout_with_delayed_reconnect_process_terminated['timeout-failure-failure'] -s -vv
(...)
2025-07-10T12:50:07.133 apsis.program.procstar.agent I reconnected: 1e8e1a45-e79c-4c5d-910a-a7e00b7b7ea9 on conn 86f1a7dc-b136-40f3-a666-3c8b94de2582
2025-07-10T12:50:07.133 apsis.program.procstar.agent I r1: timeout
2025-07-10T12:50:07.134 procstar.agent.proc      E agent error: no process
2025-07-10T12:50:07.135 apsis.program.procstar.agent E procstar
Traceback (most recent call last):
  File "/space/asd/conda7/envs/prod7-20250710-006/lib/python3.10/site-packages/apsis/program/procstar/agent.py", line 550, in updates
    fd_data = await asyncio.wait_for(
  File "/space/asd/conda7/envs/prod7-20250710-006/lib/python3.10/asyncio/tasks.py", line 445, in wait_for
    return fut.result()
  File "/space/asd/conda7/envs/prod7-20250710-006/lib/python3.10/site-packages/apsis/program/procstar/agent.py", line 326, in collect_final_fd_data
    async for update in proc.updates:
  File "/space/asd/conda7/envs/prod7-20250710-006/lib/python3.10/site-packages/procstar/agent/proc.py", line 155, in updates
    raise AgentMessageError(msg, err)
procstar.agent.proc.AgentMessageError: agent error response to message: no process
2025-07-10T12:50:07.135 apsis.run_log            I r1: error: procstar: agent error response to message: no process
2025-07-10T12:50:07.175 apsis.service.main       E caught SIGTERM
2025-07-10T12:50:07.175 apsis.service.main       I task cancelled with return: restore
2025-07-10T12:50:07.175 apsis.service.main       I task cancelled with return: Sanic
2025-07-10T12:50:07.175 apsis.apsis              I shutting down Apsis
2025-07-10T12:50:07.175 apsis.apsis              I task cancelled with CancelledError: check_async
2025-07-10T12:50:07.176 procstar.agent.server    W closed: 86f1a7dc-b136-40f3-a666-3c8b94de2582: no close frame received or sent
(...)

=============================================================================================================================== short test summary info ===============================================================================================================================
FAILED test/int/procstar/test_timeout.py::test_timeout_with_delayed_reconnect_process_terminated[timeout-failure-failure] - AssertionError: assert 'error' == 'failure'
  
  - failure
  + error
============================================================================================================================ 1 failed, 4 warnings in 5.21s ============================

@LudovicoRighi
LudovicoRighi force-pushed the no-process-error branch 3 times, most recently from 38459c5 to a307137 Compare July 10, 2025 12:40
@LudovicoRighi LudovicoRighi changed the title Fix agent error response to message: no process Reduce likelihood of timeout/signal related race condition Jul 10, 2025
@LudovicoRighi
LudovicoRighi marked this pull request as ready for review July 10, 2025 16:13
@LudovicoRighi
LudovicoRighi requested a review from gusostow July 10, 2025 17:04
@gusostow

Copy link
Copy Markdown
Contributor

Is there a way you can make send_signal a best-effort operation, so that we don't care if the agent sends back an error? That feels like fewer moving parts than pre-empt the task with a timeout

@LudovicoRighi

Copy link
Copy Markdown
Collaborator Author

Sounds good; I've opened apsis-scheduler/procstar#62.

@LudovicoRighi
LudovicoRighi changed the base branch from master to main July 16, 2025 11:46
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.

3 participants