Skip to content

[NOGIL] Restrict sharing of Consumer instances across threads - #2322

Open
Ojasva Jain (ojasvajain) wants to merge 6 commits into
dev_producer_no_gilfrom
dev_consumer_no_gil
Open

[NOGIL] Restrict sharing of Consumer instances across threads#2322
Ojasva Jain (ojasvajain) wants to merge 6 commits into
dev_producer_no_gilfrom
dev_consumer_no_gil

Conversation

@ojasvajain

@ojasvajain Ojasva Jain (ojasvajain) commented Aug 6, 2026

Copy link
Copy Markdown
Member

Introduces a reentrancy gate for the sync Consumer and AIOConsumer that rejects illegal concurrent access to a single Consumer instance from a different caller, while still allowing legitimate re-entrant calls — e.g. a rebalance or commit callback calling back into the Consumer/AIOConsumer that triggered it (on_assign calling assign(), on_commit calling commit(), etc.).

A unified gate_owner/gate_depth mechanism (Handle_gate_enter()/Handle_gate_exit() in Consumer.c) is shared between both sync and async consumer types, and every gated method resolves its own caller identity internally.
For sync consumer, calling thread ID serves as the identity while for the async consumer an ID is generated before dispatching each call and it gets carried over through the task chain using context variables. For async consumers, this is required as callbacks may get scheduled on a different worker thread, so a thread ID can not be used as an ID.

Backward compatibility: gate enforcement is controlled by CFL_CONSUMER_GATE_ENABLED (confluent_kafka.h) and only applies to Python versions this project has never shipped a wheel for — 3.15+ (any build), or 3.14 built free-threaded. On every version already shipped (≤3.14, GIL-based), the gate is compiled out entirely, so existing users see zero behavior change on upgrade.

TODO -> Add unit test cases for AIOConsumer

@confluent-cla-assistant

Copy link
Copy Markdown

🎉 All Contributor License Agreements have been signed. Ready to merge.
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.

…s protection

Introduce a unified gate_owner/gate_depth mechanism in Consumer.c that
rejects concurrent cross-caller access to a single Consumer instance
while still allowing legitimate re-entrant calls (e.g. a rebalance/commit
callback calling back into the Consumer that triggered it). The gate is
shared between the sync Consumer (thread ID as identity) and AIOConsumer
(a Python-generated logical-caller identity, since the owning caller may
move across ThreadPoolExecutor worker threads).

Gate enforcement (CFL_CONSUMER_GATE_ENABLED in confluent_kafka.h) only
applies to Python versions we've never shipped a wheel for -- 3.15+, or
3.14 built free-threaded -- so existing users on <=3.14 GIL-based Python
see zero behavior change on upgrade.

Also fixes Consumer__pause_internal/Consumer__resume_internal, which
never checked self->rk before calling into librdkafka, causing
pause()/resume() after close() to segfault instead of raising
RuntimeError.
Comment on lines 147 to +196
@@ -186,6 +190,10 @@ def worker(producer, stop_event):
def test_close_races_abort_transaction():
"""close() concurrent with abort_transaction() on another thread."""

# TODO NOGIL: move to tests/integration -- abort_transaction() needs a
# real transaction coordinator to reach the race being tested; against
# the unreachable localhost:9092 used here it fails with a genuine
# _STATE KafkaException instead of the expected RuntimeError.

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.

Will revisit this in a separate PR that I plan to raise for Transactional Producer related audit.

@sonarqube-confluent

Copy link
Copy Markdown

Quality Gate failed Quality Gate failed

Failed conditions
5.6% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant