Carry the ClickHouse patches onto v3.13.0 - #12
Merged
thevar1able merged 3 commits intoOct 1, 2026
Merged
Conversation
The adapter defers poll mutation to the event loop thread through a FIFO, but natsLibuv_Attach did not follow that discipline in two ways. First, it wrote nle->socket and nle->events directly. On a reconnect that runs on the nats-io reconnect thread (_doReconnect -> _processConnInit -> evCbs.attach), those writes race the event loop thread's write in uvPollUpdate's close branch. ThreadSanitizer reports it as a data race on a 4-byte field of natsLibuvEvents. Second, it scheduled only when it was called off the loop thread, while Read/Write/Detach also schedule when events are already queued. A loop-thread attach could therefore run ahead of removals already in the list. Both let an attach install a new socket before an older, already queued READ removal is drained. _evStopPolling only requests the removals and then _processOpError starts the reconnect thread, so when the loop thread finally drains that removal, nle->events reaches 0 and uvPollUpdate takes the close branch on the socket the attach just installed: natsConnection_ProcessCloseEvent closes a live, freshly connected fd and sets nle->socket to invalid. The reconnect still reports success (nc->el.attached = true) but the connection never polls again. Carry the socket in the ATTACH event and assign nle->socket and nle->events inside uvAsyncAttach, so both fields are written only by the loop thread, and give the attach the same "or events are queued" guard as the other three callbacks. Ordering is then enforced by the queue instead of by timing, and the race disappears because the fields are no longer shared. No new lock is introduced; nle->lock still guards only head/tail. Reproduced with a standalone harness that links this header against libuv under ThreadSanitizer and drives the interleaving deterministically: before, 25/25 runs close the freshly attached fd and 23/25 concurrent runs report the data race in natsLibuv_Attach; after, 0 in both. (cherry picked from commit 37532c3)
A `NATS` connection whose credentials the server rejects on two consecutive
reconnect attempts is closed for good by `_processAuthError`, and closing it
crashes the process:
uv_close contrib/libuv/src/unix/core.c:153
uvAsyncCb src/adapters/libuv.h:301
uv__async_io
uv__io_poll
uv_run
`_evStopPolling` runs once per reconnect attempt and once more from `_close`
when the library gives up, because `_close` only checks `nc->el.attached`,
which `_evStopPolling` does not clear. Each run asks the adapter to remove the
write and then the read event, and the removal which brings `nle->events` to
zero closes the poll handle and sets `nle->handle` to `NULL`. Every further
removal then reaches the unguarded `uv_close((uv_handle_t*) nle->handle, ...)`
with a `NULL` handle, and `uv_close` faults reading the handle's type field.
Guard `uvPollUpdate` against a handle which is already gone, and do not ask a
connection which is not being polled to stop polling: `_evStopPolling` hands
the socket close to the adapter, which by then only knows the socket it was
given when it was attached, so a second run also leaked the socket of the last
reconnect attempt.
Reported as ClickHouse/ClickHouse#118449
(cherry picked from commit 3e3a3f1)
A signal delivered to the thread interrupts poll() with EINTR. natsSock_WaitReady treated it as a socket error, so a connection attempt made by an application with a sampling profiler (which sends a signal to every thread) failed spuriously with "poll error: 4". Wait again for whatever is left of the deadline instead. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit b88704d)
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
4 participants
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.
This branch re-cuts ClickHouse's nats.c patches onto upstream tag
v3.13.0. Three patches are carried, all applied withgit cherry-pick -xso original authors and provenance are preserved: Groene AI's "libuv adapter: do not let an attach overtake a queued poll removal", and Alexey Milovidov's "Do not close an already released poll handle in the libuv adapter" and "Retry poll() on EINTR when waiting for a socket". The earlier pr#2/pr#3 build fixes (src/js.c,src/kv.c,src/include/n-unix.h) are already contained in the base branch, so they are not re-applied.Three patches are dropped as obsolete: the libc
rand()replacement, sincev3.13.0usesnats_Rand64seeded fromRAND_bytes; the legacy OpenSSL API removal, sincev3.13.0requires OpenSSL 1.1.1+; and the JetStream fetch lock-order inversion backport, which upstream released in full inv3.10.0. There were no conflicts — every cherry-pick auto-merged, two of them through real 3-way merges against upstream's rewrittensrc/adapters/libuv.hand the untouched_closeregion insrc/conn.c.Once merged, ClickHouse's
contrib/nats-iosubmodule is bumped to this branch head.