diff --git a/src/confluent_kafka/src/Producer.c b/src/confluent_kafka/src/Producer.c index 0bfc810b8..8a104ee51 100644 --- a/src/confluent_kafka/src/Producer.c +++ b/src/confluent_kafka/src/Producer.c @@ -27,6 +27,12 @@ #include "confluent_kafka.h" +#ifdef _WIN32 +#include +#else +#include +#endif + /** * @brief KNOWN ISSUES @@ -296,12 +302,11 @@ Producer_produce(Handle *self, PyObject *args, PyObject *kwargs) { if (!dr_cb || dr_cb == Py_None) dr_cb = self->u.Producer.default_dr_cb; - if (!self->rk) { + if (!Handle_enter_rk_use(self)) { #ifdef RD_KAFKA_V_HEADERS if (rd_headers) rd_kafka_headers_destroy(rd_headers); #endif - PyErr_SetString(PyExc_RuntimeError, ERR_MSG_PRODUCER_CLOSED); return NULL; } @@ -323,6 +328,8 @@ Producer_produce(Handle *self, PyObject *args, PyObject *kwargs) { key_len, msgstate); #endif + Handle_exit_rk_use(self); + if (err) { if (msgstate) Producer_msgstate_destroy(msgstate); @@ -421,12 +428,13 @@ static PyObject *Producer_poll(Handle *self, PyObject *args, PyObject *kwargs) { if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|d", kws, &tmout)) return NULL; - if (!self->rk) { - PyErr_SetString(PyExc_RuntimeError, ERR_MSG_PRODUCER_CLOSED); + if (!Handle_enter_rk_use(self)) return NULL; - } r = Producer_poll0(self, cfl_timeout_ms(tmout)); + + Handle_exit_rk_use(self); + if (r == -1) return NULL; @@ -469,10 +477,8 @@ Producer_flush(Handle *self, PyObject *args, PyObject *kwargs) { if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|d", kws, &tmout)) return NULL; - if (!self->rk) { - PyErr_SetString(PyExc_RuntimeError, ERR_MSG_PRODUCER_CLOSED); + if (!Handle_enter_rk_use(self)) return NULL; - } total_timeout_ms = cfl_timeout_ms(tmout); CallState_begin(self, &cs); @@ -507,6 +513,7 @@ Producer_flush(Handle *self, PyObject *args, PyObject *kwargs) { * interruptibility) */ chunk_count++; if (check_signals_between_chunks(self, &cs)) { + Handle_exit_rk_use(self); return NULL; /* Signal detected */ } @@ -524,12 +531,16 @@ Producer_flush(Handle *self, PyObject *args, PyObject *kwargs) { } } - if (!CallState_end(self, &cs)) + if (!CallState_end(self, &cs)) { + Handle_exit_rk_use(self); return NULL; + } if (err) /* Get the queue length on error (timeout) */ qlen = rd_kafka_outq_len(self->rk); + Handle_exit_rk_use(self); + return cfl_PyInt_FromInt(qlen); } @@ -542,6 +553,37 @@ Producer_close(Handle *self, PyObject *args, PyObject *kwargs) { if (!self->rk) Py_RETURN_TRUE; + /* Only one concurrent close() can destroy rk, otherwise, + * two threads could both reach rd_kafka_destroy() on the + * same handle (a double-free). The losing thread(s) wait for the + * winner to finish and then return True, same as a normal close(), + * rather than racing it. */ + if (!atomic_int_cas(&self->closing, 0, 1)) { + while (self->rk) { + CallState_begin(self, &cs); +#ifdef _WIN32 + Sleep(100); +#else + usleep(100000); +#endif + CallState_end(self, &cs); + } + Py_RETURN_TRUE; + } + + /* Signal in-flight calls to stop, and wait for them to finish + * using self->rk before destroying it -- see Handle_enter_rk_use(). + * New calls will see `closing` and fail with ERR_MSG_PRODUCER_CLOSED. */ + while (atomic_int_get(&self->active_calls) > 0) { + CallState_begin(self, &cs); +#ifdef _WIN32 + Sleep(100); +#else + usleep(100000); +#endif + CallState_end(self, &cs); + } + CallState_begin(self, &cs); /* Flush any pending messages (wait indefinitely to ensure delivery) */ @@ -817,10 +859,8 @@ Producer_produce_batch(Handle *self, PyObject *args, PyObject *kwargs) { return cfl_PyInt_FromInt(0); } - if (!self->rk) { - PyErr_SetString(PyExc_RuntimeError, ERR_MSG_PRODUCER_CLOSED); + if (!Handle_enter_rk_use(self)) return NULL; - } /* Allocate arrays for librdkafka messages and msgstates */ rkmessages = calloc(message_cnt, sizeof(*rkmessages)); @@ -849,6 +889,8 @@ Producer_produce_batch(Handle *self, PyObject *args, PyObject *kwargs) { messages_list, rkt, partition, rkmessages, msgstates, message_cnt); cleanup: + Handle_exit_rk_use(self); + /* Cleanup resources */ if (rkt) rd_kafka_topic_destroy(rkt); @@ -871,21 +913,22 @@ static PyObject *Producer_init_transactions(Handle *self, PyObject *args) { if (!PyArg_ParseTuple(args, "|d", &tmout)) return NULL; - if (!self->rk) { - PyErr_SetString(PyExc_RuntimeError, ERR_MSG_PRODUCER_CLOSED); + if (!Handle_enter_rk_use(self)) return NULL; - } CallState_begin(self, &cs); error = rd_kafka_init_transactions(self->rk, cfl_timeout_ms(tmout)); if (!CallState_end(self, &cs)) { + Handle_exit_rk_use(self); if (error) /* Ignore error in favour of callstate exception */ rd_kafka_error_destroy(error); return NULL; } + Handle_exit_rk_use(self); + if (error) { cfl_PyErr_from_error_destroy(error); return NULL; @@ -897,13 +940,13 @@ static PyObject *Producer_init_transactions(Handle *self, PyObject *args) { static PyObject *Producer_begin_transaction(Handle *self) { rd_kafka_error_t *error; - if (!self->rk) { - PyErr_SetString(PyExc_RuntimeError, ERR_MSG_PRODUCER_CLOSED); + if (!Handle_enter_rk_use(self)) return NULL; - } error = rd_kafka_begin_transaction(self->rk); + Handle_exit_rk_use(self); + if (error) { cfl_PyErr_from_error_destroy(error); return NULL; @@ -924,16 +967,17 @@ static PyObject *Producer_send_offsets_to_transaction(Handle *self, if (!PyArg_ParseTuple(args, "OO|d", &offsets, &metadata, &tmout)) return NULL; - if (!self->rk) { - PyErr_SetString(PyExc_RuntimeError, ERR_MSG_PRODUCER_CLOSED); + if (!Handle_enter_rk_use(self)) return NULL; - } - if (!(c_offsets = py_to_c_parts(offsets))) + if (!(c_offsets = py_to_c_parts(offsets))) { + Handle_exit_rk_use(self); return NULL; + } if (!(cgmd = py_to_c_cgmd(metadata))) { rd_kafka_topic_partition_list_destroy(c_offsets); + Handle_exit_rk_use(self); return NULL; } @@ -946,11 +990,14 @@ static PyObject *Producer_send_offsets_to_transaction(Handle *self, rd_kafka_topic_partition_list_destroy(c_offsets); if (!CallState_end(self, &cs)) { + Handle_exit_rk_use(self); if (error) /* Ignore error in favour of callstate exception */ rd_kafka_error_destroy(error); return NULL; } + Handle_exit_rk_use(self); + if (error) { cfl_PyErr_from_error_destroy(error); return NULL; @@ -967,21 +1014,22 @@ static PyObject *Producer_commit_transaction(Handle *self, PyObject *args) { if (!PyArg_ParseTuple(args, "|d", &tmout)) return NULL; - if (!self->rk) { - PyErr_SetString(PyExc_RuntimeError, ERR_MSG_PRODUCER_CLOSED); + if (!Handle_enter_rk_use(self)) return NULL; - } CallState_begin(self, &cs); error = rd_kafka_commit_transaction(self->rk, cfl_timeout_ms(tmout)); if (!CallState_end(self, &cs)) { + Handle_exit_rk_use(self); if (error) /* Ignore error in favour of callstate exception */ rd_kafka_error_destroy(error); return NULL; } + Handle_exit_rk_use(self); + if (error) { cfl_PyErr_from_error_destroy(error); return NULL; @@ -998,21 +1046,22 @@ static PyObject *Producer_abort_transaction(Handle *self, PyObject *args) { if (!PyArg_ParseTuple(args, "|d", &tmout)) return NULL; - if (!self->rk) { - PyErr_SetString(PyExc_RuntimeError, ERR_MSG_PRODUCER_CLOSED); + if (!Handle_enter_rk_use(self)) return NULL; - } CallState_begin(self, &cs); error = rd_kafka_abort_transaction(self->rk, cfl_timeout_ms(tmout)); if (!CallState_end(self, &cs)) { + Handle_exit_rk_use(self); if (error) /* Ignore error in favour of callstate exception */ rd_kafka_error_destroy(error); return NULL; } + Handle_exit_rk_use(self); + if (error) { cfl_PyErr_from_error_destroy(error); return NULL; @@ -1034,10 +1083,8 @@ static void *Producer_purge(Handle *self, PyObject *args, PyObject *kwargs) { &in_flight, &blocking)) return NULL; - if (!self->rk) { - PyErr_SetString(PyExc_RuntimeError, ERR_MSG_PRODUCER_CLOSED); + if (!Handle_enter_rk_use(self)) return NULL; - } if (in_queue) purge_strategy = RD_KAFKA_PURGE_F_QUEUE; @@ -1048,6 +1095,8 @@ static void *Producer_purge(Handle *self, PyObject *args, PyObject *kwargs) { err = rd_kafka_purge(self->rk, purge_strategy); + Handle_exit_rk_use(self); + if (err) { cfl_PyErr_Format(err, "Purge failed: %s", rd_kafka_err2str(err)); @@ -1403,9 +1452,23 @@ static PyMethodDef Producer_methods[] = { static Py_ssize_t Producer__len__(Handle *self) { - if (!self->rk) + Py_ssize_t len; + + /* __len__ must never raise, so we can't use Handle_enter_rk_use() + * (which sets an exception on failure) -- fall back to returning 0, + * , if the Handle is closed/closing. */ + if (atomic_int_get(&self->closing) || !self->rk) + return 0; + atomic_int_inc(&self->active_calls); + if (atomic_int_get(&self->closing) || !self->rk) { + atomic_int_dec(&self->active_calls); return 0; - return rd_kafka_outq_len(self->rk); + } + + len = rd_kafka_outq_len(self->rk); + + atomic_int_dec(&self->active_calls); + return len; } diff --git a/src/confluent_kafka/src/confluent_kafka.c b/src/confluent_kafka/src/confluent_kafka.c index 7ec5dd781..29d5dd9ab 100644 --- a/src/confluent_kafka/src/confluent_kafka.c +++ b/src/confluent_kafka/src/confluent_kafka.c @@ -2431,7 +2431,8 @@ int wait_for_oauth_token_set(Handle *h) { int max_wait_sec = 10; int retry_interval_sec = 1; /* Check every 1 sec */ int elapsed_sec = 0; - while (!h->oauth_token_set && elapsed_sec < max_wait_sec) { + while (!atomic_int_get(&h->oauth_token_set) && + elapsed_sec < max_wait_sec) { CallState cs; CallState_begin(h, &cs); #ifdef _WIN32 @@ -2443,7 +2444,7 @@ int wait_for_oauth_token_set(Handle *h) { elapsed_sec += retry_interval_sec; } - if (!h->oauth_token_set) { + if (!atomic_int_get(&h->oauth_token_set)) { /* Token timeout. Don't tear down here — each _init knows * whether to call rd_kafka_destroy() or * rd_kafka_share_destroy() for what it allocated. */ @@ -2529,7 +2530,7 @@ oauth_cb(rd_kafka_t *rk, const char *oauthbearer_config, void *opaque) { PyErr_Format(PyExc_ValueError, "%s", err_msg); goto fail; } - h->oauth_token_set = 1; + atomic_int_set(&h->oauth_token_set, 1); goto done; fail: @@ -3314,6 +3315,43 @@ int CallState_end(Handle *h, CallState *cs) { } +/** + * @brief Mark self->rk as in-use by the calling thread, so that close() + * (running concurrently on another thread) will wait for us before + * destroying it. Must be called before CallState_begin(). + * + * @returns 1 if self->rk is safe to use (active_calls has been + * incremented; caller must call Handle_exit_rk_use() on every + * return path), or 0 with ERR_MSG_PRODUCER_CLOSED set if the + * Handle is closed/closing (nothing to undo). + * + * @warning Not re-entrant: don't call from a method that's already + * between its own Handle_enter_rk_use()/Handle_exit_rk_use(). + */ +int Handle_enter_rk_use(Handle *h) { + if (atomic_int_get(&h->closing) || !h->rk) { + PyErr_SetString(PyExc_RuntimeError, ERR_MSG_PRODUCER_CLOSED); + return 0; + } + atomic_int_inc(&h->active_calls); + /* close() may have started between our check above and the + * increment; re-check now that we're counted. */ + if (atomic_int_get(&h->closing) || !h->rk) { + atomic_int_dec(&h->active_calls); + PyErr_SetString(PyExc_RuntimeError, ERR_MSG_PRODUCER_CLOSED); + return 0; + } + return 1; +} + +/** + * @brief Counterpart to Handle_enter_rk_use(): call on every return path + * after a successful Handle_enter_rk_use(). + */ +void Handle_exit_rk_use(Handle *h) { + atomic_int_dec(&h->active_calls); +} + /** * @brief Get the current thread's CallState and re-locks the GIL. */ diff --git a/src/confluent_kafka/src/confluent_kafka.h b/src/confluent_kafka/src/confluent_kafka.h index fc204ce02..34746d845 100644 --- a/src/confluent_kafka/src/confluent_kafka.h +++ b/src/confluent_kafka/src/confluent_kafka.h @@ -35,6 +35,55 @@ #endif +/** + * @brief Minimal portable atomic-int primitives. + * + * Not : this project's build does not pin a C standard + * version, and MSVC's C11 support is version- and flag-gated, + * so it cannot be relied on across this project's actual build matrix. + * + * Modeled on librdkafka's own rdatomic.h: GCC/Clang __atomic_* builtins + * on non-Windows, Interlocked* on Windows. + */ +#if defined(_MSC_VER) +typedef volatile LONG atomic_int_t; + +#define atomic_int_init(p, v) (*(p) = (v)) +#define atomic_int_inc(p) InterlockedIncrement((p)) +#define atomic_int_dec(p) InterlockedDecrement((p)) +#define atomic_int_get(p) InterlockedCompareExchange((p), 0, 0) +#define atomic_int_set(p, v) InterlockedExchange((p), (v)) + +/** + * @brief Atomic compare-and-swap: if *p == expected, set *p = desired and + * return 1; otherwise leave *p unchanged and return 0. + */ +static __inline int atomic_int_cas(atomic_int_t *p, LONG expected, + LONG desired) { + return InterlockedCompareExchange(p, desired, expected) == expected; +} + +#else /* gcc / clang */ +typedef int atomic_int_t; + +#define atomic_int_init(p, v) __atomic_store_n((p), (v), __ATOMIC_SEQ_CST) +#define atomic_int_inc(p) __atomic_add_fetch((p), 1, __ATOMIC_SEQ_CST) +#define atomic_int_dec(p) __atomic_sub_fetch((p), 1, __ATOMIC_SEQ_CST) +#define atomic_int_get(p) __atomic_load_n((p), __ATOMIC_SEQ_CST) +#define atomic_int_set(p, v) __atomic_store_n((p), (v), __ATOMIC_SEQ_CST) + +/** + * @brief Atomic compare-and-swap: if *p == expected, set *p = desired and + * return 1; otherwise leave *p unchanged and return 0. + */ +static inline int atomic_int_cas(atomic_int_t *p, int expected, int desired) { + return __atomic_compare_exchange_n(p, &expected, desired, + 0 /* strong */, __ATOMIC_SEQ_CST, + __ATOMIC_SEQ_CST); +} +#endif + + /** * @brief confluent-kafka-python version, must match that of pyproject.toml. */ @@ -241,7 +290,13 @@ typedef struct { PyObject *logger; PyObject *oauth_cb; - int oauth_token_set; + atomic_int_t oauth_token_set; + + /* Protects self->rk from being freed by close() while another method + * is still using it. See Handle_enter_rk_use()/Handle_exit_rk_use() + * in confluent_kafka.c. */ + atomic_int_t active_calls; + atomic_int_t closing; union { /** @@ -350,6 +405,17 @@ void CallState_begin(Handle *h, CallState *cs); */ int CallState_end(Handle *h, CallState *cs); +/** + * @brief Mark self->rk as in-use by the calling thread. + * @returns 1 if safe to use (caller must call Handle_exit_rk_use() on every + * return path), 0 with ERR_MSG_PRODUCER_CLOSED set otherwise. + */ +int Handle_enter_rk_use(Handle *h); +/** + * @brief Counterpart to Handle_enter_rk_use(). + */ +void Handle_exit_rk_use(Handle *h); + /** * @brief Get the current thread's CallState and re-locks the GIL. */ diff --git a/tests/concurrency/__init__.py b/tests/concurrency/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/concurrency/_subprocess_isolation.py b/tests/concurrency/_subprocess_isolation.py new file mode 100644 index 000000000..5f1cad234 --- /dev/null +++ b/tests/concurrency/_subprocess_isolation.py @@ -0,0 +1,67 @@ +#!/usr/bin/env python +# +# Copyright 2026 Confluent Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Shared test infrastructure for tests/concurrency/. + +Tests here deliberately race Producer/Consumer methods against each other, +so a real regression can segfault the process instead of just failing an +assertion. subprocess_isolated() below runs a decorated test in a fresh +`python -m pytest` subprocess, so a crash only fails that one test instead +of taking down the whole suite. +""" + +import functools +import os +import subprocess +import sys + +_SUBPROCESS_MARKER_ENV = "_CONCURRENCY_TESTS_SUBPROCESS" +_SUBPROCESS_TIMEOUT_SECONDS = 120 + + +def subprocess_isolated(test_func): + """ + Run `test_func` in a fresh `python -m pytest` subprocess instead of + in-process. A clean run passes normally; a crash (e.g. segfault) shows + up as a non-zero/negative subprocess return code, which is turned into + a normal assertion failure here -- so it fails only this test rather + than taking down the whole run. + """ + + @functools.wraps(test_func) + def wrapper(*args, **kwargs): + if os.environ.get(_SUBPROCESS_MARKER_ENV): + # Already inside the re-invoked subprocess: run the real body. + return test_func(*args, **kwargs) + + test_file = sys.modules[test_func.__module__].__file__ + node_id = f"{os.path.relpath(test_file)}::{test_func.__name__}" + env = dict(os.environ, **{_SUBPROCESS_MARKER_ENV: "1"}) + + result = subprocess.run( + [sys.executable, "-m", "pytest", "-p", "no:cacheprovider", "-q", node_id], + capture_output=True, + text=True, + timeout=_SUBPROCESS_TIMEOUT_SECONDS, + env=env, + ) + assert result.returncode == 0, ( + f"{test_func.__name__} crashed/failed in subprocess " + f"(returncode={result.returncode}):\nstdout:\n{result.stdout}\nstderr:\n{result.stderr}" + ) + + return wrapper diff --git a/tests/concurrency/test_producer_close_race.py b/tests/concurrency/test_producer_close_race.py new file mode 100644 index 000000000..6fff3f2d5 --- /dev/null +++ b/tests/concurrency/test_producer_close_race.py @@ -0,0 +1,298 @@ +#!/usr/bin/env python +# +# Copyright 2026 Confluent Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import threading +import time + +from confluent_kafka import Consumer, Producer, TopicPartition +from tests.concurrency._subprocess_isolation import subprocess_isolated + +############################################################################### +# Tests for races between Producer.close() and concurrent calls to +# other methods on the same Producer instance. +############################################################################### + +_PRODUCER_CONF = {'bootstrap.servers': 'localhost:9092', 'socket.timeout.ms': 10, 'message.timeout.ms': 10} +_TXN_PRODUCER_CONF = dict(_PRODUCER_CONF, **{'transactional.id': 'test-producer-close-race-txn'}) +ITERATIONS = 10 + + +def _race_close_against(worker, conf=None, num_workers=1): + """ + Run `worker(producer, stop_event)` on `num_workers` other threads while + calling close() from the main thread, repeated `iterations` times + against a fresh Producer each time. Every close() call (the main + thread's, once per iteration) must return True. + """ + errors = [] + close_results = [] + + for i in range(ITERATIONS): + producer = Producer(conf or _PRODUCER_CONF) + stop_event = threading.Event() + start_barrier = threading.Barrier(num_workers + 1) + + def run_worker(): + try: + start_barrier.wait() + worker(producer, stop_event) + except Exception as e: # noqa: BLE001 - want to see any exception, not just crashes + errors.append((i, e)) + + threads = [threading.Thread(target=run_worker) for _ in range(num_workers)] + for t in threads: + t.start() + + start_barrier.wait() + close_results.append(producer.close()) + + stop_event.set() + for t in threads: + t.join(timeout=10) + assert all(not t.is_alive() for t in threads), f"iteration {i}: a worker thread did not finish after close()" + + assert not errors, f"unexpected exceptions from worker threads: {errors}" + assert all(close_results), f"not every close() call returned True: {close_results}" + + +def _worker_produce(producer, stop_event): + while not stop_event.is_set(): + try: + producer.produce('mytopic', value=b'x') + except RuntimeError: + # Expected once close() has fully completed on this thread's + # view of self->rk; anything other than a clean RuntimeError + # (e.g. a segfault) is the bug this test is trying to catch. + break + + +def _worker_poll(producer, stop_event): + while not stop_event.is_set(): + try: + producer.poll(0) + except RuntimeError: + break + + +@subprocess_isolated +def test_close_races_produce(): + """close() concurrent with produce() on another thread.""" + _race_close_against(_worker_produce) + + +@subprocess_isolated +def test_close_races_multiple_producers_and_pollers(): + """ + close() concurrent with several threads calling produce()/poll() at + once, not just one. + """ + num_workers = 8 + _race_close_against(_worker_produce, num_workers=num_workers) + _race_close_against(_worker_poll, num_workers=num_workers) + + +@subprocess_isolated +def test_close_races_poll(): + """close() concurrent with poll() on another thread.""" + _race_close_against(_worker_poll) + + +@subprocess_isolated +def test_close_races_flush(): + """close() concurrent with flush() on another thread.""" + + def worker(producer, stop_event): + while not stop_event.is_set(): + try: + producer.flush(0.01) + except RuntimeError: + break + + _race_close_against(worker) + + +@subprocess_isolated +def test_close_races_produce_batch(): + """close() concurrent with produce_batch() on another thread.""" + + def worker(producer, stop_event): + messages = [{'value': b'x'}, {'value': b'y'}] + while not stop_event.is_set(): + try: + producer.produce_batch('mytopic', messages) + except RuntimeError: + break + + _race_close_against(worker) + + +@subprocess_isolated +def test_close_races_init_transactions(): + """close() concurrent with init_transactions() on another thread.""" + + def worker(producer, stop_event): + while not stop_event.is_set(): + try: + producer.init_transactions(0.05) + except RuntimeError: + break + except Exception: # noqa: BLE001 - librdkafka state/timeout errors are expected without a broker + pass + + _race_close_against(worker, conf=_TXN_PRODUCER_CONF) + + +@subprocess_isolated +def test_close_races_begin_transaction(): + """close() concurrent with begin_transaction() on another thread.""" + + def worker(producer, stop_event): + while not stop_event.is_set(): + try: + producer.begin_transaction() + except RuntimeError: + break + except Exception: # noqa: BLE001 - librdkafka state errors are expected without a broker + pass + + _race_close_against(worker, conf=_TXN_PRODUCER_CONF) + + +@subprocess_isolated +def test_close_races_commit_transaction(): + """close() concurrent with commit_transaction() on another thread.""" + + def worker(producer, stop_event): + while not stop_event.is_set(): + try: + producer.commit_transaction(0.05) + except RuntimeError: + break + except Exception: # noqa: BLE001 - librdkafka state/timeout errors are expected without a broker + pass + + _race_close_against(worker, conf=_TXN_PRODUCER_CONF) + + +@subprocess_isolated +def test_close_races_abort_transaction(): + """close() concurrent with abort_transaction() on another thread.""" + + def worker(producer, stop_event): + while not stop_event.is_set(): + try: + producer.abort_transaction(0.05) + except RuntimeError: + break + except Exception: # noqa: BLE001 - librdkafka state/timeout errors are expected without a broker + pass + + _race_close_against(worker, conf=_TXN_PRODUCER_CONF) + + +@subprocess_isolated +def test_close_races_send_offsets_to_transaction(): + """close() concurrent with send_offsets_to_transaction() on another thread.""" + + def worker(producer, stop_event): + # consumer_group_metadata() doesn't need a live broker connection. + consumer = Consumer({'group.id': 'test-producer-close-race', 'socket.timeout.ms': 10}) + metadata = consumer.consumer_group_metadata() + consumer.close() + + offsets = [TopicPartition('mytopic', 0, 1)] + while not stop_event.is_set(): + try: + producer.send_offsets_to_transaction(offsets, metadata, 0.05) + except RuntimeError: + break + except Exception: # noqa: BLE001 - librdkafka state/timeout errors are expected without a broker + pass + + _race_close_against(worker, conf=_TXN_PRODUCER_CONF) + + +@subprocess_isolated +def test_close_races_purge(): + """close() concurrent with purge() on another thread.""" + + def worker(producer, stop_event): + while not stop_event.is_set(): + try: + producer.purge() + except RuntimeError: + break + + _race_close_against(worker) + + +@subprocess_isolated +def test_close_races_close(): + """Multiple threads calling close() on the same Producer at once.""" + worker_close_results = [] + num_workers = 7 + + def worker(producer, stop_event): + worker_close_results.append(producer.close()) + + _race_close_against(worker, num_workers=num_workers) + + assert all(worker_close_results), f"not every worker close() call returned True: {worker_close_results}" + assert len(worker_close_results) == num_workers * ITERATIONS + + +############################################################################### +# close() blocks until an in-flight call finishes, +# rather than just not crashing while one is running. +############################################################################### + + +def test_close_waits_for_in_flight_call(): + """close() blocks until an in-flight poll() call finishes.""" + producer = Producer(_PRODUCER_CONF) + poll_started = threading.Event() + poll_duration = 10 + poll_finished_at = None + + def run_poll(): + nonlocal poll_finished_at + poll_started.set() + producer.poll(poll_duration) + poll_finished_at = time.monotonic() + + t = threading.Thread(target=run_poll) + t.start() + poll_started.wait() + time.sleep(2) + + close_start = time.monotonic() + producer.close() + close_end = time.monotonic() + + t.join(timeout=10) + assert not t.is_alive(), "poll() thread did not finish after close()" + assert poll_finished_at is not None, "poll() never finished" + + # close() should have waited for close to the actual remaining poll() + # duration + close_duration = close_end - close_start + remaining_poll_duration = poll_finished_at - close_start + assert close_duration >= remaining_poll_duration * 0.9, ( + f"close() took {close_duration:.2f}s but the in-flight poll() call " + f"was still going to run for {remaining_poll_duration:.2f}s more " + f"-- close() returned too early relative to what it should have " + f"waited for" + ) diff --git a/tests/integration/producer/test_concurrency.py b/tests/integration/producer/test_concurrency.py new file mode 100644 index 000000000..b7e95d3c2 --- /dev/null +++ b/tests/integration/producer/test_concurrency.py @@ -0,0 +1,355 @@ +#!/usr/bin/env python +# +# Copyright 2026 Confluent Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import inspect +import threading +import time +from uuid import uuid1 + +from confluent_kafka import KafkaError, KafkaException +from tests.common import TestConsumer + + +def called_by(): + return inspect.stack()[1].function + + +def prefixed_error_cb(prefix): + def error_cb(err): + """Reports global/generic errors to aid in troubleshooting test failures.""" + print("[{}]: {}".format(prefix, err)) + + return error_cb + + +class TestCloseRaceDelivery: + def test_close_delivers_in_flight_messages(self, kafka_cluster): + """ + Messages produced just before/during a concurrent close() are genuinely delivered, + not silently dropped. + """ + topic = kafka_cluster.create_topic_and_wait_propogation("test_close_delivery") + producer = kafka_cluster.producer({'error_cb': prefixed_error_cb('test_close_delivers_in_flight_messages')}) + + delivered = [] + delivery_errors = [] + + def on_delivery(err, msg): + if err: + delivery_errors.append(err) + else: + delivered.append(msg) + + produced_count = 0 + closed_runtime_error = None + + def produce_loop(): + nonlocal produced_count, closed_runtime_error + while True: + try: + producer.produce(topic, value=f'msg-{produced_count}'.encode(), on_delivery=on_delivery) + producer.poll(0) + produced_count += 1 + except RuntimeError as e: + closed_runtime_error = e + break + + t = threading.Thread(target=produce_loop) + t.start() + + # Give the worker thread a moment to actually start producing before + # racing close() against it. + time.sleep(0.1) + + print(f"{called_by()}: calling close() while produce_loop is running") + assert producer.close() is True + + t.join(timeout=30) + assert not t.is_alive(), "producer thread did not finish after close()" + print( + f"{called_by()}: produced_count={produced_count}, " + f"delivered={len(delivered)}, delivery_errors={len(delivery_errors)}" + ) + + # The worker thread must have hit the "Producer has been closed" + # RuntimeError. This proves close() genuinely raced an + # in-flight produce(). + assert closed_runtime_error is not None, "worker thread never hit the closed-producer RuntimeError" + assert not delivery_errors, f"unexpected delivery errors: {delivery_errors}" + assert produced_count > 0, "no messages were produced" + assert ( + len(delivered) == produced_count + ), f"expected all {produced_count} produced messages to be delivered, got {len(delivered)}" + + def test_concurrent_flush_from_multiple_threads(self, kafka_cluster): + """ + Two threads calling flush() concurrently -- + confirms librdkafka's own atomic flush counter (rd_kafka_flush) + correctly waits for all outstanding messages across both callers, + not just its own, and neither returns early. + """ + topic = kafka_cluster.create_topic_and_wait_propogation("test_concurrent_flush") + producer = kafka_cluster.producer( + {'error_cb': prefixed_error_cb('test_concurrent_flush_from_multiple_threads')} + ) + + num_messages = 500 + for i in range(num_messages): + producer.produce(topic, value=f'msg-{i}'.encode()) + producer.poll(0) + + flush_results = [] + + def flush_worker(): + remaining = producer.flush(30) + flush_results.append(remaining) + + threads = [threading.Thread(target=flush_worker) for _ in range(2)] + for t in threads: + t.start() + for t in threads: + t.join(timeout=35) + + print(f"{called_by()}: flush_results={flush_results}, len(producer)={len(producer)}") + assert all(not t.is_alive() for t in threads), "a flush() thread did not finish" + assert all( + r == 0 for r in flush_results + ), f"flush() returned early with messages still outstanding: {flush_results}" + assert len(producer) == 0 + + +class TestTransactionalProducerConcurrency: + def test_concurrent_produce_during_open_transaction(self, kafka_cluster): + """produce() is allowed concurrently from multiple threads while a + transaction is open (only checks an atomic flag, not an exclusive + mutex like the transaction-state APIs).""" + topic = kafka_cluster.create_topic_and_wait_propogation("test_txn_concurrent_produce") + producer = kafka_cluster.producer( + { + 'transactional.id': f'test-txn-concurrent-produce-{uuid1()}', + 'error_cb': prefixed_error_cb('test_concurrent_produce_during_open_transaction'), + } + ) + + producer.init_transactions() + producer.begin_transaction() + + num_threads = 8 + messages_per_thread = 50 + errors = [] + + def produce_worker(thread_id): + try: + for i in range(messages_per_thread): + producer.produce(topic, value=f'thread-{thread_id}-msg-{i}'.encode()) + producer.poll(0) + except Exception as e: # noqa: BLE001 - want to see any exception, not just crashes + errors.append((thread_id, e)) + + threads = [threading.Thread(target=produce_worker, args=(i,)) for i in range(num_threads)] + for t in threads: + t.start() + for t in threads: + t.join(timeout=30) + + print(f"{called_by()}: {num_threads} threads finished producing, errors={errors}") + assert all(not t.is_alive() for t in threads), "a produce() thread did not finish" + assert not errors, f"unexpected exceptions from concurrent produce() during open transaction: {errors}" + + producer.commit_transaction() + + consumer_conf = kafka_cluster.client_conf() + consumer_conf.update( + { + 'group.id': str(uuid1()), + 'auto.offset.reset': 'earliest', + 'enable.auto.commit': False, + 'enable.partition.eof': True, + 'isolation.level': 'read_committed', + } + ) + consumer = TestConsumer(consumer_conf) + consumer.subscribe([topic]) + + msg_cnt = 0 + eof_reached = False + while not eof_reached: + msg = consumer.poll(timeout=10.0) + assert msg is not None, "timed out waiting for messages" + if msg.error(): + if msg.error().code() == KafkaError._PARTITION_EOF: + eof_reached = True + continue + raise KafkaException(msg.error()) + msg_cnt += 1 + consumer.close() + + print(f"{called_by()}: consumed msg_cnt={msg_cnt}") + assert msg_cnt == num_threads * messages_per_thread + + def test_concurrent_calls_to_same_transaction_api(self, kafka_cluster): + """Two threads both calling commit_transaction() at once: exactly + one succeeds, the other gets a clean _PREV_IN_PROGRESS error.""" + topic = kafka_cluster.create_topic_and_wait_propogation("test_txn_same_api_race") + producer = kafka_cluster.producer( + { + 'transactional.id': f'test-txn-same-api-race-{uuid1()}', + 'error_cb': prefixed_error_cb('test_concurrent_calls_to_same_transaction_api'), + } + ) + + producer.init_transactions() + producer.begin_transaction() + producer.produce(topic, value=b'msg') + producer.flush() + + results = [] + barrier = threading.Barrier(2) + + def call_commit(): + barrier.wait() + try: + producer.commit_transaction() + results.append(True) + except KafkaException as e: + results.append(e.args[0]) + + threads = [threading.Thread(target=call_commit) for _ in range(2)] + for t in threads: + t.start() + for t in threads: + t.join(timeout=30) + + print(f"{called_by()}: results={results}") + assert all(not t.is_alive() for t in threads), "a commit_transaction() thread did not finish" + assert len(results) == 2 + successes = [r for r in results if r is True] + errors = [r for r in results if r is not True] + assert len(successes) == 1, f"expected exactly one successful commit, got: {results}" + assert len(errors) == 1, f"expected exactly one error result, got: {results}" + + def test_close_races_open_transaction(self, kafka_cluster): + """close() concurrent with an open (uncommitted) transaction: + close() must not crash or hang, and since the transaction was + never committed, none of its messages should become visible to + a read_committed consumer.""" + topic = kafka_cluster.create_topic_and_wait_propogation("test_txn_close_race") + producer = kafka_cluster.producer( + { + 'transactional.id': f'test-txn-close-race-{uuid1()}', + 'error_cb': prefixed_error_cb('test_close_races_open_transaction'), + } + ) + + producer.init_transactions() + producer.begin_transaction() + + produced_count = 0 + closed_runtime_error = None + + def produce_loop(): + nonlocal produced_count, closed_runtime_error + while True: + try: + producer.produce(topic, value=f'msg-{produced_count}'.encode()) + producer.poll(0) + produced_count += 1 + except RuntimeError as e: + closed_runtime_error = e + break + + t = threading.Thread(target=produce_loop) + t.start() + + time.sleep(0.1) + print(f"{called_by()}: calling close() while a transaction is still open") + assert producer.close() is True + + t.join(timeout=30) + assert not t.is_alive(), "producer thread did not finish after close()" + print(f"{called_by()}: produced_count={produced_count} before close() won the race") + assert closed_runtime_error is not None, "worker thread never hit the closed-producer RuntimeError" + assert produced_count > 0, "no messages were produced before close()" + + consumer_conf = kafka_cluster.client_conf() + consumer_conf.update( + { + 'group.id': str(uuid1()), + 'auto.offset.reset': 'earliest', + 'enable.auto.commit': False, + 'enable.partition.eof': True, + 'isolation.level': 'read_committed', + } + ) + consumer = TestConsumer(consumer_conf) + consumer.subscribe([topic]) + + msg = consumer.poll(timeout=10.0) + consumer.close() + + print(f"{called_by()}: consumer.poll() returned error={msg.error() if msg else None}") + assert msg is not None, "timed out waiting for a message/EOF" + assert msg.error() is not None and msg.error().code() == KafkaError._PARTITION_EOF, ( + "an uncommitted transaction's messages must not be visible to a " "read_committed consumer after close()" + ) + + def test_concurrent_calls_to_different_transaction_apis(self, kafka_cluster): + """One thread calls commit_transaction() while another calls + abort_transaction() at once: exactly one succeeds, the other gets + a clean _CONFLICT error.""" + topic = kafka_cluster.create_topic_and_wait_propogation("test_txn_diff_api_race") + producer = kafka_cluster.producer( + { + 'transactional.id': f'test-txn-diff-api-race-{uuid1()}', + 'error_cb': prefixed_error_cb('test_concurrent_calls_to_different_transaction_apis'), + } + ) + + producer.init_transactions() + producer.begin_transaction() + producer.produce(topic, value=b'msg') + producer.flush() + + results = {} + barrier = threading.Barrier(2) + + def call_commit(): + barrier.wait() + try: + producer.commit_transaction() + results['commit'] = True + except KafkaException as e: + results['commit'] = e.args[0] + + def call_abort(): + barrier.wait() + try: + producer.abort_transaction() + results['abort'] = True + except KafkaException as e: + results['abort'] = e.args[0] + + t1 = threading.Thread(target=call_commit) + t2 = threading.Thread(target=call_abort) + t1.start() + t2.start() + t1.join(timeout=30) + t2.join(timeout=30) + + print(f"{called_by()}: results={results}") + assert not t1.is_alive() and not t2.is_alive(), "a transaction-ending thread did not finish" + successes = [k for k, v in results.items() if v is True] + assert len(successes) == 1, f"expected exactly one of commit/abort to succeed, got: {results}"