fix: stop bundling libuv into the extension (#106) - #140
Merged
Conversation
|
Tick the box to add this pull request to the merge queue (same as
|
Every CMake preset built the extension with LINK_LIBUV_STATIC=ON, which puts a private copy of libuv inside cassandra.so. On Linux, ELF's flat namespace then makes the shared libcassandra/libscylla-cpp-driver resolve its own uv_* references against that copy instead of the libuv.so.1 it was compiled against. The driver ends up running a different libuv build than its headers described, which corrupts memory around its event loop — observed as a garbage RefCounted ref_count and an abort in RetryPolicy::dec_ref during session connect. The extension used libuv for exactly one thing: an rwlock guarding the cassandra.log INI value. Swap it for pthread_rwlock_t (statically initialised, which also removes the ordering hazard between the log callback and INI registration) and drop libuv from the build entirely. libuv stays a dependency of the C/C++ driver, where it belongs. Also link with -Wl,--exclude-libs,ALL on non-Apple platforms so no future statically linked third-party archive can interpose on the driver's own dependencies. -fvisibility=hidden was already set but only covers our own objects, never archives compiled elsewhere. macOS is unaffected either way: two-level namespaces bind each dylib to its own dependencies. Drive-by: config.m4 passed the stale -DPHP_DRIVER_STATIC, so --enable-driver-static was a no-op; the regenerated CMakePresets.json picks up the same PHP_DRIVER_* -> PHP_SCYLLADB_* rename.
CodeLieutenant
force-pushed
the
fix/106-libuv-symbol-interposition
branch
from
August 4, 2026 09:51
2ba5b5c to
5142933
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #106.
The bug
Every CMake preset set
LINK_LIBUV_STATIC=ON, socassandra.socarries its own copy of libuv. When that module is linked against a sharedlibcassandra.so.2/libscylla-cpp-driver.so, ELF's flat namespace makes the driver'suv_*references bind to the copy insidecassandra.sorather than thelibuv.so.1it was compiled against. The driver then runs a libuv build that doesn't match its headers, which corrupts memory around its event loop.In #106 that showed up as a garbage refcount and an abort inside the driver:
Frames #30–32 are the php-driver's libuv running inside libcassandra's event-loop thread — the interposition, right there in the reporter's own backtrace.
Two things this explains:
config.m4and linked libuv dynamically (PHP_ADD_LIBRARY(uv,...)); the CMake build introduced in 1.3.9 links it statically.Reduced repro of the mechanism (
gcc:13,uv_versionstanding in for libuv):The fix
The extension used libuv for exactly one thing — an rwlock guarding the
cassandra.logINI value — which does not justify the dependency.src/php_scylladb.c:uv_rwlock_t→pthread_rwlock_t, statically initialised. That also removes an ordering hazard: the log callback can fire from driver threads beforephp_scylladb_log_initialize()ranuv_rwlock_init(). Cleanup now takes the write lock instead of destroying the lock.find_package(Libuv),Libuv::Libuv,cmake/FindLibuv.cmake, theLINK_LIBUV_STATIC/BUILD_LIBUV_FROM_SRCoptions, the--enable-libuv-staticconfig.m4/PIE flag, and the preset entries. libuv remains a dependency of the C/C++ driver, which is where it belongs —scripts/compile-libuv.shand the CI action stay.-Wl,--exclude-libs,ALLon non-Apple platforms, so no future statically linked third-party archive (OpenSSL, GMP, a static cpp-driver) can interpose on the driver's dependencies.-fvisibility=hiddenwas already set, but it only covers our own objects, never archives compiled elsewhere.Reviewer notes
CMakePresets.jsonchange is the removal of theLINK_LIBUV_STATICentries — Prepare for PHP 8.6: drop removed internal APIs, add 8.6 build presets #141 already fixed the stalePHP_DRIVER_*→PHP_SCYLLADB_*variable names.config.m4had the same drift:--enable-driver-staticwas passing the dead-DPHP_DRIVER_STATICand therefore doing nothing. Now-DPHP_SCYLLADB_STATIC..github/actions/build-extensionnever passedLINK_LIBUV_STATIC, and it links the C/C++ driver statically, so libuv still arrives through the driver's own.pc.UuidTestcross-process case that needs the extension installed under its bare name.nm -D --defined-only cassandra.so | grep uv_comes back empty for a shared-driver build.