[patrickswedish]

What was wrong?

Closes #3867

In web3/_utils/async_caching.py, async_lock used:

try:
    await loop.run_in_executor(thread_pool, lock.acquire)
    yield
finally:
    if lock.locked():
        lock.release()

When an async request was cancelled (such as from a client-side asyncio.wait_for(...) timeout or task cancellation) while waiting on lock.acquire:

1. finally: was immediately triggered with acquired == False. If another concurrent task currently held the lock, if lock.locked(): lock.release() released the lock out from under the active holder, breaking mutual exclusion.
2. The background thread in thread_pool continued executing lock.acquire(). Once the active holder released, the orphaned thread acquired the lock, but because the coroutine had already exited, nobody would ever release it, permanently deadlocking HTTPSessionManager._lock for the entire process.

How was it fixed?

• Updated async_lock to track acquired state and shield the executor future (asyncio.shield(fut)).
• On asyncio.CancelledError before acquisition completes, attached fut.add_done_callback(lambda _: lock.release()) so any background acquisition that finishes post-cancellation immediately releases the lock.
• In finally:, only release the lock if acquired == True.
• Added unit tests in tests/core/caching-utils/test_async_caching.py testing standard acquisition and concurrency cancellation under lock contention.
• Added release note newsfragment newsfragments/3867.bugfix.rst.

Todo:

• Clean up commit history
• Add or update documentation related to these changes
• Add entry to the release notes

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[itzKLAUS]

One regression-test suggestion for head e359e9be5e4d58061c125e5efd8481c48ea2c7c7: test_async_lock_already_done_future_cancellation_releases_lock currently cancels from inside the context body, after acquired = True and yield. It therefore tests body cancellation rather than the fut.done() branch in the cancellation handler.

That branch can be exercised deterministically by substituting a pending asyncio future for run_in_executor, completing the acquisition, and cancelling the waiter before it gets another event-loop turn:

@pytest.mark.asyncio
async def test_completed_acquisition_before_cancelled_awaiter_resumes(monkeypatch):
    loop = asyncio.get_running_loop()
    future = loop.create_future()
    submitted = asyncio.Event()
    lock = threading.Lock()
    entered = []

    def submit(*args):
        submitted.set()
        return future

    monkeypatch.setattr(loop, "run_in_executor", submit)

    async def waiter():
        async with async_lock(None, lock):
            entered.append(True)

    task = asyncio.create_task(waiter())
    await asyncio.wait_for(submitted.wait(), timeout=1)
    future.set_result(lock.acquire())
    task.cancel()  # No await between completing acquisition and cancelling.
    with pytest.raises(asyncio.CancelledError):
        await task
    assert not entered
    assert not lock.locked()

None is only a placeholder for the executor here; the patched submission method never touches it or starts a thread. The lock is initially free, so the synchronous acquisition cannot block.

An isolated Python 3.12 stdlib harness using this ordering passes against this PR head. Two additional helper-level checks also pass: cancelling a waiting task preserves the current owner's lock, and a delayed acquisition releases after cancellation. Those two checks fail against current main. This is focused helper verification with controlled futures, not a full package/integration-suite run. The extra case would document the pre-yield branch without relying on timing sleeps.

[patrickswedish]

Thank you for the thoughtful analysis and suggestion, @itzKLAUS!

I've added the deterministic regression test est_completed_acquisition_before_cancelled_awaiter_resumes to ests/core/caching-utils/test_async_caching.py to cleanly exercise the pre-yield \ut.done()\ branch in the cancellation handler without timing dependencies.

The branch has been rebased cleanly on latest \main, squashed into a single atomic commit (\90c9ab7), and verified locally with pytest and ruff.