[MindOfqc (MindOfqc)]

Problem

RWLock documents writer priority ("Writers are prioritized when waiting", lib/crewai/src/crewai/utilities/rw_lock.py:14-17), but r_acquire only checks _writer, so a writer that is already blocked inside _cond.wait() is invisible to readers arriving afterwards. Under a steady stream of readers _readers > 0 never drops back to zero in w_acquire, and the writer can be starved indefinitely.

RWLock backs the event bus (events/event_bus.py:153), the execution event record (state/event_record.py:108) and the agent cache handler, so the starved writer is a real write out of the hot path (handler registration, event node insertion).

Fixes #8773.

Change

• w_acquire counts itself in _waiting_writers before blocking and decrements in a finally.
• r_acquire also waits while _waiting_writers > 0, so a queued writer is not overtaken.
• A thread that already holds a read lock re-enters without waiting (per-thread read depth): otherwise a reader that acquires twice — as the existing test_nested_read_locks_same_thread documents — would deadlock against its own lock as soon as a writer queues up.

Tests

Added to lib/crewai/tests/utilities/events/test_rw_lock.py:

• test_waiting_writer_blocks_new_readers — a reader that arrives while a writer waits must not acquire the lock.

• test_nested_read_locks_work_while_a_writer_waits — the re-entrant read path must keep working once a writer is queued.

• Red (HEAD): 1 failed, 13 passed — AssertionError: a reader that arrives while a writer waits must not acquire the lock

• Green: test_rw_lock.py → 14 passed

• Regression (tests/utilities/events, tests/events, test_event_record.py): 269 passed

Prepared with an AI coding agent.

---

Note

Medium Risk
Touches shared concurrency primitives backing the event bus and cache handler; behavior change is narrowly scoped and covered by new thread tests.

Overview
Aligns RWLock behavior with its documented writer priority by tracking queued writers and blocking new readers while a writer is waiting, which prevents indefinite writer starvation when readers keep arriving.

w_acquire increments _waiting_writers before waiting and clears it in a finally. r_acquire waits when _writer or _waiting_writers > 0, except on the same thread via thread-local _read_depth so nested read locks do not deadlock once a writer is queued.

Adds regression tests for reader-vs-waiting-writer ordering and nested reads under a queued writer.

^{Reviewed by Cursor Bugbot for commit 0ae4a43. Bugbot is set up for automated code reviews on this repo. Configure here.}

[strix-security]

Strix Security Review

Warning

This pull request has 81 commits after the last Strix review (5ca6ff0). Strix has not reviewed these changes.
Automatic review on push is off for this repository. To review the latest changes, tag @strix-security in a comment, or turn on re-review on push.

✅ No open security findings.

PR summary: Fixes an RWLock liveness bug by making readers wait behind a writer that is already queued (`_waiting_writers`), and adding per-thread read-depth tracking so a thread that already holds a read lock can re-enter it instead of deadlocking behind that writer. Two regression tests accompany the change.

Review summary

Reviewed the RWLock writer-priority change (lib/crewai/src/crewai/utilities/rw_lock.py) and its new tests. The two new mechanisms — a _waiting_writers counter that makes arriving readers wait behind a queued writer, and a per-thread _read_depth fast path so a thread already holding a read lock can re-enter instead of deadlocking against the writer it queued — were traced through every production call site (event_bus.py, state/event_record.py, agents/cache/cache_handler.py, utilities/streaming.py). All acquires and releases go through the r_locked()/w_locked() context managers on the acquiring thread, no read section spans an await or a cross-thread rendezvous, and mutual exclusion between readers and a writer is preserved, so the change introduces no lock-state divergence or exclusion break reachable by an attacker. Remaining observations are interrupt-only liveness nits (a KeyboardInterrupt landing in a narrow instruction window, or the counter increment sitting just outside the try that decrements it) that cross no security boundary; they were validated and rejected as findings, and are worth at most a one-line hardening note to the author.

Ruled out (4)

• Can an aborted queued writer leave readers permanently parked on a lock no one holds?
  The abort path can only be reached by a KeyboardInterrupt in the writer thread's `Condition.wait()`, and reproduction showed the state normally self-heals on the next reader, so this is a self-inflicted local-operator liveness nit, not an attacker-triggered denial of service.
• Can the new per-thread read depth desync from `_readers` and let a reader run concurrently with a writer?
  The divergence was only reproducible by injecting an asynchronous exception inside the lock's own critical section; no untrusted input drives it, and the equivalent leaked-reader-permit class already exists in the pre-PR code.
• Can a leaked `_waiting_writers` increment permanently wedge the lock for all readers?
  Landing an exception at that single instruction requires an in-process async exception from the process owner, that same window also leaves the Condition's lock held, and half a million injected interrupts never reproduced a leak, matching pre-existing interruption behaviour.
• Can a cross-thread or unbalanced `r_release` poison a thread's depth and break writer exclusion?
  Every release in the repository happens through `r_locked()` in the same frame and thread that acquired it, so the precondition has no in-repo or attacker-controlled path and it is only an API-contract hardening observation.

_{Review 1 on this PR · 5ca6ff0 · requested Oct 29 12:56 UTC when the PR was opened}

---

Reviewed by Strix
_{Re-run review · Configure security review settings}

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

^{Reviewed by Cursor Bugbot for commit 3875309. Configure here.}

[cursor]

Aborted writer starves waiting readers

Medium Severity

w_acquire drops _waiting_writers in finally when Condition.wait() raises, but never calls notify_all. Readers already blocked in r_acquire on that counter stay asleep. New readers can keep _readers above zero, so those waiters may never wake.

 

^{Reviewed by Cursor Bugbot for commit 3875309. Configure here.}