Skip to content

Use RLock instead of Lock to prevent thread deadlock#1008

Merged
Kludex merged 1 commit into
pydantic:mainfrom
mbeijen:fix/rlock-thread-deadlock
Jun 2, 2026
Merged

Use RLock instead of Lock to prevent thread deadlock#1008
Kludex merged 1 commit into
pydantic:mainfrom
mbeijen:fix/rlock-thread-deadlock

Conversation

@mbeijen
Copy link
Copy Markdown
Contributor

@mbeijen mbeijen commented Jun 1, 2026

Summary

A thread that already holds _optional_thread_lock on (Async)ConnectionPool can re-enter the lock in nested call paths. threading.Lock deadlocks on a second acquire from the same thread; threading.RLock allows re-entrant acquisition and resolves the hang.

This PR:

  • Swaps threading.Lock() for threading.RLock() in the sync variant. The async variant remains a no-op.
    - Renames ThreadLockThreadRLock and AsyncThreadLockAsyncThreadRLock in httpcore2/_synchronization.py to make the lock semantics explicit. The names are private (module is _synchronization), so this is not a public API change.
    - Updates the two import / usage sites in _async/connection_pool.py and _sync/connection_pool.py.

Ported from encode/httpcore#1003 (Zenulous), via codeberg.org/httpxyz/httpcorexyz@3058e2d.


Note: this change was prepared with AI assistance (Claude Code).

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 3 files

Re-trigger cubic

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jun 1, 2026

Merging this PR will not alter performance

✅ 15 untouched benchmarks
⏩ 7 skipped benchmarks1


Comparing mbeijen:fix/rlock-thread-deadlock (3735a33) with main (04f3804)

Open in CodSpeed

Footnotes

  1. 7 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.



class ThreadLock:
class ThreadRLock:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need to rename the classes tho?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, that was only to make it more explicit. I've updated the PR now to make the change more minimal ;-)

A thread that already holds `_optional_thread_lock` can re-enter it in
nested call paths. `threading.Lock` deadlocks on a second acquire from
the same thread; `threading.RLock` allows re-entrant acquisition.

Ported from encode/httpcore#1003 (Zenulous), via
codeberg.org/httpxyz/httpcorexyz@3058e2d.

Co-Authored-By: Zenulous <zen@zenulous.com>
@mbeijen mbeijen force-pushed the fix/rlock-thread-deadlock branch from e4339a3 to 3735a33 Compare June 2, 2026 05:46
mbeijen added a commit to mbeijen/httpx2 that referenced this pull request Jun 2, 2026
@Kludex Kludex merged commit b0710d5 into pydantic:main Jun 2, 2026
11 checks passed
@Kludex
Copy link
Copy Markdown
Member

Kludex commented Jun 2, 2026

Thanks!

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.

2 participants