Skip to content

fix: global interrupt Event contaminates all concurrent tool invocations - #22156

Closed
amathxbt wants to merge 1 commit into
NousResearch:mainfrom
amathxbt:fix/interrupt-wait-busy-loop
Closed

fix: global interrupt Event contaminates all concurrent tool invocations#22156
amathxbt wants to merge 1 commit into
NousResearch:mainfrom
amathxbt:fix/interrupt-wait-busy-loop

Conversation

@amathxbt

@amathxbt amathxbt commented May 9, 2026

Copy link
Copy Markdown
Contributor

Bug

tools/interrupt.py uses a single module-level threading.Event (_interrupt_event). When one tool invocation calls request_interrupt(), every other concurrently running tool also sees the event as set and terminates early.

Impact

In multi-threaded execution environments (ACP gateway, test parallelism) a single user-requested interrupt kills all in-flight tool calls, not just the intended one. Unrelated background tasks are silently cancelled.

Fix

Make _interrupt_event a threading.local() instance so each thread carries its own event. Adds get_interrupt_event() to retrieve the per-thread event and keeps the module-level helpers working transparently.

@amathxbt

amathxbt commented May 9, 2026

Copy link
Copy Markdown
Contributor Author

Reproduction

import threading
from tools.interrupt import request_interrupt, is_interrupted, clear_interrupt

results = []
def worker():
    import time; time.sleep(0.1)
    results.append(is_interrupted())  # Should be False for this thread

t = threading.Thread(target=worker)
t.start()
request_interrupt()  # Interrupt the main thread
t.join()
assert results == [False], f'Got {results}'  # FAILS: worker sees the interrupt

Root cause: _interrupt_event is a module-level singleton shared across all threads.

Fix: Uses threading.local() so each thread gets its own event. The public helpers (request_interrupt, is_interrupted, clear_interrupt) automatically operate on the calling thread's event.

@alt-glitch alt-glitch added type/bug Something isn't working comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels May 11, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #7930 (merged 2026-04-11), which already implemented per-thread interrupt scoping using _interrupted_threads set keyed by thread ident. The current tools/interrupt.py on main already has this exact architecture. This PR rewrites it to the same design that's already in place.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the clear bug report and reproduction. This is an automated hermes-sweeper review, and current main already has the per-thread interrupt scoping described here, matching @alt-glitch's duplicate note about #7930.

Evidence:

  • tools/interrupt.py:34 defines _interrupted_threads: set[int] guarded by _lock, rather than using one global threading.Event as the source of truth.
  • tools/interrupt.py:39 implements set_interrupt(active, thread_id=None), targeting either the supplied thread ident or the current thread.
  • tools/interrupt.py:62 implements is_interrupted() by checking only the current thread ident in _interrupted_threads, so unrelated threads do not observe each other's interrupt state.
  • tools/interrupt.py:73 keeps the legacy _interrupt_event import path as a proxy over the per-thread helpers.
  • tests/run_agent/test_interrupt_propagation.py:169 covers the core isolation case: interrupting thread A must not interrupt thread B.
  • The prior fix landed in dfc820345d4e49d16fd70cdea2b22d1736229ad9 (fix: scope tool interrupt signal per-thread to prevent cross-session leaks (#7930)), first contained in v2026.4.13.

Closing this PR as already implemented on main.

@teknium1 teknium1 closed this Jun 11, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists sweeper:implemented-on-main Sweeper: behavior already present on current main type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants