fix(ilp): fix a leaked socket and native memory when an HTTP sender fails to start

[puzpuzpuz]

Found while reviewing questdb/questdb#7434, which made the equivalent
constructors exception-safe in the core repo. That fix does not reach production:
AbstractLineHttpSender builds its client through this repo's
HttpClientFactory, and the core one has no caller outside tests. So the ILP
HTTP senders were still running the pre-fix construction sequence.

The branch carries two fixes. Both leak the same resources for the same reason -
a constructor that acquires and then throws hands no reference back, so nothing
can close what it took - but they sit at different levels of the call stack and
were found separately.

It also carries one change that has nothing to do with either: a CI deflake, in
its own commit, described at the bottom.

Fix 1: the HTTP client constructors

The defect

HttpClient's constructor takes a socket and two native buffers, then builds a
ResponseHeaders whose own parser allocates again. None of it was guarded. A
throw past the first acquisition leaves a half-built client that the caller never
receives, so close() never runs and everything taken so far leaks - the socket
file descriptor included. HttpHeaderParser had the same problem in the opposite
direction: its DirectUtf8Sink was a field initialiser, which Java runs before
the constructor body, so no try in the body could ever have covered it.

Reachable through Unsafe.malloc failing under memory pressure, and through a
SocketFactory or TLS handshake failing.

The fix

• HttpClient takes the socket, both buffers and the response parser inside one
  try and frees what it took in reverse order. A close failure attaches to the
  primary exception as suppressed rather than replacing it, so the caller still
  sees why construction failed.
• HttpClient.ResponseHeaders releases the parser its super() call completed.
  HttpClient has no reference to reach that parser through at the point the two
  response objects are built, and Java does not run a superclass close() when a
  subclass constructor fails.
• HttpHeaderParser moves the sink out of its field initialiser into the guarded
  region, with a local the catch can reach.
• HttpClientLinux, HttpClientOsx and HttpClientWindows close the completed
  base client when their poller fails to open.
• HttpClientWindows also reads the select facade before it takes the FD set.
  Nothing between that acquisition and the end of the constructor can then throw,
  which removes a partial-FDSet cleanup branch that only a Windows host could
  ever have reached - FDSet's first statement touches the Windows-only
  SelectAccessor natives, so a test on Linux or macOS cannot get past it to fail
  anything later.

ResponseHeaders's rollback is the one block with no test: its two response
objects allocate nothing native, so only a Java-heap OutOfMemoryError reaches
the catch, and there is no seam that can inject one without adding test-only
production surface. The code comment says so rather than leaving the gap implicit.

Fix 2: the sender constructor, one level up

The defect

AbstractLineHttpSender's constructor assigned this.client - either the one
createLineSender() handed in or one it built through HttpClientFactory - and
then called newRequest(), with neither statement inside a try. newRequest()
writes the POST line, the path, the User-Agent header and any auth header into
the client's request buffer, and growBuffer() throws once they exceed the
maximum. A throw there stranded a fully built client - socket, two native
buffers, response parser - that no caller ever receives a reference to.

createLineSender() cannot clean that up. Its three Misc.free(cli) calls all
sit before the switch, and the switch builds the sender inside a return
expression that no try covers. Both entry shapes leak: an explicit protocol
version makes the constructor build the client itself, and an auto-detected one
hands in the client the version probe already used.

Confirmed by direct measurement before the fix: 65552 bytes under
NATIVE_DEFAULT plus the socket, on both shapes.

How reachable it is depends on who builds the sender. Through LineSenderBuilder
the growBuffer throw is remote - maxBufferCapacity is floored at 64 KiB and
defaults to 100 MB, so the preamble would have to be absurd - which leaves a
Java-heap OutOfMemoryError from the version lookup or the User-Agent
concatenation as the realistic trigger there. io.questdb.client.cutlass.line.http
is an exported package, though, and a third-party HttpClientConfiguration sets
the maximum to anything it likes, with no floor.

The fix

The constructor takes the client, the version and the request inside one try
and closes the client on the way out, with the same suppressed-exception handling
as fix 1. It frees a handed-in client too: close() already frees the client
regardless of where it came from, so the sender owns it from the assignment on
either way, and createLineSender() has no way to free one for it.

Tradeoffs

None on the happy path, for either fix - the guarded regions add no allocation and
no branch that was not already there. Moving new DirectUtf8Sink(0) from a field
initialiser into the constructor body is runtime-identical. The two
field-assignment reorders are not observable for any configuration in this repo,
whose getters are plain field reads or INSTANCE returns.

Fix 2 changes what a failed constructor does with a client the caller supplied:
it now closes it. A third party that passes its own client, catches the failure
and then closes that client itself will double-close - which is safe, since
HttpClient.close() guards on bufLo != 0, Misc.free(socket) is idempotent,
and HttpHeaderParser.close() guards on headerPtr != 0. The alternative,
leaving a handed-in client open, contradicts close(), which frees it
unconditionally on the success path.

Unrelated: the windows-msvc-2022-x64 deflake

