Skip to content

InputMergeActor: fix deadlock#174

Merged
woutdenolf merged 1 commit into
mainfrom
173-inputmergeactor-lock-need-to-be-re-entrant
Jul 13, 2026
Merged

InputMergeActor: fix deadlock#174
woutdenolf merged 1 commit into
mainfrom
173-inputmergeactor-lock-need-to-be-re-entrant

Conversation

@woutdenolf

@woutdenolf woutdenolf commented Jul 10, 2026

Copy link
Copy Markdown
Member

An Ewoks ppf actor graph has an InputMergeActor in front of a PythonActor (see docs).

The deadlock can happen when the Ewoks task loops to itself so InputMergeActor triggers PythonActor which trigger the same InputMergeActor

InputMergeActor._execute (iteration N)      -- with self._lock:  ← lock acquired here, not yet released
  → _trigger_downstream
    → PythonActor.trigger → _execute
      → self.pool.apply_async(...)           (PythonActor)
        → ScalingPool.apply_async(...)
          → ThreadPool.apply_async(...)
            → future.add_done_callback(cb)   ← if worker already done, `cb` runs HERE, same thread, same stack

Of course this is very unlikely but could happen

        future = self._pool.submit(...)
        future.add_done_callback(cb)   # if worker already finished, `cb` runs in the current call stack

The fix is to make the lock in InputMergeActor re-entrant. Since this is the same call stack we don't have to worry these calls can happen concurrently, as opposed to calls from different upstream actors.

@woutdenolf woutdenolf linked an issue Jul 10, 2026 that may be closed by this pull request
@woutdenolf woutdenolf self-assigned this Jul 10, 2026
@woutdenolf
woutdenolf requested a review from a team July 10, 2026 14:18
@woutdenolf
woutdenolf marked this pull request as draft July 12, 2026 13:13
@woutdenolf
woutdenolf removed the request for review from a team July 12, 2026 13:14
@woutdenolf
woutdenolf force-pushed the 173-inputmergeactor-lock-need-to-be-re-entrant branch from e9b136d to 7c30c65 Compare July 12, 2026 15:30
@woutdenolf
woutdenolf marked this pull request as ready for review July 12, 2026 15:31
@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@woutdenolf
woutdenolf force-pushed the 173-inputmergeactor-lock-need-to-be-re-entrant branch from 7c30c65 to 6e081c7 Compare July 12, 2026 15:39
@woutdenolf
woutdenolf requested a review from a team July 12, 2026 15:40
Comment thread src/ewoksppf/bindings.py
Comment on lines 244 to 247

self._lock = threading.Lock()
# Re-entrant in case downstream actor triggers this actor in the same call stack.
self._lock = threading.RLock()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the fix. The rest are unit tests.

@woutdenolf
woutdenolf merged commit 3a76953 into main Jul 13, 2026
14 checks passed
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.

InputMergeActor: lock need to be re-entrant

2 participants