Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions .semaphore/semaphore.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ global_job_config:
- mkdir dest
env_vars:
- name: CACHE_TAG
value: '7'
value: '8'
- name: CI
value: 'true'
- name: KAFKA_VERSION
Expand Down Expand Up @@ -136,8 +136,11 @@ blocks:
jobs:
- name: 'Build configuration checks'
commands:
- packaging/tools/run-in-docker.sh test-runner-amd64-${CACHE_TAG} ./packaging/tools/build-configurations-checks.sh

- python3 -m pip install -U pip
- ./packaging/tools/build-configurations-checks.sh make test-runner-manylinux-amd64-${CACHE_TAG}
- ./packaging/tools/build-configurations-checks.sh cmake test-runner-manylinux-amd64-${CACHE_TAG}
- ./packaging/tools/build-configurations-checks.sh make test-runner-alpine-amd64-${CACHE_TAG}
- ./packaging/tools/build-configurations-checks.sh cmake test-runner-alpine-amd64-${CACHE_TAG}

- name: 'Linux Ubuntu amd64: share consumer tests'
dependencies: []
Expand Down
25 changes: 25 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,28 @@
# Unreleased

librdkafka Unreleased is a maintenance release:

* Fix compilation with CMake when CURL is disabled (#5136).
* Fix `rd_atomic{32,64}_set` returning the new value in CMake builds, restoring the `ALL_BROKERS_DOWN` event (#5136).


## Fixes

### General fixes

* Issues: #5135.
Fix compilation with CMake when CURL is disabled.
The OAuthBearer OIDC code included `<curl/curl.h>` under `#ifdef WITH_OAUTHBEARER_OIDC`, but
CMake always defines that macro (to 0 or 1), so CURL was required even when it was turned off.
Happening since 2.11.0 (#5136).
* Issues: #5282.
Fix `rd_atomic32_set`/`rd_atomic64_set` returning the new value instead of the previous one in CMake builds.
CMake never defined `HAVE_ATOMICS_{32,64}_ATOMIC`, so the setters used a non-atomic fallback that
returned the new value, which prevented the `ALL_BROKERS_DOWN` event from being raised under CMake.
Happening since 2.11.1 (#5136).



# librdkafka v2.14.2

librdkafka v2.14.2 is a maintenance release:
Expand Down
2 changes: 2 additions & 0 deletions examples/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,5 @@ user_scram
list_offsets
elect_leaders
share_consumer
share_consumer_commit_async
share_consumer_commit_sync
2 changes: 2 additions & 0 deletions packaging/cmake/config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#cmakedefine01 ENABLE_REFCNT_DEBUG

#cmakedefine01 HAVE_ATOMICS_32
#cmakedefine01 HAVE_ATOMICS_32_ATOMIC
#cmakedefine01 HAVE_ATOMICS_32_SYNC

#if (HAVE_ATOMICS_32)
Expand All @@ -14,6 +15,7 @@
#endif

#cmakedefine01 HAVE_ATOMICS_64
#cmakedefine01 HAVE_ATOMICS_64_ATOMIC
#cmakedefine01 HAVE_ATOMICS_64_SYNC

#if (HAVE_ATOMICS_64)
Expand Down
16 changes: 12 additions & 4 deletions packaging/cmake/try_compile/rdkafka_setup.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ try_compile(
# Atomic 32 tests {
set(LINK_ATOMIC NO)
set(HAVE_ATOMICS_32 NO)
set(HAVE_ATOMICS_32_ATOMIC NO)
set(HAVE_ATOMICS_32_SYNC NO)

try_compile(
Expand All @@ -57,7 +58,7 @@ try_compile(
)

if(_atomics_32)
set(HAVE_ATOMICS_32 YES)
set(HAVE_ATOMICS_32_ATOMIC YES)
else()
try_compile(
_atomics_32_lib
Expand All @@ -66,7 +67,7 @@ else()
LINK_LIBRARIES "-latomic"
)
if(_atomics_32_lib)
set(HAVE_ATOMICS_32 YES)
set(HAVE_ATOMICS_32_ATOMIC YES)
set(LINK_ATOMIC YES)
else()
try_compile(
Expand All @@ -76,10 +77,14 @@ else()
)
endif()
endif()
if (HAVE_ATOMICS_32_ATOMIC OR HAVE_ATOMICS_32_SYNC)
set(HAVE_ATOMICS_32 YES)
endif()
# }

# Atomic 64 tests {
set(HAVE_ATOMICS_64 NO)
set(HAVE_ATOMICS_64_ATOMIC NO)
set(HAVE_ATOMICS_64_SYNC NO)

try_compile(
Expand All @@ -89,7 +94,7 @@ try_compile(
)

if(_atomics_64)
set(HAVE_ATOMICS_64 YES)
set(HAVE_ATOMICS_64_ATOMIC YES)
else()
try_compile(
_atomics_64_lib
Expand All @@ -98,7 +103,7 @@ else()
LINK_LIBRARIES "-latomic"
)
if(_atomics_64_lib)
set(HAVE_ATOMICS_64 YES)
set(HAVE_ATOMICS_64_ATOMIC YES)
set(LINK_ATOMIC YES)
else()
try_compile(
Expand All @@ -108,6 +113,9 @@ else()
)
endif()
endif()
if (HAVE_ATOMICS_64_ATOMIC OR HAVE_ATOMICS_64_SYNC)
set(HAVE_ATOMICS_64 YES)
endif()
# }

# C11 threads
Expand Down
9 changes: 9 additions & 0 deletions packaging/tools/Dockerfile.alpine
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
FROM alpine:3.16.9

ARG UID

RUN apk add bash gcc g++ make cmake git bsd-compat-headers
RUN mkdir -p /home/user
WORKDIR /home/user
RUN chown -R ${UID} /home/user
USER ${UID}
8 changes: 8 additions & 0 deletions packaging/tools/Dockerfile.manylinux
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
FROM quay.io/pypa/manylinux_2_28_x86_64:2024.07.01-1

ARG UID

RUN mkdir -p /home/user
WORKDIR /home/user
RUN chown -R ${UID} /home/user
USER ${UID}
40 changes: 38 additions & 2 deletions packaging/tools/build-configurations-checks.sh
Original file line number Diff line number Diff line change
@@ -1,12 +1,48 @@
#!/bin/bash
#!/bin/sh
set -e
build_tool="$1"
docker_image_tag="$2"

if [ -n "$docker_image_tag" ]; then
echo "Running in docker image tag: $docker_image_tag"
# Running on the host, spin up the docker builder.
./packaging/tools/run-in-docker.sh $docker_image_tag ./packaging/tools/build-configurations-checks.sh $build_tool
# Only reached on exec error
exit $?
fi

if [ -z "$build_tool" ]; then
# Default to using make if no build tool is specified.
build_tool="make"
fi

# Clone the repo so other builds are unaffected of what we're doing
# and we get a pristine build tree.
git clone /librdkafka /home/user/librdkafka

cd /home/user/librdkafka

# Disable all flags to make sure it
# compiles correctly in all cases
./configure --install-deps --disable-ssl --disable-gssapi \
if [ "$build_tool" = "make" ]; then
./configure --disable-ssl --disable-gssapi \
--disable-curl --disable-zlib \
--disable-zstd --disable-lz4-ext --disable-regex-ext \
--disable-c11threads --disable-syslog \
--enable-werror --enable-devel
cat ./config.h
else
cmake -DWITH_SSL=OFF -DWITH_SASL_CYRUS=OFF \
-DWITH_CURL=OFF -DWITH_ZLIB=OFF \
-DWITH_ZSTD=OFF -DHAVE_REGEX=OFF -DWITH_C11THREADS=OFF \
-DWITH_LIBDL=OFF
cat ./generated/config.h
fi
make -j

export CI=true
if [ "0$build_tool" = "0make" ]; then
make -j -C tests run_local_quick
else
ctest -VV -R RdKafkaTestBrokerLessQuick --output-on-failure
fi
11 changes: 10 additions & 1 deletion packaging/tools/run-in-docker.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#!/bin/bash

set -e
if [ "$#" -lt 2 ]; then
echo "Usage: $0 <docker-image> [<args>...]"
exit 1
Expand All @@ -10,11 +11,19 @@ SCRIPT_DIR=$(dirname "$0")
ENTRYPOINT=${2}
REST=${@:3}

DOCKERFILE=Dockerfile
if [[ $IMAGE =~ ^"test-runner-manylinux-amd64-" ]]; then
DOCKERFILE=Dockerfile.manylinux
fi
if [[ $IMAGE =~ ^"test-runner-alpine-amd64-" ]]; then
DOCKERFILE=Dockerfile.alpine
fi

if [ $(which cache) ]; then
cache restore ${IMAGE}.tar
fi
if [ ! -f ./${IMAGE}.tar ]; then
docker build -f $SCRIPT_DIR/Dockerfile -t $IMAGE --build-arg UID=$UID .
docker build -f $SCRIPT_DIR/$DOCKERFILE -t $IMAGE --build-arg UID=$UID .
docker save $IMAGE -o ./${IMAGE}.tar

if [ $(which cache) ]; then
Expand Down
47 changes: 18 additions & 29 deletions src/rdatomic.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,19 +119,17 @@ static RD_INLINE int32_t RD_UNUSED rd_atomic32_set(rd_atomic32_t *ra,
int32_t v) {
#ifdef _WIN32
return InterlockedExchange((LONG *)&ra->val, v);
#elif !HAVE_ATOMICS_32
#elif HAVE_ATOMICS_32 && HAVE_ATOMICS_32_ATOMIC
return __atomic_exchange_n(&ra->val, v, __ATOMIC_SEQ_CST);
#elif HAVE_ATOMICS_32 && HAVE_ATOMICS_32_SYNC

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.

Isn't this branch a dead code in case of CMake and eventually ending up using #else branch which is safe though because lock would be defined given HAVE_ATOMICS_32 is 0?

CMake ends up setting only HAVE_ATOMICS_32_SYNC reference and not HAVE_ATOMICS_32. They both seem mutually exclusive in case of CMake. So this branch would never be entered right?

Should we change to something like below which seems to handle all scenarios and isn't based on the invariant HAVE_ATOMICS_32_ATOMIC

static RD_INLINE int32_t RD_UNUSED rd_atomic32_set(rd_atomic32_t *ra,
                                                   int32_t v) {
#ifdef _WIN32
        return InterlockedExchange((LONG *)&ra->val, v);
#elif HAVE_ATOMICS_32_SYNC
        return __sync_lock_test_and_set(&ra->val, v);
#elif !HAVE_ATOMICS_32
        int32_t r;
        mtx_lock(&ra->lock);
        r       = ra->val;
        ra->val = v;
        mtx_unlock(&ra->lock);
        return r;
#else
        return __atomic_exchange_n(&ra->val, v, __ATOMIC_SEQ_CST);
#endif
}

The only tradeoff I see is that default assumes __atomic is available.

@emasab Emanuele Sabellico (emasab) Jun 25, 2026

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.

I was uniforming cmake in this PR to set the same values as mklove, so HAVE_ATOMICS_32 for example is set to 1 in both cases and then either HAVE_ATOMICS_32_ATOMIC or HAVE_ATOMICS_32_SYNC is set to 1.
Just there's a case when using try_compile(HAVE_ATOMICS_32_SYNC where HAVE_ATOMICS_32 isn't set to true. I've pushed the cmake change. In mklove instead there are cases where HAVE_ATOMICS_32_SYNC or HAVE_ATOMICS_32_ATOMIC aren't defined if HAVE_ATOMICS_32 is false.

return __sync_lock_test_and_set(&ra->val, v);
#else
int32_t r;
mtx_lock(&ra->lock);
r = rd->val;
r = ra->val;
ra->val = v;
mtx_unlock(&ra->lock);
return r;
#elif HAVE_ATOMICS_32_ATOMIC
return __atomic_exchange_n(&ra->val, v, __ATOMIC_SEQ_CST);
#elif HAVE_ATOMICS_32_SYNC
return __sync_lock_test_and_set(&ra->val, v);
#else
return ra->val = v; // FIXME
#endif
}

Expand Down Expand Up @@ -211,19 +209,17 @@ static RD_INLINE int64_t RD_UNUSED rd_atomic64_set(rd_atomic64_t *ra,
int64_t v) {
#ifdef _WIN32
return InterlockedExchange64(&ra->val, v);
#elif !HAVE_ATOMICS_64
#elif HAVE_ATOMICS_64 && HAVE_ATOMICS_64_ATOMIC
return __atomic_exchange_n(&ra->val, v, __ATOMIC_SEQ_CST);
#elif HAVE_ATOMICS_64 && HAVE_ATOMICS_64_SYNC
return __sync_lock_test_and_set(&ra->val, v);
#else
int64_t r;
mtx_lock(&ra->lock);
r = ra->val;
ra->val = v;
mtx_unlock(&ra->lock);
return r;
#elif HAVE_ATOMICS_64_ATOMIC
return __atomic_exchange_n(&ra->val, v, __ATOMIC_SEQ_CST);
#elif HAVE_ATOMICS_64_SYNC
return __sync_lock_test_and_set(&ra->val, v);
#else
return ra->val = v; // FIXME
#endif
}

Expand All @@ -243,7 +239,13 @@ static RD_INLINE int RD_UNUSED rd_atomic64_cas(rd_atomic64_t *ra,
return InterlockedCompareExchange64((LONG64 *)&ra->val, (LONG64)desired,
(LONG64)expected) ==
(LONG64)expected;
#elif !HAVE_ATOMICS_64
#elif HAVE_ATOMICS_64 && HAVE_ATOMICS_64_ATOMIC
return __atomic_compare_exchange_n(&ra->val, &expected, desired,
0 /* strong */, __ATOMIC_SEQ_CST,
__ATOMIC_SEQ_CST);
#elif HAVE_ATOMICS_64 && HAVE_ATOMICS_64_SYNC
return __sync_bool_compare_and_swap(&ra->val, expected, desired);
#else
int r;
mtx_lock(&ra->lock);
if (ra->val == expected) {
Expand All @@ -254,19 +256,6 @@ static RD_INLINE int RD_UNUSED rd_atomic64_cas(rd_atomic64_t *ra,
}
mtx_unlock(&ra->lock);
return r;
#elif HAVE_ATOMICS_64_ATOMIC
return __atomic_compare_exchange_n(&ra->val, &expected, desired,
0 /* strong */, __ATOMIC_SEQ_CST,
__ATOMIC_SEQ_CST);
#elif HAVE_ATOMICS_64_SYNC
return __sync_bool_compare_and_swap(&ra->val, expected, desired);
#else
// FIXME
if (ra->val == expected) {
ra->val = desired;
return 1;
}
return 0;
#endif
}

Expand Down
2 changes: 1 addition & 1 deletion src/rdkafka_conf.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
#include <windows.h>
#endif

#ifdef WITH_OAUTHBEARER_OIDC
#if WITH_OAUTHBEARER_OIDC
#include <curl/curl.h>
#endif

Expand Down
16 changes: 15 additions & 1 deletion tests/0066-plugins.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,20 @@ static void do_test_plugin() {
NULL,
};

RdKafka::Conf *conf = RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL);
if (conf->set("plugin.library.paths",
"interceptor_test/interceptor_test_dummy", errstr) &&
Comment thread
emasab marked this conversation as resolved.
errstr.find(
"Configuration property \"plugin.library.paths\" not supported") !=
std::string::npos) {
delete conf;
Test::Skip(
"Configuration property \"plugin.library.paths\" not supported in this "
"build\n");
Comment thread
emasab marked this conversation as resolved.
return;
}
delete conf;

char cwd[512], *pcwd;
#ifdef _WIN32
pcwd = _getcwd(cwd, sizeof(cwd) - 1);
Expand All @@ -84,7 +98,7 @@ static void do_test_plugin() {
ictest_cnt_init(&ictest.on_new, 1, 1);

/* Config for intercepted client */
RdKafka::Conf *conf = RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL);
conf = RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL);

for (int i = 0; config[i]; i += 2) {
Test::Say(tostr() << "set(" << config[i] << ", " << config[i + 1] << ")\n");
Expand Down
4 changes: 2 additions & 2 deletions tests/0146-metadata_mock.c
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,9 @@ static void do_test_fast_metadata_refresh(int variation) {
mcluster, expected_metadata_requests, 500, is_metadata_request,
NULL);
TEST_ASSERT(expected_metadata_requests <= metadata_requests &&
metadata_requests <= expected_metadata_requests + 1,
metadata_requests <= expected_metadata_requests + 2,
"Expected %d or %d metadata request, got %d",
expected_metadata_requests, expected_metadata_requests + 1,
expected_metadata_requests, expected_metadata_requests + 2,
metadata_requests);
rd_kafka_mock_stop_request_tracking(mcluster);

Expand Down
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ target_link_libraries(test-runner PUBLIC rdkafka++)
add_test(NAME RdKafkaTestInParallel COMMAND test-runner -p5)
add_test(NAME RdKafkaTestSequentially COMMAND test-runner -p1)
add_test(NAME RdKafkaTestBrokerLess COMMAND test-runner -p5 -l)
add_test(NAME RdKafkaTestBrokerLessQuick COMMAND test-runner -p5 -l -Q)

if(NOT WIN32 AND NOT APPLE)
set(tests_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
Expand Down