Not part of either fix, and touching no file they touch. The branch's Azure run
failed on Windows in
QuestDBImplCloseLifecycleTest.facadeCloseIsBoundedUnderRepeatedInterruptsDuringSenderCreation
with sender close left its creation wait before the interrupt storm landed twice; interrupts landed: 6. Every other leg was green, including both GitHub
JDK jobs and the Linux and macOS Azure legs.

It is a false negative in the observation, not a defect in close(). Both pools
exposed the close-time creation wait to tests as
lock.hasWaiters(creationFinished), which is not a valid "is it still waiting"
probe under interrupts: when a thread parked in awaitNanos is interrupted, AQS
transferAfterCancelledWait moves its node off the condition queue and onto the
lock's sync queue to reacquire before await rethrows. hasWaiters() counts
condition-queue nodes only, so it reads false mid-wait. A harness replicating
SenderPool.close()'s loop against the same interrupt storm measures 5-8% of
polls reading false with the 1s deadline honoured exactly. The helper compounded
it by sampling the interrupt count before the probe it fails on - hence a
failure that reports 6 interrupts having landed while claiming fewer than 2 did.

SenderPool and QueryClientPool now count the threads inside the wait loop
under the lock and report that. The state no longer flickers, and the rising edge
no longer depends on the closer having actually parked - which is what the
earlier awaitCreationWaiter callers wanted anyway.
awaitRepeatedInterrupts() re-reads the count before failing.

Test plan

• New HttpClientConstructorTest: five tests, one per rollback point in fix 1,
  each with a close-counting SocketFactory and the enclosing assertMemoryLeak
  watching the native blocks.
• New HttpHeaderParserTest.testConstructorFailureFreesNativeAllocations for the
  sink.
• New LineHttpSenderConstructorTest: two tests for fix 2, one per entry shape. A
  configuration whose maximum request buffer is smaller than the preamble makes
  newRequest() throw deterministically, with no server and no new production
  seam. The self-built case asserts against the leak check; the handed-in case
  adds a close-counting socket, since the leak check cannot see a file descriptor.
• This repo has no RSS-limit seam, so these tests inject their faults differently
  from the core ones but just as deterministically, and without a host dependency:
  a negative buffer size makes sun.misc.Unsafe.allocateMemory throw
  IllegalArgumentException on every supported JDK, and a throwing configuration
  getter fails each platform subclass as a constructor argument - so
  new Kqueue(...) and new FDSet(...) are never invoked and neither
  KqueueAccessor nor SelectAccessor is ever initialised.
• Verified red. With the five fix-1 production files reverted and the tests kept,
  all six fail: five on the constructor must close the socket it took expected:<1> but was:<0>, the parser test on native memory leaked or over-freed under tag NATIVE_DIRECT_UTF8_SINK expected:<0> but was:<32>. The
  Windows one errors with UnsatisfiedLinkError: SelectAccessor.arrayOffset
  before the reorder, which is the untestability the reorder removes. With fix 2
  reverted, both new sender tests fail: one on native memory leaked or over-freed under tag NATIVE_DEFAULT expected:<0> but was:<65552>, the other on
  the socket close count.
• Full io.questdb.client.test.cutlass.** suite green: 1807 tests, 0 failures.
• For the deflake: new
  QuestDBImplCloseLifecycleTest.creationWaitStateDoesNotFlickerUnderInterruptStorm
  pins the contract the interrupt tests rely on, so nobody reverts the accessor to
  a condition-queue probe. Verified red on the old predicate - it flickers after
  46 polls with 2 interrupts landed, the same signature Windows reported - and
  green on the new one. Plus 20/20 passes of all three interrupt tests under 40
  busy-loop threads on a 24-core box.
• Whole client suite green locally: 2766 tests, 0 failures, 0 errors. Note that
  this needs a locally cmake-built libquestdb.so: the native binaries are no
  longer committed, and a stale one left in target/classes by an older checkout
  fails ~340 tests with UnsatisfiedLinkError on symbols it predates.
• Installed into the local Maven cache and re-run from the parent repo against
  the real ILP path: LineHttpSenderTest and ServerMainHttpAuthTest, 83 tests
  run, 1 skipped, 0 failures.
• Every changed file compiles at --release 8; nothing newer than Java 8 in
  production or test code.

🤖 Generated with Claude Code

[mtopolnik]

[PR Coverage check]

😍 pass : 85 / 102 (83.33%)

file detail

	path	covered line	new line	coverage
🔵	io/questdb/client/cutlass/http/client/HttpClientWindows.java	3	6	50.00%
🔵	io/questdb/client/cutlass/http/client/HttpClientOsx.java	4	7	57.14%
🔵	io/questdb/client/cutlass/http/client/HttpClient.java	28	35	80.00%
🔵	io/questdb/client/cutlass/line/http/AbstractLineHttpSender.java	13	16	81.25%
🔵	io/questdb/client/cutlass/http/HttpHeaderParser.java	12	13	92.31%
🔵	io/questdb/client/cutlass/http/client/HttpClientLinux.java	7	7	100.00%
🔵	io/questdb/client/impl/QueryClientPool.java	9	9	100.00%
🔵	io/questdb/client/impl/SenderPool.java	9	9	100.00%