Add virtual thread friendly, blocking, Smithy HTTP client that supports HTTP 1.1 and HTTP 2 bidirectional streaming#1278
Open
mtdowling wants to merge 84 commits into
Open
Add virtual thread friendly, blocking, Smithy HTTP client that supports HTTP 1.1 and HTTP 2 bidirectional streaming#1278mtdowling wants to merge 84 commits into
mtdowling wants to merge 84 commits into
Conversation
mtdowling
force-pushed
the
vt-scratch-rebased
branch
from
8532a8b to
681a999
Compare
Adds a blocking HTTP client built for virtual threads with an HTTP/1.1 and HTTP/2 implementations, connection pooling, flow control, HPACK encoding/decoding, and comprehensive test coverage including fuzz tests.
Replace the SSLEngine-specific ClientSslEngineFactory seam with a single provider-neutral SPI, so a TLS implementation that isn't based on a javax.net.ssl.SSLEngine (e.g. a native stack like s2n) can be plugged in without changing the client's public API. SPI: - TlsProvider.connect(TlsConnectionContext) -> ConnectionTransport: the provider owns the handshake and returns a ready transport. Engine-based providers build the standard transport via the new SslEngineTransports helper; others may return any ConnectionTransport. - ConnectionTransport is un-sealed so an out-of-module provider can implement it. - TlsConnectionContext carries the per-connection inputs (host, port, ALPN, timeouts, buffer sizes, connected socket). Built-in providers: - JdkTlsProvider: the default, configured from sslContext/sslParameters. HttpClient.Builder.sslContext()/sslParameters() are now documented convenience for it. - BoringSslTlsProvider (was BoringSslEngineFactory): now implements TlsProvider. Selection and discovery: - HttpClient.Builder.tlsProvider(...) selects a provider explicitly (replaces the removed sslEngineFactory()); explicit always wins. - Opt-in discovery: -Dsmithy-java.tls-provider=<fully-qualified-class-name> selects a ServiceLoader-registered provider. Classpath presence alone changes nothing. Matching is by class name over ServiceLoader instances (no reflective Class.forName), and lookup falls back to the interface's thread-context loader is null. - supportsEpoll() (default false): the internal epoll backend hands a null-socket context that only engine-based providers can consume, so the client uses the NIO socket path for any provider that doesn't opt in. JDK and BoringSSL return true. The HTTP/1.1 SSLSocket fast path is gated on the default JDK provider (defaultJdkTls) on both the direct and proxy-tunnel paths, so a custom or discovered provider owns the end-to-end handshake in both.
TlsProvider abstracts the TLS connection strategy, not engine construction, so the JDK provider picks SSLSocket (HTTP/1.1) vs SSLEngine (H2/ALPN/epoll) internally. Removes the defaultJdkTls special-case from the connection factory; all secure connections now go through TlsProvider.connect. Also: restore full SSLParameters copy on the proxy leg, and close the socket/channel when setup throws before the transport takes ownership.
Two related bugs where END_STREAM on a HEADERS frame failed to end the response body, because streamBody.complete() was only called from the DATA-frame path (enqueueData): 1. Trailers: a HEADERS frame after DATA (carrying trailers + END_STREAM) was queued in pendingHeadersQueue but never drained after the response headers were read, so the body reader blocked in take() until the 30s read-timeout watchdog fired. deliverHeaders now completes the body on END_STREAM, and the consumer drains pending trailer events at EOF. 2. High-concurrency empty body: responseBody()'s empty-response fast path used (isEndStreamReceived() && streamBody.isEmpty()). The reader sets the END_STREAM flag before offering the trailing DATA chunk, so under load a concurrent reader could observe that pair as true-while-empty and drop the body. Switched to streamBody.isCompletedEmpty(), which only reports empty once complete() is published with no chunk queued.
Outbound event streams previously lost their message boundaries twice on the write path: DefaultEventStreamWriter.toDataStream() wrapped the per-event EventPipeStream queue as a plain InputStream, and H2StreamRequestBody drained it with asInputStream().transferTo(out) — byte-oriented, so small events were coalesced into the 16 KB frame buffer and not sent until it filled or the stream closed. A request/response event protocol (send a small message, await the peer's reply, then send the next) would deadlock: the message never left the buffer, so the reply never came. Fix, via the existing DataStream.writeTo seam (no new public API, no reactive types in the blocking client): - EventPipeStream.writeMessagesTo(out) drains one queued event ByteBuffer at a time and flushes after each, preserving boundaries. - DefaultEventStreamWriter.toDataStream() returns a DataStream whose writeTo routes through writeMessagesTo (and still works as a normal InputStream for any other consumer). - H2StreamRequestBody drives the streaming write through body.writeTo(out) instead of asInputStream().transferTo(out). The default writeTo is still transferTo, so bulk uploads (S3 etc.) are unchanged. Tests: EventPipeStreamTest.writeMessagesToFlushesOncePerEvent (unit, one flush per event) and PerMessageFlushHttp2Test (integ, verified discriminator — a ping-pong body that awaits each message's echo before sending the next; passes with per-message flush, deadlocks to the @timeout on the coalescing path).
mtdowling
force-pushed
the
vt-scratch-rebased
branch
from
681a999 to
cbaaf25
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
2 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.
What behavior changes?
Describe the observable difference in behavior before and after this change.
Add virtual thread friendly, blocking, Smithy HTTP client that supports HTTP 1.1 and HTTP 2 bidirectional streaming
Why is this change needed?
Explain the motivation: bug, feature request, refactor, performance, etc.
No blocking style HTTP client for Java supports H2 bidirectional streaming, and we need that for event streams.
How was this validated?
List tests added, benchmarks run, or manual verification performed.
Lots and lots of unit and integration tests.
What should reviewers focus on?
Point reviewers to the files or sections that contain the interesting logic.
Additional Links
Related issues, design docs, or prior art.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.