From d39597889e816f931481fcaebdd9a02dfbce0fd9 Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Wed, 10 Jun 2026 10:34:43 -0700 Subject: [PATCH 01/12] Add hybrid pull-pump for sync CRT client Move blocking InputStream.read() off the CRT event-loop thread to fix a deadlock when a PUT body is sourced from a GET InputStream that shares the same event loop. Caller thread runs a producer loop reading the body into a bounded chunk pool; CRT's pull callback drains via non-blocking pollDrain. Pool is allocated lazily after stream acquisition (depth=4 chunks, 128KB each) to avoid pinning heap on requests queued for a connection. --- .../amazon/awssdk/spotbugs-suppressions.xml | 5 + .../awssdk/http/crt/AwsCrtHttpClient.java | 61 ++- .../http/crt/internal/CrtRequestExecutor.java | 89 ++-- .../crt/internal/request/BodyChunkPipe.java | 241 +++++++++++ .../http/crt/internal/request/Chunk.java | 58 +++ .../internal/request/CrtRequestAdapter.java | 59 ++- .../request/CrtRequestInputStreamAdapter.java | 78 ---- .../request/PipeBackedRequestBodyStream.java | 49 +++ .../internal/request/SyncRequestBodyPump.java | 79 ++++ .../crt/AwsCrtHttpClientWireMockTest.java | 303 ++++++++++++++ .../crt/internal/CrtRequestExecutorTest.java | 10 +- .../internal/request/BodyChunkPipeTest.java | 396 ++++++++++++++++++ .../PipeBackedRequestBodyStreamTest.java | 141 +++++++ .../request/SyncRequestBodyPumpTest.java | 239 +++++++++++ .../S3WithCrtHttpClientsIntegrationTests.java | 22 +- 15 files changed, 1700 insertions(+), 130 deletions(-) create mode 100644 http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java create mode 100644 http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/Chunk.java delete mode 100644 http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/CrtRequestInputStreamAdapter.java create mode 100644 http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/PipeBackedRequestBodyStream.java create mode 100644 http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java create mode 100644 http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipeTest.java create mode 100644 http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/PipeBackedRequestBodyStreamTest.java create mode 100644 http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPumpTest.java diff --git a/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml b/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml index 05606dc6d574..0f6ab0d22c64 100644 --- a/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml +++ b/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml @@ -348,6 +348,11 @@ + + + diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java index 9c6d769e48fa..709cc9121528 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java @@ -24,6 +24,7 @@ import java.util.function.Consumer; import software.amazon.awssdk.annotations.SdkPublicApi; import software.amazon.awssdk.crt.http.HttpException; +import software.amazon.awssdk.crt.http.HttpStreamBase; import software.amazon.awssdk.crt.http.HttpStreamManager; import software.amazon.awssdk.http.ExecutableHttpRequest; import software.amazon.awssdk.http.HttpExecuteRequest; @@ -35,6 +36,7 @@ import software.amazon.awssdk.http.crt.internal.AwsCrtClientBuilderBase; import software.amazon.awssdk.http.crt.internal.CrtRequestContext; import software.amazon.awssdk.http.crt.internal.CrtRequestExecutor; +import software.amazon.awssdk.http.crt.internal.request.SyncRequestBodyPump; import software.amazon.awssdk.utils.AttributeMap; import software.amazon.awssdk.utils.CompletableFutureUtils; @@ -109,6 +111,7 @@ public ExecutableHttpRequest prepareRequest(HttpExecuteRequest request) { private static final class CrtHttpRequest implements ExecutableHttpRequest { private final CrtRequestContext context; private volatile CompletableFuture responseFuture; + private volatile SyncRequestBodyPump pump; private CrtHttpRequest(CrtRequestContext context) { this.context = context; @@ -119,7 +122,38 @@ public HttpExecuteResponse call() throws IOException { HttpExecuteResponse.Builder builder = HttpExecuteResponse.builder(); try { - responseFuture = new CrtRequestExecutor().execute(context); + CrtRequestExecutor.Result result = new CrtRequestExecutor().execute(context); + responseFuture = result.responseFuture(); + pump = result.pump(); + + // Abort the pump to unblock a parked producer when CRT signals request failure + // via responseFuture. + if (pump != null) { + SyncRequestBodyPump pumpRef = pump; + responseFuture.whenComplete((r, t) -> { + if (t != null) { + pumpRef.abort(); + } + }); + } + + boolean streamAcquired = waitForStreamAcquired(result.streamFuture()); + + // No body case (pump == null): CRT writes the request line + headers when the stream + // is acquired and emits no body callbacks. We only need to join responseFuture below. + if (pump != null) { + if (streamAcquired) { + try { + pump.pump(); + } catch (IOException ioe) { + responseFuture.completeExceptionally(ioe); + throw ioe; + } + } else { + pump.abort(); + } + } + SdkHttpFullResponse response = CompletableFutureUtils.joinInterruptibly(responseFuture); builder.response(response); builder.responseBody(response.content().orElse(null)); @@ -128,13 +162,17 @@ public HttpExecuteResponse call() throws IOException { Throwable cause = e.getCause(); // Complete the future exceptionally to trigger connection cleanup in the response handler. - // Handles thread-interrupt case where joinInterruptibly throws due to - // InterruptedException. Without this, the - // Ensures that closeConnection() is invoked to prevent leaking the connection from the pool. + // Handles the thread-interrupt case where joinInterruptibly throws due to + // InterruptedException, ensuring closeConnection() is invoked to prevent leaking the + // connection from the pool. if (responseFuture != null) { responseFuture.completeExceptionally(cause != null ? cause : e); } + if (pump != null) { + pump.abort(); + } + if (cause instanceof IOException) { throw (IOException) cause; } @@ -156,6 +194,21 @@ public void abort() { if (responseFuture != null) { responseFuture.completeExceptionally(new IOException("Request was cancelled")); } + if (pump != null) { + pump.abort(); + } + } + + private boolean waitForStreamAcquired(CompletableFuture streamFuture) { + if (streamFuture == null) { + return false; + } + try { + CompletableFutureUtils.joinInterruptibly(streamFuture); + return true; + } catch (CompletionException e) { + return false; + } } } diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java index 7165696435e1..9c3974843dbd 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java @@ -20,11 +20,12 @@ import java.util.concurrent.CompletableFuture; import software.amazon.awssdk.annotations.SdkInternalApi; -import software.amazon.awssdk.crt.http.HttpRequest; import software.amazon.awssdk.crt.http.HttpStreamBase; import software.amazon.awssdk.crt.http.HttpStreamBaseResponseHandler; import software.amazon.awssdk.http.SdkHttpFullResponse; import software.amazon.awssdk.http.crt.internal.request.CrtRequestAdapter; +import software.amazon.awssdk.http.crt.internal.request.CrtRequestAdapter.SyncCrtRequest; +import software.amazon.awssdk.http.crt.internal.request.SyncRequestBodyPump; import software.amazon.awssdk.http.crt.internal.response.InputStreamAdaptingHttpStreamResponseHandler; import software.amazon.awssdk.metrics.MetricCollector; import software.amazon.awssdk.metrics.NoOpMetricCollector; @@ -32,49 +33,71 @@ @SdkInternalApi public final class CrtRequestExecutor { - public CompletableFuture execute(CrtRequestContext executionContext) { + public Result execute(CrtRequestContext executionContext) { CompletableFuture requestFuture = new CompletableFuture<>(); + MetricCollector metricCollector = executionContext.metricCollector(); + boolean shouldPublishMetrics = metricCollector != null && !(metricCollector instanceof NoOpMetricCollector); + + // get acquireStartTime as early as possible for the concurrency timer, but only when metrics are + // enabled since clock_gettime() is a full sys call barrier (multiple mutexes and a hw interrupt). + long acquireStartTime = shouldPublishMetrics ? System.nanoTime() : 0; try { - doExecute(executionContext, requestFuture); + HttpStreamBaseResponseHandler crtResponseHandler = new InputStreamAdaptingHttpStreamResponseHandler(requestFuture); + SyncCrtRequest syncCrtRequest = CrtRequestAdapter.toCrtRequest(executionContext); + CompletableFuture streamFuture = + executionContext.streamManager().acquireStream(syncCrtRequest.httpRequest(), crtResponseHandler); + + long finalAcquireStartTime = acquireStartTime; + streamFuture.whenComplete((streamBase, throwable) -> { + if (shouldPublishMetrics) { + reportMetrics(executionContext.streamManager(), metricCollector, finalAcquireStartTime); + } + if (throwable != null) { + requestFuture.completeExceptionally(wrapCrtException(throwable)); + } + }); + + return new Result(requestFuture, syncCrtRequest.pump(), streamFuture); } catch (Throwable t) { requestFuture.completeExceptionally(t); + return new Result(requestFuture, null, null); } - - return requestFuture; } - private void doExecute(CrtRequestContext executionContext, CompletableFuture requestFuture) { - MetricCollector metricCollector = executionContext.metricCollector(); - boolean shouldPublishMetrics = metricCollector != null && !(metricCollector instanceof NoOpMetricCollector); - - long acquireStartTime = 0; - - if (shouldPublishMetrics) { - // go ahead and get acquireStartTime for the concurrency timer as early as possible, - // so it's as accurate as possible, but only do it in a branch since clock_gettime() - // results in a full sys call barrier (multiple mutexes and a hw interrupt). - acquireStartTime = System.nanoTime(); + /** + * Result of {@link #execute(CrtRequestContext)}: bundles the response future with the optional + * caller-thread body pump (null when the request has no body) and the future that completes + * when the CRT stream has been acquired from the connection pool. + */ + public static final class Result { + private final CompletableFuture responseFuture; + private final SyncRequestBodyPump pump; + private final CompletableFuture streamFuture; + + Result(CompletableFuture responseFuture, + SyncRequestBodyPump pump, + CompletableFuture streamFuture) { + this.responseFuture = responseFuture; + this.pump = pump; + this.streamFuture = streamFuture; } - HttpStreamBaseResponseHandler crtResponseHandler = new InputStreamAdaptingHttpStreamResponseHandler(requestFuture); - - HttpRequest crtRequest = CrtRequestAdapter.toCrtRequest(executionContext); - - CompletableFuture streamFuture = - executionContext.streamManager().acquireStream(crtRequest, crtResponseHandler); - - long finalAcquireStartTime = acquireStartTime; + public CompletableFuture responseFuture() { + return responseFuture; + } - streamFuture.whenComplete((streamBase, throwable) -> { - if (shouldPublishMetrics) { - reportMetrics(executionContext.streamManager(), metricCollector, finalAcquireStartTime); - } + public SyncRequestBodyPump pump() { + return pump; + } - if (throwable != null) { - Throwable toThrow = wrapCrtException(throwable); - requestFuture.completeExceptionally(toThrow); - } - }); + /** + * Future that completes when the CRT stream has been acquired (or acquisition has failed). + * The caller blocks on this before running the body pump so per-request body buffers are + * not allocated while a request is queued on the connection pool. + */ + public CompletableFuture streamFuture() { + return streamFuture; + } } } diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java new file mode 100644 index 000000000000..fdaed9a7bc6a --- /dev/null +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java @@ -0,0 +1,241 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.awssdk.http.crt.internal.request; + +import java.nio.ByteBuffer; +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.atomic.AtomicReference; +import software.amazon.awssdk.annotations.SdkInternalApi; +import software.amazon.awssdk.annotations.SdkTestInternalApi; + +/** + * Bounded producer/consumer hand-off between the caller thread (producer) and the CRT event-loop thread (consumer). + * + *

The producer reads from the customer's {@code InputStream} and {@link #publish(Chunk) publishes} chunks + * into a bounded {@link ArrayBlockingQueue}. The consumer drains those chunks via {@link #pollDrain(ByteBuffer)}, + * which is non-blocking: if no data is ready the consumer returns 0 bytes and CRT reschedules itself via + * {@code aws_channel_schedule_task_now}. + * + *

Drained chunks are returned to a free {@link ArrayDeque} (LIFO for cache hotness) guarded by this + * monitor. The producer parks on this monitor when the free deque is empty, providing back-pressure. + * + *

Chunk byte[] buffers are allocated lazily on the producer's first {@link #acquireForFill()}, not in + * the constructor. This keeps per-request heap minimal while a request is queued on the CRT connection + * pool waiting for a stream: the pipe object exists but its backing buffers do not. + * + *

State machine: {@code OPEN -> {EOF | ERROR | ABORTED}}. Transitions are one-way. + */ +@SdkInternalApi +final class BodyChunkPipe { + + enum State { + OPEN, + EOF, + ERROR, + ABORTED + } + + /** + * Defense-in-depth wait timeout for {@link #acquireForFill()}. Even if a code path forgets + * to call {@link #abort()}, a parked producer wakes every {@value} ms to re-check state. + * Spurious wakeups are harmless. + */ + private static final long ACQUIRE_WAIT_TIMEOUT_MS = 50L; + + private final int depth; + private final int chunkSize; + private final ArrayBlockingQueue ready; + private final Deque free; + private final AtomicReference state = new AtomicReference<>(State.OPEN); + /** + * Guards the free deque, allocated counter, and producer wait/notify protocol. Kept private + * so external code cannot synchronize on the pipe instance and stall the producer. + */ + private final Object freeLock = new Object(); + + private int allocated; + private volatile Throwable error; + private Chunk pendingDrain; + + BodyChunkPipe(int depth, int chunkSize) { + if (depth < 1) { + throw new IllegalArgumentException("depth must be >= 1"); + } + if (chunkSize < 1) { + throw new IllegalArgumentException("chunkSize must be >= 1"); + } + this.depth = depth; + this.chunkSize = chunkSize; + this.ready = new ArrayBlockingQueue<>(depth); + this.free = new ArrayDeque<>(depth); + } + + /** + * Producer side: acquire a chunk to fill. Blocks if all chunks are currently in flight. + * Returns {@code null} only if the pipe was aborted while the producer was waiting. + * + *

Allocates the chunk's backing byte[] on first use up to the configured depth. This keeps the + * per-request footprint minimal until the producer actually starts pumping (i.e., until after the + * CRT stream has been acquired). + */ + Chunk acquireForFill() throws InterruptedException { + synchronized (freeLock) { + while (true) { + State s = state.get(); + if (s == State.ABORTED || s == State.ERROR) { + return null; + } + Chunk c = free.pollFirst(); + if (c != null) { + return c; + } + if (allocated < depth) { + allocated++; + return new Chunk(chunkSize); + } + freeLock.wait(ACQUIRE_WAIT_TIMEOUT_MS); + } + } + } + + /** + * Producer side: publish a filled chunk to the consumer. Caller must have set + * {@link Chunk#len(int)} before calling. + * + *

If the chunk is empty (zero-length read), it is recycled back to the free deque rather than + * pushed to the ready queue: an empty chunk would otherwise be leaked from the bounded pool, and + * the consumer would interpret it as a no-op anyway. + */ + void publish(Chunk chunk) throws InterruptedException { + if (chunk.len() == 0) { + recycle(chunk); + return; + } + // ready.put() blocks if the queue is full, but the queue capacity == pool size, + // so this can only block briefly while the consumer drains. + ready.put(chunk); + } + + /** + * Producer side: signal end-of-stream. Idempotent. + */ + void signalEof() { + state.compareAndSet(State.OPEN, State.EOF); + } + + /** + * Producer side: signal a fatal producer-side error. Idempotent. + */ + void signalError(Throwable t) { + synchronized (freeLock) { + // Publish the cause BEFORE flipping state so a consumer's lock-free read in pollDrain + // never observes state==ERROR with error==null. The volatile write to `error` is + // harmless if the CAS later loses (idempotent signal). + error = t; + state.compareAndSet(State.OPEN, State.ERROR); + freeLock.notifyAll(); + } + } + + /** + * External-cancel: clear ready queue, flip state, wake producer. + */ + void abort() { + synchronized (freeLock) { + if (state.compareAndSet(State.OPEN, State.ABORTED)) { + ready.clear(); + } + freeLock.notifyAll(); + } + } + + /** + * Consumer side: drain bytes into {@code dst}. NEVER blocks. + * + *

Single-consumer: CRT invokes this only on the request's outgoing-stream task, which is + * scheduled serially on one event-loop thread per stream. {@code pendingDrain} is therefore + * not volatile - it is written and read by that single consumer thread. + * + * @return number of bytes drained, or {@code -1} on EOF with no remaining data. + * @throws RuntimeException if the pipe is in ERROR or ABORTED state with no remaining data. + */ + int pollDrain(ByteBuffer dst) { + int totalBytesConsumed = 0; + while (dst.hasRemaining()) { + if (pendingDrain == null) { + pendingDrain = ready.poll(); + } + if (pendingDrain == null) { + switch (state.get()) { + case ERROR: + throw new RuntimeException("Producer failed", error); + case ABORTED: + throw new RuntimeException("Request body stream was aborted"); + case EOF: + return totalBytesConsumed > 0 ? totalBytesConsumed : -1; + case OPEN: + default: + // OPEN with empty queue: return what we have (possibly 0); CRT will retry. + return totalBytesConsumed; + } + } + int n = Math.min(dst.remaining(), pendingDrain.len() - pendingDrain.pos()); + dst.put(pendingDrain.data(), pendingDrain.pos(), n); + pendingDrain.pos(pendingDrain.pos() + n); + totalBytesConsumed += n; + if (pendingDrain.pos() >= pendingDrain.len()) { + // The chunk has been fully copied into dst, so we return it to the free deque + // (and notify the producer in case it was waiting). This is what bounds the pool: + // chunks only re-enter the producer pool after the consumer has drained them. + Chunk drained = pendingDrain; + pendingDrain = null; + recycle(drained); + } + } + return totalBytesConsumed; + } + + /** + * Visible-for-test / test-only helper: current pipe state. + */ + @SdkTestInternalApi + State state() { + return state.get(); + } + + /** + * Visible-for-test / test-only helper: number of {@link Chunk} buffers minted so far. The pipe + * lazily allocates chunks on the producer's first {@link #acquireForFill()}, so this is 0 until + * the producer starts pumping and grows up to {@code depth}. + */ + @SdkTestInternalApi + int allocatedForTest() { + synchronized (freeLock) { + return allocated; + } + } + + private void recycle(Chunk c) { + c.pos(0); + c.len(0); + synchronized (freeLock) { + free.push(c); + freeLock.notifyAll(); + } + } +} diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/Chunk.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/Chunk.java new file mode 100644 index 000000000000..a7e97b292aab --- /dev/null +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/Chunk.java @@ -0,0 +1,58 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.awssdk.http.crt.internal.request; + +import software.amazon.awssdk.annotations.SdkInternalApi; + +/** + * A reusable byte buffer carrying request body data through the {@link BodyChunkPipe}. + * + *

Internal to this package; used only by {@link BodyChunkPipe} and {@link SyncRequestBodyPump}. + * + *

{@code pos} and {@code len} are intentionally non-volatile: hand-off between the producer and + * consumer always goes through the {@code ArrayBlockingQueue} (or the {@code freeLock} monitor on + * recycle), both of which provide release/acquire happens-before for the field writes. + */ +@SdkInternalApi +final class Chunk { + private final byte[] data; + private int pos; + private int len; + + Chunk(int chunkSize) { + this.data = new byte[chunkSize]; + } + + byte[] data() { + return data; + } + + int pos() { + return pos; + } + + void pos(int pos) { + this.pos = pos; + } + + int len() { + return len; + } + + void len(int len) { + this.len = len; + } +} diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/CrtRequestAdapter.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/CrtRequestAdapter.java index 8672d80b0d1b..4a3991c5d44a 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/CrtRequestAdapter.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/CrtRequestAdapter.java @@ -23,6 +23,7 @@ import software.amazon.awssdk.crt.http.HttpHeader; import software.amazon.awssdk.crt.http.HttpRequest; import software.amazon.awssdk.crt.http.HttpRequestBase; +import software.amazon.awssdk.http.ContentStreamProvider; import software.amazon.awssdk.http.Header; import software.amazon.awssdk.http.HttpExecuteRequest; import software.amazon.awssdk.http.Protocol; @@ -33,6 +34,16 @@ @SdkInternalApi public final class CrtRequestAdapter { + /** + * Per-chunk size used by the sync request-body pipe. + */ + private static final int CHUNK_SIZE = 128 * 1024; + + /** + * Number of in-flight chunks the pipe holds. + */ + private static final int PIPE_DEPTH = 4; + private CrtRequestAdapter() { } @@ -60,7 +71,12 @@ public static HttpRequestBase toAsyncCrtRequest(CrtAsyncRequestContext request) crtRequestBodyAdapter); } - public static HttpRequest toCrtRequest(CrtRequestContext request) { + /** + * Build the CRT request for the sync path. When the SDK request has a body, this also constructs the + * {@link BodyChunkPipe} and a {@link SyncRequestBodyPump}; the caller thread is expected to drive + * the pump after the stream is activated. + */ + public static SyncCrtRequest toCrtRequest(CrtRequestContext request) { HttpExecuteRequest sdkExecuteRequest = request.sdkRequest(); SdkHttpRequest sdkRequest = sdkExecuteRequest.httpRequest(); @@ -78,14 +94,39 @@ public static HttpRequest toCrtRequest(CrtRequestContext request) { HttpHeader[] crtHeaderArray = asArray(createHttpHeaderList(sdkRequest.getUri(), sdkExecuteRequest)); String finalEncodedPath = encodedPath + encodedQueryString; - return sdkExecuteRequest.contentStreamProvider() - .map(provider -> new HttpRequest(method, - finalEncodedPath, - crtHeaderArray, - new CrtRequestInputStreamAdapter(provider))) - .orElse(new HttpRequest(method, - finalEncodedPath, - crtHeaderArray, null)); + + Optional providerOpt = sdkExecuteRequest.contentStreamProvider(); + if (!providerOpt.isPresent()) { + return new SyncCrtRequest(new HttpRequest(method, finalEncodedPath, crtHeaderArray, null), null); + } + + BodyChunkPipe pipe = new BodyChunkPipe(PIPE_DEPTH, CHUNK_SIZE); + PipeBackedRequestBodyStream bodyStream = new PipeBackedRequestBodyStream(pipe); + SyncRequestBodyPump pump = new SyncRequestBodyPump(providerOpt.get(), pipe); + HttpRequest crtRequest = new HttpRequest(method, finalEncodedPath, crtHeaderArray, bodyStream); + return new SyncCrtRequest(crtRequest, pump); + } + + /** + * Holder returned from {@link #toCrtRequest(CrtRequestContext)} bundling the CRT-side request and the + * caller-thread producer pump (null when the SDK request has no body). + */ + public static final class SyncCrtRequest { + private final HttpRequest httpRequest; + private final SyncRequestBodyPump pump; + + SyncCrtRequest(HttpRequest httpRequest, SyncRequestBodyPump pump) { + this.httpRequest = httpRequest; + this.pump = pump; + } + + public HttpRequest httpRequest() { + return httpRequest; + } + + public SyncRequestBodyPump pump() { + return pump; + } } private static HttpHeader[] asArray(List crtHeaderList) { diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/CrtRequestInputStreamAdapter.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/CrtRequestInputStreamAdapter.java deleted file mode 100644 index 68f418b9e1df..000000000000 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/CrtRequestInputStreamAdapter.java +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"). - * You may not use this file except in compliance with the License. - * A copy of the License is located at - * - * http://aws.amazon.com/apache2.0 - * - * or in the "license" file accompanying this file. This file is distributed - * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either - * express or implied. See the License for the specific language governing - * permissions and limitations under the License. - */ - -package software.amazon.awssdk.http.crt.internal.request; - -import static java.lang.Math.min; - -import java.io.IOException; -import java.io.InputStream; -import java.nio.ByteBuffer; -import software.amazon.awssdk.annotations.SdkInternalApi; -import software.amazon.awssdk.crt.http.HttpRequestBodyStream; -import software.amazon.awssdk.http.ContentStreamProvider; - -@SdkInternalApi -final class CrtRequestInputStreamAdapter implements HttpRequestBodyStream { - private static final int READ_BUFFER_SIZE = 16 * 1024; - - private final ContentStreamProvider provider; - private volatile InputStream providerStream; - private final byte[] readBuffer = new byte[READ_BUFFER_SIZE]; - - CrtRequestInputStreamAdapter(ContentStreamProvider provider) { - this.provider = provider; - } - - @Override - public boolean sendRequestBody(ByteBuffer bodyBytesOut) { - int read; - - try { - if (providerStream == null) { - createNewStream(); - } - - int toRead = min(READ_BUFFER_SIZE, bodyBytesOut.remaining()); - read = providerStream.read(readBuffer, 0, toRead); - - if (read > 0) { - bodyBytesOut.put(readBuffer, 0, read); - } - } catch (IOException ioe) { - throw new RuntimeException(ioe); - } - - return read < 0; - } - - @Override - public boolean resetPosition() { - try { - createNewStream(); - } catch (IOException ioe) { - throw new RuntimeException(ioe); - } - - return true; - } - - private void createNewStream() throws IOException { - if (providerStream != null) { - providerStream.close(); - } - providerStream = provider.newStream(); - } -} diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/PipeBackedRequestBodyStream.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/PipeBackedRequestBodyStream.java new file mode 100644 index 000000000000..a2dd78eea0c4 --- /dev/null +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/PipeBackedRequestBodyStream.java @@ -0,0 +1,49 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.awssdk.http.crt.internal.request; + +import java.nio.ByteBuffer; +import software.amazon.awssdk.annotations.SdkInternalApi; +import software.amazon.awssdk.crt.http.HttpRequestBodyStream; + +/** + * A {@link HttpRequestBodyStream} adapter whose {@link #sendRequestBody(ByteBuffer)} drains bytes from a + * {@link BodyChunkPipe} that is fed by the caller thread. The pull callback NEVER blocks: if no data is ready, + * it returns 0 bytes and CRT reschedules the outgoing-stream task via {@code aws_channel_schedule_task_now}, + * allowing other event-loop tasks (such as a concurrent GET response delivery) to run before the retry. + */ +@SdkInternalApi +final class PipeBackedRequestBodyStream implements HttpRequestBodyStream { + + private final BodyChunkPipe pipe; + + PipeBackedRequestBodyStream(BodyChunkPipe pipe) { + this.pipe = pipe; + } + + @Override + public boolean sendRequestBody(ByteBuffer bodyBytesOut) { + int drained = pipe.pollDrain(bodyBytesOut); + return drained < 0; + } + + @Override + public boolean resetPosition() { + // The SDK retry layer (RetryableStage) handles request-level retries by calling prepareRequest() again, + // CRT does not currently exercise resetPosition for HTTP/1.1, so opting out is safe in practice. + return false; + } +} diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java new file mode 100644 index 000000000000..dbacf58eedee --- /dev/null +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java @@ -0,0 +1,79 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.awssdk.http.crt.internal.request; + +import java.io.IOException; +import java.io.InputStream; +import software.amazon.awssdk.annotations.SdkInternalApi; +import software.amazon.awssdk.http.ContentStreamProvider; + +/** + * Caller-thread producer that reads from the customer's {@link InputStream} and publishes chunks to a + * {@link BodyChunkPipe}. Runs on the caller (sync) thread between stream activation and + * {@code responseFuture.join()}, ensuring the blocking {@code read()} happens off the CRT event loop. + */ +@SdkInternalApi +public final class SyncRequestBodyPump { + + private final ContentStreamProvider contentStreamProvider; + private final BodyChunkPipe pipe; + + SyncRequestBodyPump(ContentStreamProvider contentStreamProvider, BodyChunkPipe pipe) { + this.contentStreamProvider = contentStreamProvider; + this.pipe = pipe; + } + + /** + * Pump the entire input stream into the pipe. Runs on the caller thread; never invoked on the CRT + * event-loop thread. On EOF signals the pipe normally; on {@link IOException} signals an error and rethrows. + */ + public void pump() throws IOException { + try (InputStream in = contentStreamProvider.newStream()) { + while (true) { + Chunk chunk = pipe.acquireForFill(); + if (chunk == null) { + // pipe was aborted while we were waiting; stop without signaling EOF. + return; + } + int read; + try { + read = in.read(chunk.data(), 0, chunk.data().length); + } catch (IOException ioe) { + pipe.signalError(ioe); + throw ioe; + } + if (read < 0) { + pipe.signalEof(); + return; + } + chunk.pos(0); + chunk.len(read); + pipe.publish(chunk); + } + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + pipe.abort(); + throw new IOException("Interrupted while writing request body", ie); + } + } + + /** + * Abort the underlying pipe (e.g., when the caller's {@code call()} is cancelled). + */ + public void abort() { + pipe.abort(); + } +} diff --git a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/AwsCrtHttpClientWireMockTest.java b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/AwsCrtHttpClientWireMockTest.java index ce5d778f06a1..5171451a4f04 100644 --- a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/AwsCrtHttpClientWireMockTest.java +++ b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/AwsCrtHttpClientWireMockTest.java @@ -17,8 +17,13 @@ import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; import static com.github.tomakehurst.wiremock.client.WireMock.any; +import static com.github.tomakehurst.wiremock.client.WireMock.equalTo; +import static com.github.tomakehurst.wiremock.client.WireMock.equalToIgnoreCase; +import static com.github.tomakehurst.wiremock.client.WireMock.put; +import static com.github.tomakehurst.wiremock.client.WireMock.putRequestedFor; import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo; +import static com.github.tomakehurst.wiremock.client.WireMock.verify; import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; import static org.apache.commons.lang3.RandomStringUtils.randomAlphabetic; import static org.assertj.core.api.Assertions.assertThat; @@ -26,13 +31,21 @@ import static software.amazon.awssdk.http.SdkHttpConfigurationOption.PROTOCOL; import static software.amazon.awssdk.http.crt.CrtHttpClientTestUtils.createRequest; +import com.github.tomakehurst.wiremock.http.Fault; import com.github.tomakehurst.wiremock.junit.WireMockRule; import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.InputStream; import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Rule; @@ -45,6 +58,8 @@ import software.amazon.awssdk.http.HttpMetric; import software.amazon.awssdk.http.Protocol; import software.amazon.awssdk.http.SdkHttpClient; +import software.amazon.awssdk.http.SdkHttpFullRequest; +import software.amazon.awssdk.http.SdkHttpMethod; import software.amazon.awssdk.http.SdkHttpRequest; import software.amazon.awssdk.metrics.MetricCollection; import software.amazon.awssdk.metrics.MetricCollector; @@ -133,6 +148,294 @@ public void abortRequest_shouldFailTheExceptionWithIOException() throws Exceptio } } + @Test + public void putRequest_withInputStreamBody_serverReceivesBody() throws Exception { + try (SdkHttpClient client = AwsCrtHttpClient.create()) { + String body = "hello pull pump"; + byte[] bodyBytes = body.getBytes(StandardCharsets.UTF_8); + URI uri = URI.create("http://localhost:" + mockServer.port()); + stubFor(put(urlPathEqualTo("/sink")).willReturn(aResponse().withStatus(200))); + + SdkHttpFullRequest request = SdkHttpFullRequest.builder() + .uri(uri) + .method(SdkHttpMethod.PUT) + .encodedPath("/sink") + .putHeader("Host", uri.getHost()) + .putHeader("Content-Length", Integer.toString(bodyBytes.length)) + .build(); + + HttpExecuteRequest executeRequest = HttpExecuteRequest.builder() + .request(request) + .contentStreamProvider(() -> new ByteArrayInputStream(bodyBytes)) + .build(); + + HttpExecuteResponse response = client.prepareRequest(executeRequest).call(); + + assertThat(response.httpResponse().statusCode()).isEqualTo(200); + verify(putRequestedFor(urlPathEqualTo("/sink")) + .withHeader("Content-Length", equalTo(Integer.toString(bodyBytes.length))) + .withRequestBody(equalToIgnoreCase(body))); + } + } + + @Test + public void inputStreamThrows_connectionReturnedToPool_subsequentRequestSucceeds() throws Exception { + // Bound the pool to a single connection: if the failed request leaks its connection, the + // second call() either fails to acquire (with the explicit timeout below) or blocks until + // the test framework times out. Either manifests as a deterministic failure rather than a hang. + try (SdkHttpClient client = AwsCrtHttpClient.builder() + .maxConcurrency(1) + .connectionAcquisitionTimeout(Duration.ofSeconds(10)) + .build()) { + URI uri = URI.create("http://localhost:" + mockServer.port()); + stubFor(put(urlPathEqualTo("/sink")).willReturn(aResponse().withStatus(200))); + + IOException expected = new IOException("simulated upstream failure"); + SdkHttpFullRequest failingRequest = SdkHttpFullRequest.builder() + .uri(uri) + .method(SdkHttpMethod.PUT) + .encodedPath("/sink") + .putHeader("Host", uri.getHost()) + .putHeader("Content-Length", "100") + .build(); + HttpExecuteRequest failingExecute = + HttpExecuteRequest.builder() + .request(failingRequest) + .contentStreamProvider(() -> new InputStream() { + @Override + public int read() throws IOException { + throw expected; + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + throw expected; + } + }) + .build(); + + assertThatThrownBy(() -> client.prepareRequest(failingExecute).call()) + .isInstanceOf(IOException.class); + + // If the previous failure leaked the connection, this second call would fail to acquire + // (bounded by the connectionAcquisitionTimeout configured above) instead of hanging. + String body = "second request body"; + byte[] bodyBytes = body.getBytes(StandardCharsets.UTF_8); + SdkHttpFullRequest okRequest = SdkHttpFullRequest.builder() + .uri(uri) + .method(SdkHttpMethod.PUT) + .encodedPath("/sink") + .putHeader("Host", uri.getHost()) + .putHeader("Content-Length", Integer.toString(bodyBytes.length)) + .build(); + HttpExecuteRequest okExecute = HttpExecuteRequest.builder() + .request(okRequest) + .contentStreamProvider(() -> new ByteArrayInputStream(bodyBytes)) + .build(); + + HttpExecuteResponse response = client.prepareRequest(okExecute).call(); + assertThat(response.httpResponse().statusCode()).isEqualTo(200); + } + } + + @Test + public void abortMidRequest_connectionReturnedToPool_subsequentRequestSucceeds() throws Exception { + try (SdkHttpClient client = AwsCrtHttpClient.builder() + .maxConcurrency(1) + .connectionAcquisitionTimeout(Duration.ofSeconds(10)) + .build()) { + URI uri = URI.create("http://localhost:" + mockServer.port()); + stubFor(any(urlPathEqualTo("/")).willReturn(aResponse().withFixedDelay(2000).withBody("hello"))); + stubFor(put(urlPathEqualTo("/sink")).willReturn(aResponse().withStatus(200))); + + SdkHttpRequest delayedRequest = createRequest(uri); + HttpExecuteRequest delayedExecute = HttpExecuteRequest.builder() + .request(delayedRequest) + .contentStreamProvider(() -> new ByteArrayInputStream(new byte[0])) + .build(); + + ExecutableHttpRequest abortable = client.prepareRequest(delayedExecute); + executorService.schedule(abortable::abort, 100, TimeUnit.MILLISECONDS); + assertThatThrownBy(abortable::call).isInstanceOf(IOException.class).hasMessageContaining("cancelled"); + + String body = "after abort"; + byte[] bodyBytes = body.getBytes(StandardCharsets.UTF_8); + SdkHttpFullRequest okRequest = SdkHttpFullRequest.builder() + .uri(uri) + .method(SdkHttpMethod.PUT) + .encodedPath("/sink") + .putHeader("Host", uri.getHost()) + .putHeader("Content-Length", Integer.toString(bodyBytes.length)) + .build(); + HttpExecuteRequest okExecute = HttpExecuteRequest.builder() + .request(okRequest) + .contentStreamProvider(() -> new ByteArrayInputStream(bodyBytes)) + .build(); + HttpExecuteResponse response = client.prepareRequest(okExecute).call(); + assertThat(response.httpResponse().statusCode()).isEqualTo(200); + } + } + + @Test + public void serverResetsConnection_connectionReturnedToPool_subsequentRequestSucceeds() throws Exception { + try (SdkHttpClient client = AwsCrtHttpClient.builder() + .maxConcurrency(1) + .connectionAcquisitionTimeout(Duration.ofSeconds(10)) + .build()) { + URI uri = URI.create("http://localhost:" + mockServer.port()); + stubFor(put(urlPathEqualTo("/sink")) + .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))); + + byte[] bodyBytes = randomAlphabetic(64).getBytes(StandardCharsets.UTF_8); + SdkHttpFullRequest failingRequest = SdkHttpFullRequest.builder() + .uri(uri) + .method(SdkHttpMethod.PUT) + .encodedPath("/sink") + .putHeader("Host", uri.getHost()) + .putHeader("Content-Length", Integer.toString(bodyBytes.length)) + .build(); + HttpExecuteRequest failingExecute = HttpExecuteRequest.builder() + .request(failingRequest) + .contentStreamProvider(() -> new ByteArrayInputStream(bodyBytes)) + .build(); + + assertThatThrownBy(() -> client.prepareRequest(failingExecute).call()) + .isInstanceOf(IOException.class); + + stubFor(put(urlPathEqualTo("/sink2")).willReturn(aResponse().withStatus(200))); + byte[] okBytes = "ok".getBytes(StandardCharsets.UTF_8); + SdkHttpFullRequest okRequest = SdkHttpFullRequest.builder() + .uri(uri) + .method(SdkHttpMethod.PUT) + .encodedPath("/sink2") + .putHeader("Host", uri.getHost()) + .putHeader("Content-Length", Integer.toString(okBytes.length)) + .build(); + HttpExecuteRequest okExecute = HttpExecuteRequest.builder() + .request(okRequest) + .contentStreamProvider(() -> new ByteArrayInputStream(okBytes)) + .build(); + + HttpExecuteResponse response = client.prepareRequest(okExecute).call(); + assertThat(response.httpResponse().statusCode()).isEqualTo(200); + } + } + + /** + * Regression test for the deadlock the pull-pump fix addresses. On master, the request body's + * {@code InputStream.read(...)} ran on the CRT event-loop thread (via the body callback), which + * meant a body sourced from a {@code GET}'s {@code ResponseInputStream} on the same event loop + * could deadlock: the GET held the event loop while the PUT body waited for it. + * + *

Pull-pump moves the read to the caller (sync) thread. This test verifies that load-bearing + * claim by recording the thread that performs the body read and asserting it is the caller + * thread - not a CRT event-loop thread. Failure of either the assertion or the test timeout + * (a hang) is the deadlock signal. + */ + @Test + public void putBodyReadHappensOnCallerThread_notOnCrtEventLoop() throws Exception { + try (SdkHttpClient client = AwsCrtHttpClient.builder() + .maxConcurrency(1) + .connectionAcquisitionTimeout(Duration.ofSeconds(10)) + .build()) { + URI uri = URI.create("http://localhost:" + mockServer.port()); + stubFor(put(urlPathEqualTo("/sink")).willReturn(aResponse().withStatus(200))); + + byte[] bodyBytes = "body-on-caller".getBytes(StandardCharsets.UTF_8); + AtomicReference readThreadName = new AtomicReference<>(); + SdkHttpFullRequest request = SdkHttpFullRequest.builder() + .uri(uri) + .method(SdkHttpMethod.PUT) + .encodedPath("/sink") + .putHeader("Host", uri.getHost()) + .putHeader("Content-Length", Integer.toString(bodyBytes.length)) + .build(); + HttpExecuteRequest executeRequest = + HttpExecuteRequest.builder() + .request(request) + .contentStreamProvider(() -> new ByteArrayInputStream(bodyBytes) { + @Override + public synchronized int read(byte[] b, int off, int len) { + readThreadName.compareAndSet(null, Thread.currentThread().getName()); + return super.read(b, off, len); + } + }) + .build(); + + String callerThreadName = Thread.currentThread().getName(); + HttpExecuteResponse response = client.prepareRequest(executeRequest).call(); + assertThat(response.httpResponse().statusCode()).isEqualTo(200); + + String observed = readThreadName.get(); + assertThat(observed) + .as("body read should happen on the caller thread, not the CRT event loop") + .isNotNull() + .isEqualTo(callerThreadName) + .doesNotContainIgnoringCase("AwsEventLoop") + .doesNotContainIgnoringCase("aws-event-loop"); + } + } + + /** + * Stress companion to {@link #putBodyReadHappensOnCallerThread_notOnCrtEventLoop}. Issues a + * delayed GET (response delayed server-side) and a PUT in parallel through the same + * {@code maxConcurrency(1)} client. On master, sequencing them through a single connection + * with the body read tied to the event-loop thread could deadlock; here both calls must + * complete within the test timeout. + */ + @Test + public void getInFlight_concurrentPut_bothComplete() throws Exception { + try (SdkHttpClient client = AwsCrtHttpClient.builder() + .maxConcurrency(1) + .connectionAcquisitionTimeout(Duration.ofSeconds(15)) + .build()) { + URI uri = URI.create("http://localhost:" + mockServer.port()); + stubFor(any(urlPathEqualTo("/slow")) + .willReturn(aResponse().withFixedDelay(2_000).withBody("hello"))); + stubFor(put(urlPathEqualTo("/sink")).willReturn(aResponse().withStatus(200))); + + SdkHttpRequest getRequest = SdkHttpFullRequest.builder() + .uri(uri) + .method(SdkHttpMethod.GET) + .encodedPath("/slow") + .putHeader("Host", uri.getHost()) + .build(); + HttpExecuteRequest getExecute = HttpExecuteRequest.builder() + .request(getRequest) + .contentStreamProvider(() -> new ByteArrayInputStream(new byte[0])) + .build(); + + byte[] putBytes = "put-body".getBytes(StandardCharsets.UTF_8); + SdkHttpFullRequest putRequest = SdkHttpFullRequest.builder() + .uri(uri) + .method(SdkHttpMethod.PUT) + .encodedPath("/sink") + .putHeader("Host", uri.getHost()) + .putHeader("Content-Length", Integer.toString(putBytes.length)) + .build(); + HttpExecuteRequest putExecute = HttpExecuteRequest.builder() + .request(putRequest) + .contentStreamProvider(() -> new ByteArrayInputStream(putBytes)) + .build(); + + ExecutorService pool = Executors.newFixedThreadPool(2); + try { + Callable getTask = () -> client.prepareRequest(getExecute).call(); + Callable putTask = () -> client.prepareRequest(putExecute).call(); + Future getFuture = pool.submit(getTask); + Future putFuture = pool.submit(putTask); + + HttpExecuteResponse getResponse = getFuture.get(15, TimeUnit.SECONDS); + HttpExecuteResponse putResponse = putFuture.get(15, TimeUnit.SECONDS); + assertThat(getResponse.httpResponse().statusCode()).isEqualTo(200); + assertThat(putResponse.httpResponse().statusCode()).isEqualTo(200); + } finally { + pool.shutdownNow(); + pool.awaitTermination(5, TimeUnit.SECONDS); + } + } + } + /** * Make a simple request and wait for it to finish. * diff --git a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutorTest.java b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutorTest.java index 456000ac1150..8b916ba1ee98 100644 --- a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutorTest.java +++ b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutorTest.java @@ -87,7 +87,7 @@ public void execute_requestConversionFails_failsFuture() { .request(HttpExecuteRequest.builder().build()) .build(); - CompletableFuture executeFuture = requestExecutor.execute(context); + CompletableFuture executeFuture = requestExecutor.execute(context).responseFuture(); assertThat(executeFuture).hasFailedWithThrowableThat().isInstanceOf(NullPointerException.class); } @@ -102,7 +102,7 @@ public void execute_acquireStreamFails_wrapsWithIOException() { .thenReturn(completableFuture); completableFuture.completeExceptionally(exception); - CompletableFuture executeFuture = requestExecutor.execute(context); + CompletableFuture executeFuture = requestExecutor.execute(context).responseFuture(); assertThat(executeFuture).hasFailedWithThrowableThat().hasCause(exception).isInstanceOf(IOException.class); } @@ -116,7 +116,7 @@ public void execute_retryableException_wrapsWithIOException(Throwable throwable) Mockito.when(streamManager.acquireStream(Mockito.any(HttpRequest.class), Mockito.any(HttpStreamBaseResponseHandler.class))) .thenReturn(completableFuture); - CompletableFuture executeFuture = requestExecutor.execute(context); + CompletableFuture executeFuture = requestExecutor.execute(context).responseFuture(); assertThat(executeFuture).hasFailedWithThrowableThat().hasCause(throwable).isInstanceOf(IOException.class); } @@ -133,7 +133,7 @@ public void execute_httpException_mapsToCorrectException(Entry executeFuture = requestExecutor.execute(context); + CompletableFuture executeFuture = requestExecutor.execute(context).responseFuture(); assertThatThrownBy(executeFuture::join).hasCauseInstanceOf(expectedExceptionClass); } @@ -146,7 +146,7 @@ public void execute_nonRetryableHttpException_doesNotWrapWithIOException() { Mockito.when(streamManager.acquireStream(Mockito.any(HttpRequest.class), Mockito.any(HttpStreamBaseResponseHandler.class))) .thenReturn(completableFuture); - CompletableFuture executeFuture = requestExecutor.execute(context); + CompletableFuture executeFuture = requestExecutor.execute(context).responseFuture(); assertThatThrownBy(executeFuture::join).hasCause(exception); } diff --git a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipeTest.java b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipeTest.java new file mode 100644 index 000000000000..72f29278589b --- /dev/null +++ b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipeTest.java @@ -0,0 +1,396 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.awssdk.http.crt.internal.request; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.nio.ByteBuffer; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.jupiter.api.RepeatedTest; +import org.junit.jupiter.api.Test; + +class BodyChunkPipeTest { + + @Test + void pollDrain_emptyOpenPipe_returnsZero() { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + ByteBuffer dst = ByteBuffer.allocate(8); + + int n = pipe.pollDrain(dst); + + assertThat(n).isZero(); + assertThat(dst.position()).isZero(); + } + + @Test + void pollDrain_afterEofWithEmptyQueue_returnsMinusOne() { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + pipe.signalEof(); + + int n = pipe.pollDrain(ByteBuffer.allocate(8)); + + assertThat(n).isEqualTo(-1); + } + + @Test + void publish_thenDrain_consumerSeesProducerBytes() throws Exception { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + Chunk c = pipe.acquireForFill(); + byte[] payload = {1, 2, 3, 4, 5}; + System.arraycopy(payload, 0, c.data(), 0, payload.length); + c.pos(0); + c.len(payload.length); + pipe.publish(c); + pipe.signalEof(); + ByteBuffer dst = ByteBuffer.allocate(16); + + int first = pipe.pollDrain(dst); + int second = pipe.pollDrain(dst); + + assertThat(first).isEqualTo(payload.length); + assertThat(second).isEqualTo(-1); + dst.flip(); + byte[] out = new byte[dst.remaining()]; + dst.get(out); + assertThat(out).containsExactly(payload); + } + + @Test + void signalError_pollDrainThrows() { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + pipe.signalError(new RuntimeException("boom")); + + assertThatThrownBy(() -> pipe.pollDrain(ByteBuffer.allocate(8))) + .hasMessageContaining("Producer failed") + .hasRootCauseMessage("boom"); + } + + @Test + void abort_emptiesReadyAndChangesState() throws Exception { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + Chunk c = pipe.acquireForFill(); + c.len(4); + pipe.publish(c); + + pipe.abort(); + + assertThatThrownBy(() -> pipe.pollDrain(ByteBuffer.allocate(8))) + .hasMessageContaining("aborted"); + } + + @Test + void pollDrain_signalErrorWithQueuedChunks_drainsThenThrows() throws Exception { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + Chunk c = pipe.acquireForFill(); + byte[] payload = {7, 8, 9}; + System.arraycopy(payload, 0, c.data(), 0, payload.length); + c.len(payload.length); + pipe.publish(c); + pipe.signalError(new RuntimeException("boom")); + + ByteBuffer dst = ByteBuffer.allocate(payload.length); + int drained = pipe.pollDrain(dst); + + assertThat(drained).isEqualTo(payload.length); + dst.flip(); + byte[] out = new byte[dst.remaining()]; + dst.get(out); + assertThat(out).containsExactly(payload); + assertThatThrownBy(() -> pipe.pollDrain(ByteBuffer.allocate(8))) + .hasMessageContaining("Producer failed") + .hasRootCauseMessage("boom"); + } + + @Test + void pollDrain_signalEofWithQueuedChunks_drainsThenReturnsMinusOne() throws Exception { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + Chunk c = pipe.acquireForFill(); + byte[] payload = {10, 20, 30}; + System.arraycopy(payload, 0, c.data(), 0, payload.length); + c.len(payload.length); + pipe.publish(c); + pipe.signalEof(); + + ByteBuffer dst = ByteBuffer.allocate(payload.length); + int drained = pipe.pollDrain(dst); + int afterDrain = pipe.pollDrain(ByteBuffer.allocate(8)); + + assertThat(drained).isEqualTo(payload.length); + dst.flip(); + byte[] out = new byte[dst.remaining()]; + dst.get(out); + assertThat(out).containsExactly(payload); + assertThat(afterDrain).isEqualTo(-1); + } + + @Test + void abort_afterSignalEof_leavesStateAsEof() { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + pipe.signalEof(); + + pipe.abort(); + + assertThat(pipe.state()).isEqualTo(BodyChunkPipe.State.EOF); + assertThat(pipe.pollDrain(ByteBuffer.allocate(8))).isEqualTo(-1); + } + + @Test + void abort_afterSignalEofWithQueuedChunks_doesNotClearReady() throws Exception { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + Chunk c = pipe.acquireForFill(); + byte[] payload = {1, 2, 3}; + System.arraycopy(payload, 0, c.data(), 0, payload.length); + c.len(payload.length); + pipe.publish(c); + pipe.signalEof(); + + pipe.abort(); + + assertThat(pipe.state()).isEqualTo(BodyChunkPipe.State.EOF); + ByteBuffer dst = ByteBuffer.allocate(payload.length); + int drained = pipe.pollDrain(dst); + assertThat(drained).isEqualTo(payload.length); + assertThat(pipe.pollDrain(ByteBuffer.allocate(8))).isEqualTo(-1); + } + + @Test + void recycle_intoEofPipe_doesNotThrowAndDoesNotCorruptPool() throws Exception { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + Chunk c = pipe.acquireForFill(); + c.len(4); + pipe.publish(c); + pipe.signalEof(); + + ByteBuffer dst = ByteBuffer.allocate(8); + int drained = pipe.pollDrain(dst); + int afterDrain = pipe.pollDrain(ByteBuffer.allocate(8)); + + assertThat(drained).isEqualTo(4); + assertThat(afterDrain).isEqualTo(-1); + assertThat(pipe.allocatedForTest()).isEqualTo(1); + } + + @Test + void recycle_intoAbortedPipe_doesNotThrow() throws Exception { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + Chunk c = pipe.acquireForFill(); + pipe.abort(); + + c.len(0); + pipe.publish(c); + + assertThat(pipe.state()).isEqualTo(BodyChunkPipe.State.ABORTED); + } + + @Test + void recycle_intoErrorPipe_doesNotThrow() throws Exception { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + Chunk c = pipe.acquireForFill(); + pipe.signalError(new RuntimeException("boom")); + + c.len(0); + pipe.publish(c); + + assertThat(pipe.state()).isEqualTo(BodyChunkPipe.State.ERROR); + } + + @Test + void constructor_doesNotAllocateChunks() { + BodyChunkPipe pipe = new BodyChunkPipe(4, 16); + + assertThat(pipe.allocatedForTest()).isZero(); + } + + @Test + void acquireForFill_firstCall_allocatesOneChunk() throws Exception { + BodyChunkPipe pipe = new BodyChunkPipe(4, 16); + + Chunk c = pipe.acquireForFill(); + + assertThat(c).isNotNull(); + assertThat(c.data()).hasSize(16); + assertThat(pipe.allocatedForTest()).isEqualTo(1); + } + + @Test + void acquireForFill_uniqueChunksUpToDepth_thenStopsAllocating() throws Exception { + BodyChunkPipe pipe = new BodyChunkPipe(3, 8); + Chunk c1 = pipe.acquireForFill(); + Chunk c2 = pipe.acquireForFill(); + Chunk c3 = pipe.acquireForFill(); + + c1.len(1); + pipe.publish(c1); + pipe.pollDrain(ByteBuffer.allocate(8)); + Chunk reused = pipe.acquireForFill(); + + assertThat(c1).isNotSameAs(c2).isNotSameAs(c3); + assertThat(c2).isNotSameAs(c3); + assertThat(pipe.allocatedForTest()).isEqualTo(3); + assertThat(reused).isSameAs(c1); + } + + @Test + void acquireForFill_recycledChunkReused_noNewAllocation() throws Exception { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + Chunk c = pipe.acquireForFill(); + c.len(3); + pipe.publish(c); + pipe.pollDrain(ByteBuffer.allocate(8)); + + Chunk reused = pipe.acquireForFill(); + + assertThat(reused).isSameAs(c); + assertThat(pipe.allocatedForTest()).isEqualTo(1); + } + + @Test + void acquireForFill_afterAbort_returnsNull() throws Exception { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + pipe.abort(); + + Chunk c = pipe.acquireForFill(); + + assertThat(c).isNull(); + } + + @Test + void acquireForFill_afterSignalError_returnsNull() throws Exception { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + pipe.signalError(new RuntimeException("boom")); + + Chunk c = pipe.acquireForFill(); + + assertThat(c).isNull(); + } + + @Test + void constructor_invalidDepth_throws() { + assertThatThrownBy(() -> new BodyChunkPipe(0, 8)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("depth"); + } + + @Test + void constructor_invalidChunkSize_throws() { + assertThatThrownBy(() -> new BodyChunkPipe(2, 0)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("chunkSize"); + } + + /** + * Multi-threaded ordering test: producer races to call {@link BodyChunkPipe#signalError(Throwable)} + * while a consumer is concurrently spinning on {@link BodyChunkPipe#pollDrain(java.nio.ByteBuffer)}. + * The contract is that whenever the consumer observes the ERROR state, the cause must already + * be visible (no {@code RuntimeException("Producer failed", null)}). RepeatedTest amplifies the + * race window. With the cause published before the CAS, this should pass on every iteration. + */ + @RepeatedTest(50) + void signalError_concurrentPollDrain_consumerNeverSeesNullCause() throws Exception { + BodyChunkPipe pipe = new BodyChunkPipe(2, 16); + IllegalStateException expected = new IllegalStateException("boom"); + CountDownLatch start = new CountDownLatch(1); + AtomicReference consumerError = new AtomicReference<>(); + AtomicReference nullCauseSighting = new AtomicReference<>(); + + Thread consumer = new Thread(() -> { + try { + start.await(); + ByteBuffer dst = ByteBuffer.allocate(16); + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); + while (System.nanoTime() < deadline) { + try { + int n = pipe.pollDrain(dst); + if (n < 0) { + return; + } + dst.clear(); + } catch (RuntimeException re) { + if (re.getCause() == null) { + nullCauseSighting.set(re); + } + return; + } + } + } catch (Throwable t) { + consumerError.set(t); + } + }, "pipe-consumer"); + + Thread producer = new Thread(() -> { + try { + start.await(); + pipe.signalError(expected); + } catch (Throwable t) { + consumerError.set(t); + } + }, "pipe-producer"); + + consumer.start(); + producer.start(); + start.countDown(); + producer.join(5_000); + consumer.join(5_000); + + assertThat(consumer.isAlive()).isFalse(); + assertThat(producer.isAlive()).isFalse(); + assertThat(consumerError.get()).isNull(); + assertThat(nullCauseSighting.get()).isNull(); + } + + /** + * Multi-threaded test for the recycle/notify path: producer is forced to block on + * {@link BodyChunkPipe#acquireForFill()} because all chunks are in flight, then the consumer + * drains a chunk which {@code recycle()}s and notifies the producer to wake. This exercises the + * full {@code freeLock.notifyAll()} hand-off rather than relying on the defensive 50ms wakeup. + */ + @Test + void acquireForFill_blocksUntilConsumerRecycles_thenWakesAndCompletes() throws Exception { + BodyChunkPipe pipe = new BodyChunkPipe(1, 8); + Chunk first = pipe.acquireForFill(); + first.len(4); + pipe.publish(first); + + CountDownLatch producerEntered = new CountDownLatch(1); + AtomicReference reused = new AtomicReference<>(); + AtomicReference producerError = new AtomicReference<>(); + Thread producer = new Thread(() -> { + try { + producerEntered.countDown(); + Chunk c = pipe.acquireForFill(); + reused.set(c); + } catch (Throwable t) { + producerError.set(t); + } + }, "pipe-producer"); + + producer.start(); + producerEntered.await(); + // Drain so the chunk is recycled and the producer is woken via notifyAll. + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); + while (reused.get() == null && System.nanoTime() < deadline) { + pipe.pollDrain(ByteBuffer.allocate(8)); + producer.join(50); + } + + assertThat(producerError.get()).isNull(); + assertThat(reused.get()).isSameAs(first); + assertThat(pipe.allocatedForTest()).isEqualTo(1); + } +} diff --git a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/PipeBackedRequestBodyStreamTest.java b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/PipeBackedRequestBodyStreamTest.java new file mode 100644 index 000000000000..1add84bb4cdf --- /dev/null +++ b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/PipeBackedRequestBodyStreamTest.java @@ -0,0 +1,141 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.awssdk.http.crt.internal.request; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.nio.ByteBuffer; +import org.junit.jupiter.api.Test; + +class PipeBackedRequestBodyStreamTest { + + @Test + void sendRequestBody_emptyOpenPipe_returnsFalseAndCopiesNothing() { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + PipeBackedRequestBodyStream stream = new PipeBackedRequestBodyStream(pipe); + ByteBuffer dst = ByteBuffer.allocate(8); + + boolean done = stream.sendRequestBody(dst); + + assertThat(done).isFalse(); + assertThat(dst.position()).isZero(); + } + + @Test + void sendRequestBody_afterEofAndDrained_returnsTrue() throws Exception { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + Chunk c = pipe.acquireForFill(); + byte[] payload = {1, 2, 3}; + System.arraycopy(payload, 0, c.data(), 0, payload.length); + c.len(payload.length); + pipe.publish(c); + pipe.signalEof(); + PipeBackedRequestBodyStream stream = new PipeBackedRequestBodyStream(pipe); + + ByteBuffer first = ByteBuffer.allocate(8); + boolean firstDone = stream.sendRequestBody(first); + ByteBuffer second = ByteBuffer.allocate(8); + boolean secondDone = stream.sendRequestBody(second); + + assertThat(firstDone).isFalse(); + assertThat(first.position()).isEqualTo(payload.length); + assertThat(secondDone).isTrue(); + assertThat(second.position()).isZero(); + } + + @Test + void sendRequestBody_pipeInError_throwsRuntimeExceptionWithCause() { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + IllegalStateException cause = new IllegalStateException("upstream broke"); + pipe.signalError(cause); + PipeBackedRequestBodyStream stream = new PipeBackedRequestBodyStream(pipe); + + assertThatThrownBy(() -> stream.sendRequestBody(ByteBuffer.allocate(8))) + .isInstanceOf(RuntimeException.class) + .hasMessageContaining("Producer failed") + .hasRootCauseMessage("upstream broke"); + } + + @Test + void sendRequestBody_pipeAborted_throwsRuntimeException() { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + pipe.abort(); + PipeBackedRequestBodyStream stream = new PipeBackedRequestBodyStream(pipe); + + assertThatThrownBy(() -> stream.sendRequestBody(ByteBuffer.allocate(8))) + .isInstanceOf(RuntimeException.class) + .hasMessageContaining("aborted"); + } + + @Test + void resetPosition_returnsFalse() { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + PipeBackedRequestBodyStream stream = new PipeBackedRequestBodyStream(pipe); + + assertThat(stream.resetPosition()).isFalse(); + } + + /** + * When CRT's destination buffer is smaller than the chunk size, draining a single chunk + * requires multiple {@code sendRequestBody} calls. This exercises {@link BodyChunkPipe#pollDrain}'s + * {@code pendingDrain} state being carried across consumer invocations. + */ + @Test + void sendRequestBody_destinationSmallerThanChunk_drainsAcrossMultipleCalls() throws Exception { + BodyChunkPipe pipe = new BodyChunkPipe(2, 16); + Chunk c = pipe.acquireForFill(); + byte[] payload = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}; + System.arraycopy(payload, 0, c.data(), 0, payload.length); + c.len(payload.length); + pipe.publish(c); + pipe.signalEof(); + PipeBackedRequestBodyStream stream = new PipeBackedRequestBodyStream(pipe); + + ByteBuffer first = ByteBuffer.allocate(3); + ByteBuffer second = ByteBuffer.allocate(3); + ByteBuffer third = ByteBuffer.allocate(3); + ByteBuffer fourth = ByteBuffer.allocate(3); + ByteBuffer fifth = ByteBuffer.allocate(3); + boolean firstDone = stream.sendRequestBody(first); + boolean secondDone = stream.sendRequestBody(second); + boolean thirdDone = stream.sendRequestBody(third); + boolean fourthDone = stream.sendRequestBody(fourth); + boolean fifthDone = stream.sendRequestBody(fifth); + + assertThat(firstDone).isFalse(); + assertThat(secondDone).isFalse(); + assertThat(thirdDone).isFalse(); + assertThat(fourthDone).isFalse(); + assertThat(fifthDone).isTrue(); + assertThat(first.position()).isEqualTo(3); + assertThat(second.position()).isEqualTo(3); + assertThat(third.position()).isEqualTo(3); + assertThat(fourth.position()).isEqualTo(1); + assertThat(fifth.position()).isZero(); + + byte[] reassembled = new byte[payload.length]; + first.flip(); + first.get(reassembled, 0, 3); + second.flip(); + second.get(reassembled, 3, 3); + third.flip(); + third.get(reassembled, 6, 3); + fourth.flip(); + fourth.get(reassembled, 9, 1); + assertThat(reassembled).containsExactly(payload); + } +} diff --git a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPumpTest.java b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPumpTest.java new file mode 100644 index 000000000000..21d0d4fa1ec6 --- /dev/null +++ b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPumpTest.java @@ -0,0 +1,239 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.awssdk.http.crt.internal.request; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.ByteBuffer; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.locks.LockSupport; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.http.ContentStreamProvider; +import software.amazon.awssdk.http.SdkHttpFullResponse; + +class SyncRequestBodyPumpTest { + + @Test + void pump_happyPath_consumerSeesAllProducerBytes() throws Exception { + byte[] payload = new byte[200]; + for (int i = 0; i < payload.length; i++) { + payload[i] = (byte) (i & 0xFF); + } + BodyChunkPipe pipe = new BodyChunkPipe(2, 32); + SyncRequestBodyPump pump = new SyncRequestBodyPump(ContentStreamProvider.fromByteArray(payload), pipe); + AtomicReference producerError = new AtomicReference<>(); + Thread producer = new Thread(() -> { + try { + pump.pump(); + } catch (Throwable t) { + producerError.set(t); + } + }, "pump-producer"); + + producer.start(); + byte[] consumed = drainAll(pipe, payload.length); + producer.join(5_000); + + assertThat(producerError.get()).isNull(); + assertThat(producer.isAlive()).isFalse(); + assertThat(consumed).containsExactly(payload); + assertThat(pipe.state()).isEqualTo(BodyChunkPipe.State.EOF); + } + + @Test + void pump_emptyStream_signalsEofWithoutPublish() throws Exception { + BodyChunkPipe pipe = new BodyChunkPipe(2, 16); + SyncRequestBodyPump pump = new SyncRequestBodyPump(ContentStreamProvider.fromByteArray(new byte[0]), pipe); + + pump.pump(); + + assertThat(pipe.state()).isEqualTo(BodyChunkPipe.State.EOF); + assertThat(pipe.pollDrain(ByteBuffer.allocate(8))).isEqualTo(-1); + } + + @Test + void pump_inputStreamThrowsIoException_pumpSignalsErrorAndRethrows() { + IOException ioe = new IOException("disk gone"); + BodyChunkPipe pipe = new BodyChunkPipe(2, 16); + ContentStreamProvider provider = () -> new InputStream() { + @Override + public int read() { + throw new UnsupportedOperationException(); + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + throw ioe; + } + }; + SyncRequestBodyPump pump = new SyncRequestBodyPump(provider, pipe); + + assertThatThrownBy(pump::pump).isSameAs(ioe); + assertThat(pipe.state()).isEqualTo(BodyChunkPipe.State.ERROR); + assertThatThrownBy(() -> pipe.pollDrain(ByteBuffer.allocate(8))) + .hasMessageContaining("Producer failed") + .hasRootCauseMessage("disk gone"); + } + + @Test + void pump_abortedWhilePumping_returnsWithoutSignalingEof() throws Exception { + // pipe depth 1 + payload larger than chunk forces producer to block on second acquireForFill, + // giving the test thread a deterministic point to call abort(). + BodyChunkPipe pipe = new BodyChunkPipe(1, 8); + byte[] payload = new byte[64]; + SyncRequestBodyPump pump = new SyncRequestBodyPump(ContentStreamProvider.fromByteArray(payload), pipe); + AtomicReference producerError = new AtomicReference<>(); + Thread producer = new Thread(() -> { + try { + pump.pump(); + } catch (Throwable t) { + producerError.set(t); + } + }, "pump-producer"); + + producer.start(); + waitUntilStateIsOpenWithChunkInFlight(pipe); + pump.abort(); + producer.join(5_000); + + assertThat(producer.isAlive()).isFalse(); + assertThat(producerError.get()).isNull(); + assertThat(pipe.state()).isEqualTo(BodyChunkPipe.State.ABORTED); + } + + /** + * Regression test for the producer-livelock-on-CRT-failure path. + * + *

If CRT signals request failure (network error, idle/health timeout, etc.) while the + * producer is parked in {@link BodyChunkPipe#acquireForFill()}, nothing in the pipe's normal + * contract wakes it without a recycle/abort. The fix in {@code AwsCrtHttpClient.CrtHttpRequest.call()} + * registers a {@code responseFuture.whenComplete(...)} hook that calls {@code pump.abort()} + * when the response future completes exceptionally. This test reproduces that wiring + * at the unit level: a pump runs against a pipe with no consumer, the producer parks once the + * pipe is full, and we then complete a separate response future exceptionally with the same + * hook to verify the producer unblocks and {@code pump()} returns. + * + *

Without the hook (or equivalent abort path), {@code producer.join(5_000)} would time out + * and the test would fail. + */ + @Test + void pump_responseFutureFailsExceptionally_whileProducerParked_unblocksProducerViaAbortHook() throws Exception { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + // Payload larger than depth*chunkSize forces the producer to park on acquireForFill once + // both chunks are sitting in the ready queue with no consumer draining. + byte[] payload = new byte[128]; + SyncRequestBodyPump pump = new SyncRequestBodyPump(ContentStreamProvider.fromByteArray(payload), pipe); + CompletableFuture responseFuture = new CompletableFuture<>(); + responseFuture.whenComplete((r, t) -> { + if (t != null) { + pump.abort(); + } + }); + + AtomicReference producerError = new AtomicReference<>(); + Thread producer = new Thread(() -> { + try { + pump.pump(); + } catch (Throwable t) { + producerError.set(t); + } + }, "pump-producer"); + + producer.start(); + waitUntilProducerIsParked(pipe); + responseFuture.completeExceptionally(new IOException("simulated CRT failure")); + producer.join(5_000); + + assertThat(producer.isAlive()).isFalse(); + assertThat(producerError.get()).isNull(); + assertThat(pipe.state()).isEqualTo(BodyChunkPipe.State.ABORTED); + } + + @Test + void abort_propagatesToPipe() { + BodyChunkPipe pipe = new BodyChunkPipe(2, 8); + SyncRequestBodyPump pump = new SyncRequestBodyPump( + ContentStreamProvider.fromByteArray(new byte[0]), pipe); + + pump.abort(); + + assertThat(pipe.state()).isEqualTo(BodyChunkPipe.State.ABORTED); + } + + private static byte[] drainAll(BodyChunkPipe pipe, int expected) { + byte[] out = new byte[expected]; + int written = 0; + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); + while (written < expected && System.nanoTime() < deadline) { + ByteBuffer scratch = ByteBuffer.allocate(Math.min(64, expected - written)); + int n = pipe.pollDrain(scratch); + if (n < 0) { + break; + } + if (n == 0) { + LockSupport.parkNanos(TimeUnit.MILLISECONDS.toNanos(1)); + continue; + } + scratch.flip(); + scratch.get(out, written, n); + written += n; + } + if (written < expected) { + throw new AssertionError("Drained only " + written + " of " + expected + " bytes"); + } + return out; + } + + private static void waitUntilStateIsOpenWithChunkInFlight(BodyChunkPipe pipe) throws InterruptedException { + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); + while (System.nanoTime() < deadline) { + if (pipe.allocatedForTest() >= 1) { + return; + } + Thread.sleep(1); + } + throw new AssertionError("Producer did not allocate a chunk within timeout"); + } + + /** + * Wait for the producer to park on {@code acquireForFill}. Detected by the pipe reaching its + * configured depth in allocations and then staying there for a couple of consecutive observations + * (the producer can't make further progress without a recycle). + */ + private static void waitUntilProducerIsParked(BodyChunkPipe pipe) throws InterruptedException { + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); + int stableObservations = 0; + int lastAllocated = -1; + while (System.nanoTime() < deadline) { + int allocated = pipe.allocatedForTest(); + if (allocated == lastAllocated && allocated > 0) { + if (++stableObservations >= 3) { + return; + } + } else { + stableObservations = 0; + lastAllocated = allocated; + } + Thread.sleep(20); + } + throw new AssertionError("Producer did not park within timeout"); + } +} diff --git a/services/s3/src/it/java/software/amazon/awssdk/services/s3/crthttpclient/S3WithCrtHttpClientsIntegrationTests.java b/services/s3/src/it/java/software/amazon/awssdk/services/s3/crthttpclient/S3WithCrtHttpClientsIntegrationTests.java index f1eb1c6e4f23..943635dd41f0 100644 --- a/services/s3/src/it/java/software/amazon/awssdk/services/s3/crthttpclient/S3WithCrtHttpClientsIntegrationTests.java +++ b/services/s3/src/it/java/software/amazon/awssdk/services/s3/crthttpclient/S3WithCrtHttpClientsIntegrationTests.java @@ -16,12 +16,13 @@ package software.amazon.awssdk.services.s3.crthttpclient; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; import static software.amazon.awssdk.testutils.service.S3BucketUtils.temporaryBucketName; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import org.assertj.core.api.Assertions; +import java.time.Duration; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -32,6 +33,7 @@ import software.amazon.awssdk.services.s3.S3Client; import software.amazon.awssdk.services.s3.S3IntegrationTestBase; import software.amazon.awssdk.services.s3.model.GetObjectResponse; +import software.amazon.awssdk.services.s3.model.PutObjectResponse; import software.amazon.awssdk.services.s3.utils.ChecksumUtils; import software.amazon.awssdk.testutils.RandomTempFile; import software.amazon.awssdk.utils.Md5Utils; @@ -80,4 +82,22 @@ void getObject_toFile_objectSentCorrectly() throws Exception { assertThat(Md5Utils.md5AsBase64(destination.toFile())).isEqualTo(Md5Utils.md5AsBase64(testFile)); } + + @Test + void getObject_responseStreamPipedIntoPutObject_completesWithoutDeadlock() throws Exception { + String destinationKey = "piped-" + TEST_KEY; + try (ResponseInputStream sourceStream = + s3WithCrtHttpClient.getObject(r -> r.bucket(TEST_BUCKET).key(TEST_KEY), + ResponseTransformer.toInputStream())) { + long contentLength = sourceStream.response().contentLength(); + + PutObjectResponse putResponse = assertTimeoutPreemptively( + Duration.ofSeconds(120), + () -> s3WithCrtHttpClient.putObject( + r -> r.bucket(TEST_BUCKET).key(destinationKey).contentLength(contentLength), + RequestBody.fromInputStream(sourceStream, contentLength))); + + assertThat(putResponse.eTag()).isNotBlank(); + } + } } From 3fe06e311ac06b10b9f6ec8c61462babe69165fb Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Wed, 10 Jun 2026 11:50:09 -0700 Subject: [PATCH 02/12] Add changelog entry for sync CRT pull-pump fix --- .../next-release/bugfix-AWSCRTbasedHTTPClient-d1be626.json | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/next-release/bugfix-AWSCRTbasedHTTPClient-d1be626.json diff --git a/.changes/next-release/bugfix-AWSCRTbasedHTTPClient-d1be626.json b/.changes/next-release/bugfix-AWSCRTbasedHTTPClient-d1be626.json new file mode 100644 index 000000000000..bf864237642e --- /dev/null +++ b/.changes/next-release/bugfix-AWSCRTbasedHTTPClient-d1be626.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "AWS CRT-based HTTP Client", + "contributor": "", + "description": "Fixed an issue where AwsCrtHttpClient (sync) could deadlock when a request body was sourced from an InputStream that depends on the same CRT event loop, for example when piping a GetObject ResponseInputStream into a PutObject body. The InputStream read now happens on the caller thread instead of the CRT event-loop thread." +} From ba13c6884ed15a01f6a8bbe69eb6041bfd21d892 Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Wed, 10 Jun 2026 14:07:45 -0700 Subject: [PATCH 03/12] Add timeout and diagnostic logging to waitForStreamAcquired --- .../amazon/awssdk/spotbugs-suppressions.xml | 4 ++ .../awssdk/http/crt/AwsCrtHttpClient.java | 59 +++++++++++++++---- .../awssdk/http/crt/AwsCrtHttpClientBase.java | 2 +- .../http/crt/internal/CrtRequestContext.java | 12 ++++ .../http/crt/internal/CrtRequestExecutor.java | 13 ++++ .../crt/internal/request/BodyChunkPipe.java | 12 +++- .../internal/request/SyncRequestBodyPump.java | 9 ++- 7 files changed, 95 insertions(+), 16 deletions(-) diff --git a/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml b/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml index 0f6ab0d22c64..446d747b4353 100644 --- a/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml +++ b/build-tools/src/main/resources/software/amazon/awssdk/spotbugs-suppressions.xml @@ -353,6 +353,10 @@ caller (sync) thread, never on the CRT event loop. --> + + + diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java index 709cc9121528..dbf58ef21f58 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java @@ -21,6 +21,9 @@ import java.time.Duration; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.function.Consumer; import software.amazon.awssdk.annotations.SdkPublicApi; import software.amazon.awssdk.crt.http.HttpException; @@ -39,6 +42,7 @@ import software.amazon.awssdk.http.crt.internal.request.SyncRequestBodyPump; import software.amazon.awssdk.utils.AttributeMap; import software.amazon.awssdk.utils.CompletableFutureUtils; +import software.amazon.awssdk.utils.Logger; /** * An implementation of {@link SdkHttpClient} that uses the AWS Common Runtime (CRT) Http Client to communicate with @@ -104,11 +108,14 @@ public ExecutableHttpRequest prepareRequest(HttpExecuteRequest request) { .streamManager(streamManager) .readBufferSize(this.readBufferSize) .request(request) + .connectionAcquisitionTimeoutMillis(this.connectionAcquisitionTimeout) .build(); return new CrtHttpRequest(context); } private static final class CrtHttpRequest implements ExecutableHttpRequest { + private static final Logger LOG = Logger.loggerFor(CrtHttpRequest.class); + private final CrtRequestContext context; private volatile CompletableFuture responseFuture; private volatile SyncRequestBodyPump pump; @@ -120,56 +127,68 @@ private CrtHttpRequest(CrtRequestContext context) { @Override public HttpExecuteResponse call() throws IOException { HttpExecuteResponse.Builder builder = HttpExecuteResponse.builder(); + boolean hasBody = context.sdkRequest().contentStreamProvider().isPresent(); + LOG.info(() -> "call() entered, hasBody=" + hasBody); try { CrtRequestExecutor.Result result = new CrtRequestExecutor().execute(context); responseFuture = result.responseFuture(); pump = result.pump(); + LOG.info(() -> "call() executor.execute() returned, streamFuture pending, pump=" + + (pump != null ? "non-null" : "null")); - // Abort the pump to unblock a parked producer when CRT signals request failure - // via responseFuture. if (pump != null) { SyncRequestBodyPump pumpRef = pump; responseFuture.whenComplete((r, t) -> { if (t != null) { + LOG.info(() -> "responseFuture hook: invoking pump.abort() (cause=" + + t.getClass().getSimpleName() + ")"); pumpRef.abort(); } }); } - boolean streamAcquired = waitForStreamAcquired(result.streamFuture()); + LOG.info(() -> "call() entering waitForStreamAcquired, timeoutMillis=" + + context.connectionAcquisitionTimeoutMillis()); + boolean streamAcquired = waitForStreamAcquired(result.streamFuture(), + context.connectionAcquisitionTimeoutMillis()); + LOG.info(() -> "call() waitForStreamAcquired returned " + streamAcquired); - // No body case (pump == null): CRT writes the request line + headers when the stream - // is acquired and emits no body callbacks. We only need to join responseFuture below. if (pump != null) { if (streamAcquired) { + LOG.info(() -> "call() entering pump.pump()"); try { pump.pump(); + LOG.info(() -> "call() pump.pump() returned"); } catch (IOException ioe) { + LOG.info(() -> "call() pump.pump() threw IOException: " + ioe.getMessage()); responseFuture.completeExceptionally(ioe); throw ioe; } } else { + LOG.info(() -> "call() invoking pump.abort() (post-wait, streamAcquired=false)"); pump.abort(); } } + LOG.info(() -> "call() entering joinInterruptibly(responseFuture)"); SdkHttpFullResponse response = CompletableFutureUtils.joinInterruptibly(responseFuture); + LOG.info(() -> "call() responseFuture joined: success"); builder.response(response); builder.responseBody(response.content().orElse(null)); + LOG.info(() -> "call() exiting normally"); return builder.build(); } catch (CompletionException e) { Throwable cause = e.getCause(); + LOG.info(() -> "call() catch CompletionException, cause=" + + (cause == null ? "" : cause.getClass().getName() + ": " + cause.getMessage())); - // Complete the future exceptionally to trigger connection cleanup in the response handler. - // Handles the thread-interrupt case where joinInterruptibly throws due to - // InterruptedException, ensuring closeConnection() is invoked to prevent leaking the - // connection from the pool. if (responseFuture != null) { responseFuture.completeExceptionally(cause != null ? cause : e); } if (pump != null) { + LOG.info(() -> "call() catch invoking pump.abort()"); pump.abort(); } @@ -191,22 +210,38 @@ public HttpExecuteResponse call() throws IOException { @Override public void abort() { + LOG.info(() -> "abort() called externally"); if (responseFuture != null) { responseFuture.completeExceptionally(new IOException("Request was cancelled")); } if (pump != null) { + LOG.info(() -> "abort() invoking pump.abort()"); pump.abort(); } } - private boolean waitForStreamAcquired(CompletableFuture streamFuture) { + private boolean waitForStreamAcquired(CompletableFuture streamFuture, long timeoutMillis) { if (streamFuture == null) { + LOG.info(() -> "waitForStreamAcquired: streamFuture==null, returning false"); return false; } + LOG.info(() -> "waitForStreamAcquired: starting, timeout=" + timeoutMillis + "ms"); try { - CompletableFutureUtils.joinInterruptibly(streamFuture); + streamFuture.get(timeoutMillis, TimeUnit.MILLISECONDS); + LOG.info(() -> "waitForStreamAcquired: streamFuture completed normally"); return true; - } catch (CompletionException e) { + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + LOG.info(() -> "waitForStreamAcquired: interrupted"); + return false; + } catch (TimeoutException e) { + LOG.warn(() -> "waitForStreamAcquired: timed out after " + timeoutMillis + + "ms - streamFuture still pending"); + return false; + } catch (ExecutionException e) { + Throwable cause = e.getCause(); + LOG.info(() -> "waitForStreamAcquired: streamFuture completed exceptionally: " + + (cause == null ? e.getMessage() : cause.getClass().getName() + ": " + cause.getMessage())); return false; } } diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClientBase.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClientBase.java index 50689d2236d5..afe19159e3c2 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClientBase.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClientBase.java @@ -61,6 +61,7 @@ abstract class AwsCrtHttpClientBase implements SdkAutoCloseable { protected final long readBufferSize; protected final Protocol protocol; + protected final long connectionAcquisitionTimeout; private final Map connectionPools = new ConcurrentHashMap<>(); private final LinkedList ownedSubResources = new LinkedList<>(); private final ClientBootstrap bootstrap; @@ -70,7 +71,6 @@ abstract class AwsCrtHttpClientBase implements SdkAutoCloseable { private final HttpMonitoringOptions monitoringOptions; private final long maxConnectionIdleInMilliseconds; private final int maxStreamsPerEndpoint; - private final long connectionAcquisitionTimeout; private final TlsContextOptions tlsContextOptions; private boolean isClosed = false; diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestContext.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestContext.java index ba97cc3466e8..76771f859d0c 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestContext.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestContext.java @@ -26,12 +26,14 @@ public final class CrtRequestContext { private final long readBufferSize; private final HttpStreamManager streamManager; private final MetricCollector metricCollector; + private final long connectionAcquisitionTimeoutMillis; private CrtRequestContext(Builder builder) { this.request = builder.request; this.readBufferSize = builder.readBufferSize; this.streamManager = builder.streamManager; this.metricCollector = request.metricCollector().orElse(null); + this.connectionAcquisitionTimeoutMillis = builder.connectionAcquisitionTimeoutMillis; } public static Builder builder() { @@ -54,10 +56,15 @@ public MetricCollector metricCollector() { return metricCollector; } + public long connectionAcquisitionTimeoutMillis() { + return connectionAcquisitionTimeoutMillis; + } + public static final class Builder { private HttpExecuteRequest request; private long readBufferSize; private HttpStreamManager streamManager; + private long connectionAcquisitionTimeoutMillis; private Builder() { } @@ -77,6 +84,11 @@ public Builder streamManager(HttpStreamManager streamManager) { return this; } + public Builder connectionAcquisitionTimeoutMillis(long connectionAcquisitionTimeoutMillis) { + this.connectionAcquisitionTimeoutMillis = connectionAcquisitionTimeoutMillis; + return this; + } + public CrtRequestContext build() { return new CrtRequestContext(this); } diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java index 03fccd4ccd7a..5838a0216b06 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java @@ -28,9 +28,11 @@ import software.amazon.awssdk.http.crt.internal.response.InputStreamAdaptingHttpStreamResponseHandler; import software.amazon.awssdk.metrics.MetricCollector; import software.amazon.awssdk.metrics.NoOpMetricCollector; +import software.amazon.awssdk.utils.Logger; @SdkInternalApi public final class CrtRequestExecutor { + private static final Logger LOG = Logger.loggerFor(CrtRequestExecutor.class); public Result execute(CrtRequestContext executionContext) { CompletableFuture requestFuture = new CompletableFuture<>(); @@ -45,29 +47,40 @@ public Result execute(CrtRequestContext executionContext) { InputStreamAdaptingHttpStreamResponseHandler crtResponseHandler = new InputStreamAdaptingHttpStreamResponseHandler(requestFuture); SyncCrtRequest syncCrtRequest = CrtRequestAdapter.toCrtRequest(executionContext); + LOG.info(() -> "execute() acquireStream invoked"); CompletableFuture streamFuture = executionContext.streamManager().acquireStream(syncCrtRequest.httpRequest(), crtResponseHandler); // Evict the connection from the pool on failure so it is not reused. requestFuture.whenComplete((r, t) -> { if (t != null) { + LOG.info(() -> "execute() requestFuture exceptional: closeConnection() (cause=" + + t.getClass().getSimpleName() + ")"); crtResponseHandler.closeConnection(); } }); long finalAcquireStartTime = acquireStartTime; streamFuture.whenComplete((streamBase, throwable) -> { + if (throwable == null) { + LOG.info(() -> "execute() streamFuture.whenComplete fired with success"); + } else { + LOG.info(() -> "execute() streamFuture.whenComplete fired with throwable=" + throwable.getClass().getName() + + ": " + throwable.getMessage()); + } crtResponseHandler.onAcquireStream(streamBase); if (shouldPublishMetrics) { reportMetrics(executionContext.streamManager(), metricCollector, finalAcquireStartTime); } if (throwable != null) { + LOG.info(() -> "execute() completing requestFuture exceptionally from streamFuture failure"); requestFuture.completeExceptionally(wrapCrtException(throwable)); } }); return new Result(requestFuture, syncCrtRequest.pump(), streamFuture); } catch (Throwable t) { + LOG.info(() -> "execute() outer catch, completing requestFuture exceptionally: " + t.getClass().getName()); requestFuture.completeExceptionally(t); return new Result(requestFuture, null, null); } diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java index fdaed9a7bc6a..7aa6d31b2d6a 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java @@ -22,6 +22,7 @@ import java.util.concurrent.atomic.AtomicReference; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.annotations.SdkTestInternalApi; +import software.amazon.awssdk.utils.Logger; /** * Bounded producer/consumer hand-off between the caller thread (producer) and the CRT event-loop thread (consumer). @@ -42,6 +43,7 @@ */ @SdkInternalApi final class BodyChunkPipe { + private static final Logger LOG = Logger.loggerFor(BodyChunkPipe.class); enum State { OPEN, @@ -98,6 +100,7 @@ Chunk acquireForFill() throws InterruptedException { while (true) { State s = state.get(); if (s == State.ABORTED || s == State.ERROR) { + LOG.debug(() -> "acquireForFill returning null, state=" + s); return null; } Chunk c = free.pollFirst(); @@ -135,7 +138,9 @@ void publish(Chunk chunk) throws InterruptedException { * Producer side: signal end-of-stream. Idempotent. */ void signalEof() { - state.compareAndSet(State.OPEN, State.EOF); + if (state.compareAndSet(State.OPEN, State.EOF)) { + LOG.debug(() -> "state OPEN -> EOF"); + } } /** @@ -147,7 +152,9 @@ void signalError(Throwable t) { // never observes state==ERROR with error==null. The volatile write to `error` is // harmless if the CAS later loses (idempotent signal). error = t; - state.compareAndSet(State.OPEN, State.ERROR); + if (state.compareAndSet(State.OPEN, State.ERROR)) { + LOG.debug(() -> "state OPEN -> ERROR (" + t.getClass().getSimpleName() + ")"); + } freeLock.notifyAll(); } } @@ -158,6 +165,7 @@ void signalError(Throwable t) { void abort() { synchronized (freeLock) { if (state.compareAndSet(State.OPEN, State.ABORTED)) { + LOG.debug(() -> "state OPEN -> ABORTED"); ready.clear(); } freeLock.notifyAll(); diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java index dbacf58eedee..015dd0d0ea3c 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java @@ -19,6 +19,7 @@ import java.io.InputStream; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.http.ContentStreamProvider; +import software.amazon.awssdk.utils.Logger; /** * Caller-thread producer that reads from the customer's {@link InputStream} and publishes chunks to a @@ -27,6 +28,7 @@ */ @SdkInternalApi public final class SyncRequestBodyPump { + private static final Logger LOG = Logger.loggerFor(SyncRequestBodyPump.class); private final ContentStreamProvider contentStreamProvider; private final BodyChunkPipe pipe; @@ -41,21 +43,24 @@ public final class SyncRequestBodyPump { * event-loop thread. On EOF signals the pipe normally; on {@link IOException} signals an error and rethrows. */ public void pump() throws IOException { + LOG.info(() -> "pump() entered"); try (InputStream in = contentStreamProvider.newStream()) { while (true) { Chunk chunk = pipe.acquireForFill(); if (chunk == null) { - // pipe was aborted while we were waiting; stop without signaling EOF. + LOG.info(() -> "pump() exiting due to abort (acquireForFill returned null)"); return; } int read; try { read = in.read(chunk.data(), 0, chunk.data().length); } catch (IOException ioe) { + LOG.info(() -> "pump() exiting due to error: " + ioe.getMessage()); pipe.signalError(ioe); throw ioe; } if (read < 0) { + LOG.info(() -> "pump() exiting due to eof"); pipe.signalEof(); return; } @@ -65,6 +70,7 @@ public void pump() throws IOException { } } catch (InterruptedException ie) { Thread.currentThread().interrupt(); + LOG.info(() -> "pump() exiting due to interrupt"); pipe.abort(); throw new IOException("Interrupted while writing request body", ie); } @@ -74,6 +80,7 @@ public void pump() throws IOException { * Abort the underlying pipe (e.g., when the caller's {@code call()} is cancelled). */ public void abort() { + LOG.info(() -> "pump.abort() called"); pipe.abort(); } } From f2172b43c8804bd7b3396f4c7a2c1bbd7057dc60 Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Thu, 11 Jun 2026 10:06:32 -0700 Subject: [PATCH 04/12] Dump threads on long-running-request test timeout (diagnostic) When assertFailsWithinTimeBound times out waiting for the request future, print a full thread dump (via ThreadMXBean.dumpAllThreads with locked monitors and synchronizers) to stderr before throwing the AssertionError. Surefire captures the forked-JVM stderr to surefire-reports, so the dump survives Catapult's report-export step. Search the per-class -output.txt for the marker "=== THREAD DUMP ===". Adds checkstyle-suppressions entries for LongRunningRequestTestSupport so the java.lang.management imports and System.err usage do not trip NonJavaBaseModuleCheck and the System-console Regexp rule. --- .../amazon/awssdk/checkstyle-suppressions.xml | 7 +++++++ .../http/LongRunningRequestTestSupport.java | 16 ++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/build-tools/src/main/resources/software/amazon/awssdk/checkstyle-suppressions.xml b/build-tools/src/main/resources/software/amazon/awssdk/checkstyle-suppressions.xml index 4e81373b0be3..d09f3d004884 100644 --- a/build-tools/src/main/resources/software/amazon/awssdk/checkstyle-suppressions.xml +++ b/build-tools/src/main/resources/software/amazon/awssdk/checkstyle-suppressions.xml @@ -72,4 +72,11 @@ + + + + diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/LongRunningRequestTestSupport.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/LongRunningRequestTestSupport.java index 723869ad8525..e8ffd490c5c7 100644 --- a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/LongRunningRequestTestSupport.java +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/LongRunningRequestTestSupport.java @@ -20,6 +20,9 @@ import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo; import com.github.tomakehurst.wiremock.junit5.WireMockExtension; +import java.lang.management.ManagementFactory; +import java.lang.management.ThreadInfo; +import java.lang.management.ThreadMXBean; import java.time.Duration; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; @@ -79,6 +82,8 @@ public static void assertFailsWithinTimeBound(CompletableFuture future, Durat future.get(maxWait.toMillis(), TimeUnit.MILLISECONDS); throw new AssertionError("Expected request to throw an exception but it completed successfully"); } catch (TimeoutException e) { + // Dump threads BEFORE cancelling the future so the snapshot reflects the hang. + System.err.println(dumpAllThreads()); future.cancel(true); throw new AssertionError( "Expected request to fail within " + maxWait + " but it was still running - client appears to hang", @@ -90,4 +95,15 @@ public static void assertFailsWithinTimeBound(CompletableFuture future, Durat // expected } } + + static String dumpAllThreads() { + ThreadMXBean tmx = ManagementFactory.getThreadMXBean(); + ThreadInfo[] infos = tmx.dumpAllThreads(true, true); + StringBuilder sb = new StringBuilder("=== THREAD DUMP ===\n"); + for (ThreadInfo info : infos) { + sb.append(info.toString()).append('\n'); + } + sb.append("=== END THREAD DUMP ===\n"); + return sb.toString(); + } } From 31bb55bdc27c3eacb54f08bfc964683f08503b93 Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Thu, 11 Jun 2026 11:45:07 -0700 Subject: [PATCH 05/12] Use logger instead of System.err for thread dump Routes the hang-time thread dump through Logger.error so it lands in the SDK's configured appender alongside the lifecycle logs, instead of racing with CRT native stderr writes (the surefire "Corrupted channel by directly writing to native stream" warning). Drops the now-unneeded Regexp suppression for LongRunningRequestTestSupport; keeps the NonJavaBaseModuleCheck suppression because java.lang.management is still used by dumpAllThreads(). --- .../software/amazon/awssdk/checkstyle-suppressions.xml | 6 ++---- .../amazon/awssdk/http/LongRunningRequestTestSupport.java | 5 ++++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/build-tools/src/main/resources/software/amazon/awssdk/checkstyle-suppressions.xml b/build-tools/src/main/resources/software/amazon/awssdk/checkstyle-suppressions.xml index d09f3d004884..8988a3bcc74a 100644 --- a/build-tools/src/main/resources/software/amazon/awssdk/checkstyle-suppressions.xml +++ b/build-tools/src/main/resources/software/amazon/awssdk/checkstyle-suppressions.xml @@ -73,10 +73,8 @@ - + - diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/LongRunningRequestTestSupport.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/LongRunningRequestTestSupport.java index e8ffd490c5c7..098ad470b962 100644 --- a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/LongRunningRequestTestSupport.java +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/LongRunningRequestTestSupport.java @@ -28,6 +28,7 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import software.amazon.awssdk.utils.Logger; /** * Shared helpers for the long-running request test suites. @@ -39,6 +40,8 @@ public final class LongRunningRequestTestSupport { public static final Duration TIME_BOUND_SAFETY_MARGIN = Duration.ofSeconds(10); public static final Duration HANG_DELAY = Duration.ofMinutes(1); + private static final Logger log = Logger.loggerFor(LongRunningRequestTestSupport.class); + private LongRunningRequestTestSupport() { } @@ -83,7 +86,7 @@ public static void assertFailsWithinTimeBound(CompletableFuture future, Durat throw new AssertionError("Expected request to throw an exception but it completed successfully"); } catch (TimeoutException e) { // Dump threads BEFORE cancelling the future so the snapshot reflects the hang. - System.err.println(dumpAllThreads()); + log.error(() -> dumpAllThreads()); future.cancel(true); throw new AssertionError( "Expected request to fail within " + maxWait + " but it was still running - client appears to hang", From 06c4063baf2937fa4fed556ea94f5f3925dafeaf Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Thu, 11 Jun 2026 12:42:57 -0700 Subject: [PATCH 06/12] Address PR review comments from alextwoods - Add Chunk.reset() and use it in BodyChunkPipe.recycle so the position+length reset is a single named call. - Swap the order in SyncRequestBodyPump's InterruptedException handler so cleanup (pipe.abort()) runs before re-asserting the thread interrupt, matching the conventional "do work, then re-interrupt last" idiom. --- .../awssdk/http/crt/internal/request/BodyChunkPipe.java | 3 +-- .../amazon/awssdk/http/crt/internal/request/Chunk.java | 5 +++++ .../http/crt/internal/request/SyncRequestBodyPump.java | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java index 7aa6d31b2d6a..c6848f7872c9 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java @@ -239,8 +239,7 @@ int allocatedForTest() { } private void recycle(Chunk c) { - c.pos(0); - c.len(0); + c.reset(); synchronized (freeLock) { free.push(c); freeLock.notifyAll(); diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/Chunk.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/Chunk.java index a7e97b292aab..0797b8cbc67a 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/Chunk.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/Chunk.java @@ -55,4 +55,9 @@ int len() { void len(int len) { this.len = len; } + + void reset() { + this.pos = 0; + this.len = 0; + } } diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java index 015dd0d0ea3c..40ebe39902ec 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java @@ -69,9 +69,9 @@ public void pump() throws IOException { pipe.publish(chunk); } } catch (InterruptedException ie) { - Thread.currentThread().interrupt(); LOG.info(() -> "pump() exiting due to interrupt"); pipe.abort(); + Thread.currentThread().interrupt(); throw new IOException("Interrupted while writing request body", ie); } } From 3a60e4c503f9346db2341d7908561fdc756cff88 Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Thu, 11 Jun 2026 17:34:51 -0700 Subject: [PATCH 07/12] Don't call onAcquireStream when streamFuture failed Gates crtResponseHandler.onAcquireStream(stream) on throwable == null in both CrtRequestExecutor and CrtAsyncRequestExecutor. On the failure path, streamBase is null and onAcquireStream(null) is wrong even though it happens to be a no-op today: passing null to a method that expects a real stream is a contract violation. Knowingly diverges from origin/master, which has the same issue. --- .../awssdk/http/crt/internal/CrtAsyncRequestExecutor.java | 4 +++- .../amazon/awssdk/http/crt/internal/CrtRequestExecutor.java | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtAsyncRequestExecutor.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtAsyncRequestExecutor.java index ca9e04fa229a..b3b6c232261c 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtAsyncRequestExecutor.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtAsyncRequestExecutor.java @@ -82,7 +82,9 @@ private void doExecute(CrtAsyncRequestContext executionContext, long finalAcquireStartTime = acquireStartTime; streamFuture.whenComplete((stream, throwable) -> { - crtResponseHandler.onAcquireStream(stream); + if (throwable == null) { + crtResponseHandler.onAcquireStream(stream); + } if (shouldPublishMetrics) { reportMetrics(executionContext.streamManager(), metricCollector, finalAcquireStartTime); } diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java index 5838a0216b06..53ee36bb1725 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java @@ -64,11 +64,11 @@ public Result execute(CrtRequestContext executionContext) { streamFuture.whenComplete((streamBase, throwable) -> { if (throwable == null) { LOG.info(() -> "execute() streamFuture.whenComplete fired with success"); + crtResponseHandler.onAcquireStream(streamBase); } else { LOG.info(() -> "execute() streamFuture.whenComplete fired with throwable=" + throwable.getClass().getName() + ": " + throwable.getMessage()); } - crtResponseHandler.onAcquireStream(streamBase); if (shouldPublishMetrics) { reportMetrics(executionContext.streamManager(), metricCollector, finalAcquireStartTime); } From 54ceeb2364f98a9b69878440fe933e22fa22edba Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Thu, 11 Jun 2026 17:54:05 -0700 Subject: [PATCH 08/12] Add request correlation ID to diagnostic logs (test-controllable) --- .../awssdk/http/crt/AwsCrtHttpClient.java | 54 ++++++----- .../http/crt/internal/CrtRequestContext.java | 12 +++ .../http/crt/internal/CrtRequestExecutor.java | 15 +-- .../crt/internal/request/BodyChunkPipe.java | 14 ++- .../internal/request/CrtRequestAdapter.java | 5 +- .../internal/request/SyncRequestBodyPump.java | 18 ++-- .../http/LongRunningRequestTestSupport.java | 96 ++++++++++++++++++- ...HttpClientLongRunningRequestTestSuite.java | 58 ++--------- 8 files changed, 176 insertions(+), 96 deletions(-) diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java index dbf58ef21f58..1fa66cb940a6 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java @@ -22,6 +22,7 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.ExecutionException; +import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.function.Consumer; @@ -104,11 +105,16 @@ public ExecutableHttpRequest prepareRequest(HttpExecuteRequest request) { * request) */ HttpStreamManager streamManager = getOrCreateConnectionPool(poolKey(request.httpRequest())); + // Tests may override via x-aws-sdk-test-id so surefire output can be grep'd by request. + String reqId = request.httpRequest() + .firstMatchingHeader("x-aws-sdk-test-id") + .orElseGet(() -> String.format("%08x", ThreadLocalRandom.current().nextInt())); CrtRequestContext context = CrtRequestContext.builder() .streamManager(streamManager) .readBufferSize(this.readBufferSize) .request(request) .connectionAcquisitionTimeoutMillis(this.connectionAcquisitionTimeout) + .reqId(reqId) .build(); return new CrtHttpRequest(context); } @@ -117,70 +123,74 @@ private static final class CrtHttpRequest implements ExecutableHttpRequest { private static final Logger LOG = Logger.loggerFor(CrtHttpRequest.class); private final CrtRequestContext context; + private final String reqId; + private final String tag; private volatile CompletableFuture responseFuture; private volatile SyncRequestBodyPump pump; private CrtHttpRequest(CrtRequestContext context) { this.context = context; + this.reqId = context.reqId(); + this.tag = "[reqId=" + reqId + "] "; } @Override public HttpExecuteResponse call() throws IOException { HttpExecuteResponse.Builder builder = HttpExecuteResponse.builder(); boolean hasBody = context.sdkRequest().contentStreamProvider().isPresent(); - LOG.info(() -> "call() entered, hasBody=" + hasBody); + LOG.info(() -> tag + "call() entered, hasBody=" + hasBody); try { CrtRequestExecutor.Result result = new CrtRequestExecutor().execute(context); responseFuture = result.responseFuture(); pump = result.pump(); - LOG.info(() -> "call() executor.execute() returned, streamFuture pending, pump=" + LOG.info(() -> tag + "call() executor.execute() returned, streamFuture pending, pump=" + (pump != null ? "non-null" : "null")); if (pump != null) { SyncRequestBodyPump pumpRef = pump; responseFuture.whenComplete((r, t) -> { if (t != null) { - LOG.info(() -> "responseFuture hook: invoking pump.abort() (cause=" + LOG.info(() -> tag + "responseFuture hook: invoking pump.abort() (cause=" + t.getClass().getSimpleName() + ")"); pumpRef.abort(); } }); } - LOG.info(() -> "call() entering waitForStreamAcquired, timeoutMillis=" + LOG.info(() -> tag + "call() entering waitForStreamAcquired, timeoutMillis=" + context.connectionAcquisitionTimeoutMillis()); boolean streamAcquired = waitForStreamAcquired(result.streamFuture(), context.connectionAcquisitionTimeoutMillis()); - LOG.info(() -> "call() waitForStreamAcquired returned " + streamAcquired); + LOG.info(() -> tag + "call() waitForStreamAcquired returned " + streamAcquired); if (pump != null) { if (streamAcquired) { - LOG.info(() -> "call() entering pump.pump()"); + LOG.info(() -> tag + "call() entering pump.pump()"); try { pump.pump(); - LOG.info(() -> "call() pump.pump() returned"); + LOG.info(() -> tag + "call() pump.pump() returned"); } catch (IOException ioe) { - LOG.info(() -> "call() pump.pump() threw IOException: " + ioe.getMessage()); + LOG.info(() -> tag + "call() pump.pump() threw IOException: " + ioe.getMessage()); responseFuture.completeExceptionally(ioe); throw ioe; } } else { - LOG.info(() -> "call() invoking pump.abort() (post-wait, streamAcquired=false)"); + LOG.info(() -> tag + "call() invoking pump.abort() (post-wait, streamAcquired=false)"); pump.abort(); } } - LOG.info(() -> "call() entering joinInterruptibly(responseFuture)"); + LOG.info(() -> tag + "call() entering joinInterruptibly(responseFuture)"); SdkHttpFullResponse response = CompletableFutureUtils.joinInterruptibly(responseFuture); - LOG.info(() -> "call() responseFuture joined: success"); + LOG.info(() -> tag + "call() responseFuture joined: success"); builder.response(response); builder.responseBody(response.content().orElse(null)); - LOG.info(() -> "call() exiting normally"); + LOG.info(() -> tag + "call() exiting normally"); return builder.build(); } catch (CompletionException e) { Throwable cause = e.getCause(); - LOG.info(() -> "call() catch CompletionException, cause=" + LOG.info(() -> tag + "call() catch CompletionException, cause=" + (cause == null ? "" : cause.getClass().getName() + ": " + cause.getMessage())); if (responseFuture != null) { @@ -188,7 +198,7 @@ public HttpExecuteResponse call() throws IOException { } if (pump != null) { - LOG.info(() -> "call() catch invoking pump.abort()"); + LOG.info(() -> tag + "call() catch invoking pump.abort()"); pump.abort(); } @@ -210,37 +220,37 @@ public HttpExecuteResponse call() throws IOException { @Override public void abort() { - LOG.info(() -> "abort() called externally"); + LOG.info(() -> tag + "abort() called externally"); if (responseFuture != null) { responseFuture.completeExceptionally(new IOException("Request was cancelled")); } if (pump != null) { - LOG.info(() -> "abort() invoking pump.abort()"); + LOG.info(() -> tag + "abort() invoking pump.abort()"); pump.abort(); } } private boolean waitForStreamAcquired(CompletableFuture streamFuture, long timeoutMillis) { if (streamFuture == null) { - LOG.info(() -> "waitForStreamAcquired: streamFuture==null, returning false"); + LOG.info(() -> tag + "waitForStreamAcquired: streamFuture==null, returning false"); return false; } - LOG.info(() -> "waitForStreamAcquired: starting, timeout=" + timeoutMillis + "ms"); + LOG.info(() -> tag + "waitForStreamAcquired: starting, timeout=" + timeoutMillis + "ms"); try { streamFuture.get(timeoutMillis, TimeUnit.MILLISECONDS); - LOG.info(() -> "waitForStreamAcquired: streamFuture completed normally"); + LOG.info(() -> tag + "waitForStreamAcquired: streamFuture completed normally"); return true; } catch (InterruptedException e) { Thread.currentThread().interrupt(); - LOG.info(() -> "waitForStreamAcquired: interrupted"); + LOG.info(() -> tag + "waitForStreamAcquired: interrupted"); return false; } catch (TimeoutException e) { - LOG.warn(() -> "waitForStreamAcquired: timed out after " + timeoutMillis + LOG.warn(() -> tag + "waitForStreamAcquired: timed out after " + timeoutMillis + "ms - streamFuture still pending"); return false; } catch (ExecutionException e) { Throwable cause = e.getCause(); - LOG.info(() -> "waitForStreamAcquired: streamFuture completed exceptionally: " + LOG.info(() -> tag + "waitForStreamAcquired: streamFuture completed exceptionally: " + (cause == null ? e.getMessage() : cause.getClass().getName() + ": " + cause.getMessage())); return false; } diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestContext.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestContext.java index 76771f859d0c..ced8fd555082 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestContext.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestContext.java @@ -27,6 +27,7 @@ public final class CrtRequestContext { private final HttpStreamManager streamManager; private final MetricCollector metricCollector; private final long connectionAcquisitionTimeoutMillis; + private final String reqId; private CrtRequestContext(Builder builder) { this.request = builder.request; @@ -34,6 +35,7 @@ private CrtRequestContext(Builder builder) { this.streamManager = builder.streamManager; this.metricCollector = request.metricCollector().orElse(null); this.connectionAcquisitionTimeoutMillis = builder.connectionAcquisitionTimeoutMillis; + this.reqId = builder.reqId; } public static Builder builder() { @@ -60,11 +62,16 @@ public long connectionAcquisitionTimeoutMillis() { return connectionAcquisitionTimeoutMillis; } + public String reqId() { + return reqId; + } + public static final class Builder { private HttpExecuteRequest request; private long readBufferSize; private HttpStreamManager streamManager; private long connectionAcquisitionTimeoutMillis; + private String reqId; private Builder() { } @@ -89,6 +96,11 @@ public Builder connectionAcquisitionTimeoutMillis(long connectionAcquisitionTime return this; } + public Builder reqId(String reqId) { + this.reqId = reqId; + return this; + } + public CrtRequestContext build() { return new CrtRequestContext(this); } diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java index 53ee36bb1725..9c1514774009 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java @@ -38,6 +38,7 @@ public Result execute(CrtRequestContext executionContext) { CompletableFuture requestFuture = new CompletableFuture<>(); MetricCollector metricCollector = executionContext.metricCollector(); boolean shouldPublishMetrics = metricCollector != null && !(metricCollector instanceof NoOpMetricCollector); + String tag = "[reqId=" + executionContext.reqId() + "] "; // get acquireStartTime as early as possible for the concurrency timer, but only when metrics are // enabled since clock_gettime() is a full sys call barrier (multiple mutexes and a hw interrupt). @@ -47,14 +48,14 @@ public Result execute(CrtRequestContext executionContext) { InputStreamAdaptingHttpStreamResponseHandler crtResponseHandler = new InputStreamAdaptingHttpStreamResponseHandler(requestFuture); SyncCrtRequest syncCrtRequest = CrtRequestAdapter.toCrtRequest(executionContext); - LOG.info(() -> "execute() acquireStream invoked"); + LOG.info(() -> tag + "execute() acquireStream invoked"); CompletableFuture streamFuture = executionContext.streamManager().acquireStream(syncCrtRequest.httpRequest(), crtResponseHandler); // Evict the connection from the pool on failure so it is not reused. requestFuture.whenComplete((r, t) -> { if (t != null) { - LOG.info(() -> "execute() requestFuture exceptional: closeConnection() (cause=" + LOG.info(() -> tag + "execute() requestFuture exceptional: closeConnection() (cause=" + t.getClass().getSimpleName() + ")"); crtResponseHandler.closeConnection(); } @@ -63,24 +64,24 @@ public Result execute(CrtRequestContext executionContext) { long finalAcquireStartTime = acquireStartTime; streamFuture.whenComplete((streamBase, throwable) -> { if (throwable == null) { - LOG.info(() -> "execute() streamFuture.whenComplete fired with success"); + LOG.info(() -> tag + "execute() streamFuture.whenComplete fired with success"); crtResponseHandler.onAcquireStream(streamBase); } else { - LOG.info(() -> "execute() streamFuture.whenComplete fired with throwable=" + throwable.getClass().getName() - + ": " + throwable.getMessage()); + LOG.info(() -> tag + "execute() streamFuture.whenComplete fired with throwable=" + + throwable.getClass().getName() + ": " + throwable.getMessage()); } if (shouldPublishMetrics) { reportMetrics(executionContext.streamManager(), metricCollector, finalAcquireStartTime); } if (throwable != null) { - LOG.info(() -> "execute() completing requestFuture exceptionally from streamFuture failure"); + LOG.info(() -> tag + "execute() completing requestFuture exceptionally from streamFuture failure"); requestFuture.completeExceptionally(wrapCrtException(throwable)); } }); return new Result(requestFuture, syncCrtRequest.pump(), streamFuture); } catch (Throwable t) { - LOG.info(() -> "execute() outer catch, completing requestFuture exceptionally: " + t.getClass().getName()); + LOG.info(() -> tag + "execute() outer catch, completing requestFuture exceptionally: " + t.getClass().getName()); requestFuture.completeExceptionally(t); return new Result(requestFuture, null, null); } diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java index c6848f7872c9..986eff9fe1a1 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java @@ -64,6 +64,7 @@ enum State { private final ArrayBlockingQueue ready; private final Deque free; private final AtomicReference state = new AtomicReference<>(State.OPEN); + private final String tag; /** * Guards the free deque, allocated counter, and producer wait/notify protocol. Kept private * so external code cannot synchronize on the pipe instance and stall the producer. @@ -75,6 +76,10 @@ enum State { private Chunk pendingDrain; BodyChunkPipe(int depth, int chunkSize) { + this(depth, chunkSize, "-"); + } + + BodyChunkPipe(int depth, int chunkSize, String reqId) { if (depth < 1) { throw new IllegalArgumentException("depth must be >= 1"); } @@ -85,6 +90,7 @@ enum State { this.chunkSize = chunkSize; this.ready = new ArrayBlockingQueue<>(depth); this.free = new ArrayDeque<>(depth); + this.tag = "[reqId=" + reqId + "] "; } /** @@ -100,7 +106,7 @@ Chunk acquireForFill() throws InterruptedException { while (true) { State s = state.get(); if (s == State.ABORTED || s == State.ERROR) { - LOG.debug(() -> "acquireForFill returning null, state=" + s); + LOG.debug(() -> tag + "acquireForFill returning null, state=" + s); return null; } Chunk c = free.pollFirst(); @@ -139,7 +145,7 @@ void publish(Chunk chunk) throws InterruptedException { */ void signalEof() { if (state.compareAndSet(State.OPEN, State.EOF)) { - LOG.debug(() -> "state OPEN -> EOF"); + LOG.debug(() -> tag + "state OPEN -> EOF"); } } @@ -153,7 +159,7 @@ void signalError(Throwable t) { // harmless if the CAS later loses (idempotent signal). error = t; if (state.compareAndSet(State.OPEN, State.ERROR)) { - LOG.debug(() -> "state OPEN -> ERROR (" + t.getClass().getSimpleName() + ")"); + LOG.debug(() -> tag + "state OPEN -> ERROR (" + t.getClass().getSimpleName() + ")"); } freeLock.notifyAll(); } @@ -165,7 +171,7 @@ void signalError(Throwable t) { void abort() { synchronized (freeLock) { if (state.compareAndSet(State.OPEN, State.ABORTED)) { - LOG.debug(() -> "state OPEN -> ABORTED"); + LOG.debug(() -> tag + "state OPEN -> ABORTED"); ready.clear(); } freeLock.notifyAll(); diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/CrtRequestAdapter.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/CrtRequestAdapter.java index 4a3991c5d44a..734c5a352227 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/CrtRequestAdapter.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/CrtRequestAdapter.java @@ -100,9 +100,10 @@ public static SyncCrtRequest toCrtRequest(CrtRequestContext request) { return new SyncCrtRequest(new HttpRequest(method, finalEncodedPath, crtHeaderArray, null), null); } - BodyChunkPipe pipe = new BodyChunkPipe(PIPE_DEPTH, CHUNK_SIZE); + String reqId = request.reqId(); + BodyChunkPipe pipe = new BodyChunkPipe(PIPE_DEPTH, CHUNK_SIZE, reqId); PipeBackedRequestBodyStream bodyStream = new PipeBackedRequestBodyStream(pipe); - SyncRequestBodyPump pump = new SyncRequestBodyPump(providerOpt.get(), pipe); + SyncRequestBodyPump pump = new SyncRequestBodyPump(providerOpt.get(), pipe, reqId); HttpRequest crtRequest = new HttpRequest(method, finalEncodedPath, crtHeaderArray, bodyStream); return new SyncCrtRequest(crtRequest, pump); } diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java index 40ebe39902ec..254b4127ea3e 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java @@ -32,10 +32,16 @@ public final class SyncRequestBodyPump { private final ContentStreamProvider contentStreamProvider; private final BodyChunkPipe pipe; + private final String tag; SyncRequestBodyPump(ContentStreamProvider contentStreamProvider, BodyChunkPipe pipe) { + this(contentStreamProvider, pipe, "-"); + } + + SyncRequestBodyPump(ContentStreamProvider contentStreamProvider, BodyChunkPipe pipe, String reqId) { this.contentStreamProvider = contentStreamProvider; this.pipe = pipe; + this.tag = "[reqId=" + reqId + "] "; } /** @@ -43,24 +49,24 @@ public final class SyncRequestBodyPump { * event-loop thread. On EOF signals the pipe normally; on {@link IOException} signals an error and rethrows. */ public void pump() throws IOException { - LOG.info(() -> "pump() entered"); + LOG.info(() -> tag + "pump() entered"); try (InputStream in = contentStreamProvider.newStream()) { while (true) { Chunk chunk = pipe.acquireForFill(); if (chunk == null) { - LOG.info(() -> "pump() exiting due to abort (acquireForFill returned null)"); + LOG.info(() -> tag + "pump() exiting due to abort (acquireForFill returned null)"); return; } int read; try { read = in.read(chunk.data(), 0, chunk.data().length); } catch (IOException ioe) { - LOG.info(() -> "pump() exiting due to error: " + ioe.getMessage()); + LOG.info(() -> tag + "pump() exiting due to error: " + ioe.getMessage()); pipe.signalError(ioe); throw ioe; } if (read < 0) { - LOG.info(() -> "pump() exiting due to eof"); + LOG.info(() -> tag + "pump() exiting due to eof"); pipe.signalEof(); return; } @@ -69,7 +75,7 @@ public void pump() throws IOException { pipe.publish(chunk); } } catch (InterruptedException ie) { - LOG.info(() -> "pump() exiting due to interrupt"); + LOG.info(() -> tag + "pump() exiting due to interrupt"); pipe.abort(); Thread.currentThread().interrupt(); throw new IOException("Interrupted while writing request body", ie); @@ -80,7 +86,7 @@ public void pump() throws IOException { * Abort the underlying pipe (e.g., when the caller's {@code call()} is cancelled). */ public void abort() { - LOG.info(() -> "pump.abort() called"); + LOG.info(() -> tag + "pump.abort() called"); pipe.abort(); } } diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/LongRunningRequestTestSupport.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/LongRunningRequestTestSupport.java index 098ad470b962..21d04a5c9446 100644 --- a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/LongRunningRequestTestSupport.java +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/LongRunningRequestTestSupport.java @@ -20,12 +20,18 @@ import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo; import com.github.tomakehurst.wiremock.junit5.WireMockExtension; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.UncheckedIOException; import java.lang.management.ManagementFactory; import java.lang.management.ThreadInfo; import java.lang.management.ThreadMXBean; +import java.net.URI; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; +import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import software.amazon.awssdk.utils.Logger; @@ -78,27 +84,109 @@ public static void stubHanging(WireMockExtension mockServer) { .withFixedDelay((int) HANG_DELAY.toMillis()))); } + /** + * Async-executes a POST against {@code mockServer} on a worker thread. A per-call random + * {@code testReqId} is logged BEFORE the request is dispatched and propagated to the SDK via the + * {@code x-aws-sdk-test-id} header so the SDK can prefix its lifecycle logs with the same id. + */ + public static TestRequestExecution executeAsync(SdkHttpClient client, WireMockExtension mockServer) { + String testReqId = "test-" + String.format("%08x", ThreadLocalRandom.current().nextInt()); + log.info(() -> "TEST REQUEST ID: " + testReqId + " (dispatching)"); + CompletableFuture future = CompletableFuture.supplyAsync(() -> { + executeRequest(client, mockServer, testReqId); + return null; + }); + return new TestRequestExecution(testReqId, future); + } + + private static void executeRequest(SdkHttpClient client, WireMockExtension mockServer, String testReqId) { + URI uri = URI.create("http://localhost:" + mockServer.getPort()); + SdkHttpFullRequest request = SdkHttpFullRequest.builder() + .uri(uri) + .method(SdkHttpMethod.POST) + .putHeader("Host", uri.getHost()) + .putHeader("Content-Length", "4") + .putHeader("x-aws-sdk-test-id", testReqId) + .contentStreamProvider(() -> new ByteArrayInputStream( + "Body".getBytes(StandardCharsets.UTF_8))) + .build(); + try { + HttpExecuteResponse response = client.prepareRequest(HttpExecuteRequest.builder() + .request(request) + .contentStreamProvider( + request.contentStreamProvider() + .orElse(null)) + .build()) + .call(); + response.responseBody().ifPresent(body -> { + try { + while (body.read() != -1) { + // drain body so mid-body timeouts surface + } + body.close(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + public static void assertFailsWithinTimeBound(TestRequestExecution execution, Duration expectedTimeout) { + assertFailsWithinTimeBound(execution.future(), execution.testReqId(), expectedTimeout); + } + public static void assertFailsWithinTimeBound(CompletableFuture future, Duration expectedTimeout) { + assertFailsWithinTimeBound(future, "(unknown)", expectedTimeout); + } + + private static void assertFailsWithinTimeBound(CompletableFuture future, String testReqId, Duration expectedTimeout) { Duration maxWait = expectedTimeout.plus(TIME_BOUND_SAFETY_MARGIN); try { future.get(maxWait.toMillis(), TimeUnit.MILLISECONDS); - throw new AssertionError("Expected request to throw an exception but it completed successfully"); + throw new AssertionError("Expected request " + testReqId + + " to throw an exception but it completed successfully"); } catch (TimeoutException e) { - // Dump threads BEFORE cancelling the future so the snapshot reflects the hang. + // Bookend the thread dump with the test reqId so surefire output can be grep'd by request. + log.error(() -> "TEST REQUEST ID: " + testReqId + " (timed out, dumping threads)"); log.error(() -> dumpAllThreads()); future.cancel(true); throw new AssertionError( - "Expected request to fail within " + maxWait + " but it was still running - client appears to hang", + "Expected request " + testReqId + " to fail within " + maxWait + + " but it was still running - client appears to hang", e); } catch (InterruptedException e) { Thread.currentThread().interrupt(); - throw new AssertionError("Unexpected interruption while waiting for request to fail", e); + throw new AssertionError("Unexpected interruption while waiting for request " + testReqId + " to fail", e); } catch (ExecutionException e) { // expected } } + /** + * Bundles a worker future with the per-call test reqId so the assertion helper can reference it in + * failure messages. + */ + public static final class TestRequestExecution { + private final String testReqId; + private final CompletableFuture future; + + TestRequestExecution(String testReqId, CompletableFuture future) { + this.testReqId = testReqId; + this.future = future; + } + + public String testReqId() { + return testReqId; + } + + public CompletableFuture future() { + return future; + } + } + static String dumpAllThreads() { ThreadMXBean tmx = ManagementFactory.getThreadMXBean(); ThreadInfo[] infos = tmx.dumpAllThreads(true, true); diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientLongRunningRequestTestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientLongRunningRequestTestSuite.java index 0e52eb19eb49..4403db37a425 100644 --- a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientLongRunningRequestTestSuite.java +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientLongRunningRequestTestSuite.java @@ -19,19 +19,15 @@ import static software.amazon.awssdk.http.LongRunningRequestTestSupport.CONFIGURED_TIMEOUT; import static software.amazon.awssdk.http.LongRunningRequestTestSupport.HANG_DELAY; import static software.amazon.awssdk.http.LongRunningRequestTestSupport.assertFailsWithinTimeBound; +import static software.amazon.awssdk.http.LongRunningRequestTestSupport.executeAsync; import static software.amazon.awssdk.http.LongRunningRequestTestSupport.stubHanging; import static software.amazon.awssdk.http.LongRunningRequestTestSupport.stubLongPolling; import static software.amazon.awssdk.http.LongRunningRequestTestSupport.stubStreamingWithPauses; import com.github.tomakehurst.wiremock.junit5.WireMockExtension; -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.io.UncheckedIOException; -import java.net.URI; -import java.nio.charset.StandardCharsets; -import java.util.concurrent.CompletableFuture; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import software.amazon.awssdk.http.LongRunningRequestTestSupport.TestRequestExecution; import software.amazon.awssdk.utils.AttributeMap; /** @@ -56,7 +52,7 @@ public void executeWhenReadTimeoutAndServerDelaysResponseFailsWithinTimeoutBound CONFIGURED_TIMEOUT) .build()); try { - assertFailsWithinTimeBound(executeAsync(client), CONFIGURED_TIMEOUT); + assertFailsWithinTimeBound(executeAsync(client, mockServer), CONFIGURED_TIMEOUT); } finally { client.close(); } @@ -71,7 +67,7 @@ public void executeWhenReadTimeoutAndStreamingResponsePausesFailsWithinTimeoutBo CONFIGURED_TIMEOUT) .build()); try { - assertFailsWithinTimeBound(executeAsync(client), CONFIGURED_TIMEOUT); + assertFailsWithinTimeBound(executeAsync(client, mockServer), CONFIGURED_TIMEOUT); } finally { client.close(); } @@ -89,54 +85,14 @@ public void executeWhenConnectionAcquireTimeoutAndPoolExhaustedFailsWithinTimeou CONFIGURED_TIMEOUT) .build()); try { - CompletableFuture firstRequest = executeAsync(client); + TestRequestExecution firstRequest = executeAsync(client, mockServer); Thread.sleep(500); - assertFailsWithinTimeBound(executeAsync(client), CONFIGURED_TIMEOUT); + assertFailsWithinTimeBound(executeAsync(client, mockServer), CONFIGURED_TIMEOUT); - firstRequest.cancel(true); + firstRequest.future().cancel(true); } finally { client.close(); } } - - private CompletableFuture executeAsync(SdkHttpClient client) { - return CompletableFuture.supplyAsync(() -> { - executeRequest(client); - return null; - }); - } - - private void executeRequest(SdkHttpClient client) { - URI uri = URI.create("http://localhost:" + mockServer.getPort()); - SdkHttpFullRequest request = SdkHttpFullRequest.builder() - .uri(uri) - .method(SdkHttpMethod.POST) - .putHeader("Host", uri.getHost()) - .putHeader("Content-Length", "4") - .contentStreamProvider(() -> new ByteArrayInputStream( - "Body".getBytes(StandardCharsets.UTF_8))) - .build(); - try { - HttpExecuteResponse response = client.prepareRequest(HttpExecuteRequest.builder() - .request(request) - .contentStreamProvider( - request.contentStreamProvider() - .orElse(null)) - .build()) - .call(); - response.responseBody().ifPresent(body -> { - try { - while (body.read() != -1) { - // drain body so mid-body timeouts surface - } - body.close(); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - }); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } } From f4920aa715322d576e97e6bdaf75f3243dcf139f Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Sun, 14 Jun 2026 11:37:11 -0700 Subject: [PATCH 09/12] Fix BodyChunkPipe consumer race that drops chunks on terminal state The consumer's pollDrain could observe an empty ready queue followed by a terminal state (EOF/ERROR/ABORTED) even when the producer had published a chunk before flipping state. The window: consumer's first ready.poll runs before the producer's ready.put becomes visible, then the consumer reads the volatile state (already flipped) and returns -1, dropping the chunk. On Catapult JDK8 / weakly-ordered hardware, CRT then aborts the stream with AWS_ERROR_HTTP_OUTGOING_STREAM_LENGTH_INCORRECT. Fix: after observing a terminal state with an empty queue, re-poll once. The producer's program order is publish() (ready.put) THEN the volatile state CAS, so once we have observed the volatile transition, the prior put is guaranteed visible to a subsequent poll on this thread. Also moves Chunk.reset() inside the freeLock-guarded block in recycle() to remove a fragile transitive happens-before path on chunk reuse, and strengthens the single-consumer warning on pollDrain's javadoc since pendingDrain is non-volatile and concurrent invocation would silently corrupt body delivery. --- .../crt/internal/request/BodyChunkPipe.java | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java index 986eff9fe1a1..852a1730ab12 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java @@ -181,9 +181,9 @@ void abort() { /** * Consumer side: drain bytes into {@code dst}. NEVER blocks. * - *

Single-consumer: CRT invokes this only on the request's outgoing-stream task, which is - * scheduled serially on one event-loop thread per stream. {@code pendingDrain} is therefore - * not volatile - it is written and read by that single consumer thread. + *

Single-consumer only. {@code pendingDrain} is non-volatile, so this method MUST be + * invoked from a single thread. CRT honors that by scheduling the outgoing-stream task serially + * on one event-loop thread per stream. Concurrent invocation will silently corrupt body delivery. * * @return number of bytes drained, or {@code -1} on EOF with no remaining data. * @throws RuntimeException if the pipe is in ERROR or ABORTED state with no remaining data. @@ -195,16 +195,28 @@ int pollDrain(ByteBuffer dst) { pendingDrain = ready.poll(); } if (pendingDrain == null) { - switch (state.get()) { + State s = state.get(); + if (s == State.OPEN) { + return totalBytesConsumed; + } + // JMM happens-before: the producer's program order is publish() (ready.put) THEN + // signalEof/Error/abort (volatile state CAS). Once we observe the volatile state + // transition here, the producer's prior ready.put is guaranteed visible to a + // subsequent poll on this thread. Re-poll once to drain any chunk that landed in + // ready before the producer's terminal CAS - otherwise we'd drop it, causing CRT + // to fire AWS_ERROR_HTTP_OUTGOING_STREAM_LENGTH_INCORRECT. + pendingDrain = ready.poll(); + if (pendingDrain != null) { + continue; + } + switch (s) { case ERROR: throw new RuntimeException("Producer failed", error); case ABORTED: throw new RuntimeException("Request body stream was aborted"); case EOF: return totalBytesConsumed > 0 ? totalBytesConsumed : -1; - case OPEN: default: - // OPEN with empty queue: return what we have (possibly 0); CRT will retry. return totalBytesConsumed; } } @@ -245,8 +257,8 @@ int allocatedForTest() { } private void recycle(Chunk c) { - c.reset(); synchronized (freeLock) { + c.reset(); free.push(c); freeLock.notifyAll(); } From 94e885a8ffb081f0938473d3804d50b6c3520c06 Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Sun, 14 Jun 2026 13:47:16 -0700 Subject: [PATCH 10/12] Replace Chunk class with ByteBuffer The pipe's hand-off buffer was a custom Chunk type wrapping a byte[] plus pos/len/reset. ByteBuffer is the standard NIO primitive for exactly that role, so use it directly: position/limit replace pos/len, clear() replaces reset(), and the producer/consumer mode transitions follow the standard flip()/clear() conventions. Producer reads via input.read(bb.array(), bb.arrayOffset()+pos, bb.remaining()), advances position, then flip()s to read mode before publish. Consumer's pollDrain copies into dst by capping pendingDrain.limit so dst.put(src) consumes only the bytes that fit. No behavior change. All 284 aws-crt-client tests still pass. --- .../crt/internal/request/BodyChunkPipe.java | 97 +++++++++------- .../http/crt/internal/request/Chunk.java | 63 ---------- .../internal/request/SyncRequestBodyPump.java | 9 +- .../internal/request/BodyChunkPipeTest.java | 108 +++++++++--------- .../PipeBackedRequestBodyStreamTest.java | 16 +-- 5 files changed, 123 insertions(+), 170 deletions(-) delete mode 100644 http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/Chunk.java diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java index 852a1730ab12..3e6f84ad41d0 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java @@ -27,18 +27,23 @@ /** * Bounded producer/consumer hand-off between the caller thread (producer) and the CRT event-loop thread (consumer). * - *

The producer reads from the customer's {@code InputStream} and {@link #publish(Chunk) publishes} chunks - * into a bounded {@link ArrayBlockingQueue}. The consumer drains those chunks via {@link #pollDrain(ByteBuffer)}, - * which is non-blocking: if no data is ready the consumer returns 0 bytes and CRT reschedules itself via - * {@code aws_channel_schedule_task_now}. + *

The producer reads from the customer's {@code InputStream} into heap {@link ByteBuffer}s acquired + * via {@link #acquireForFill()}, then {@link #publish(ByteBuffer) publishes} them into a bounded + * {@link ArrayBlockingQueue}. The consumer drains those buffers via {@link #pollDrain(ByteBuffer)}, + * which is non-blocking: if no data is ready the consumer returns 0 bytes and CRT reschedules itself + * via {@code aws_channel_schedule_task_now}. * - *

Drained chunks are returned to a free {@link ArrayDeque} (LIFO for cache hotness) guarded by this - * monitor. The producer parks on this monitor when the free deque is empty, providing back-pressure. + *

Drained buffers are returned to a free {@link ArrayDeque} (LIFO for cache hotness) guarded by a + * private monitor. The producer parks on this monitor when the free deque is empty, providing back-pressure. * - *

Chunk byte[] buffers are allocated lazily on the producer's first {@link #acquireForFill()}, not in - * the constructor. This keeps per-request heap minimal while a request is queued on the CRT connection + *

Buffers are heap-allocated lazily on the producer's first {@link #acquireForFill()}, not in the + * constructor. This keeps per-request heap minimal while a request is queued on the CRT connection * pool waiting for a stream: the pipe object exists but its backing buffers do not. * + *

Buffer modes follow standard NIO conventions: {@link #acquireForFill()} returns a buffer in + * write mode (cleared); the producer fills it, calls {@code flip()}, and {@link #publish(ByteBuffer) + * publishes}. The consumer drains in read mode and {@code clear()}s during recycle. + * *

State machine: {@code OPEN -> {EOF | ERROR | ABORTED}}. Transitions are one-way. */ @SdkInternalApi @@ -61,8 +66,8 @@ enum State { private final int depth; private final int chunkSize; - private final ArrayBlockingQueue ready; - private final Deque free; + private final ArrayBlockingQueue ready; + private final Deque free; private final AtomicReference state = new AtomicReference<>(State.OPEN); private final String tag; /** @@ -73,7 +78,7 @@ enum State { private int allocated; private volatile Throwable error; - private Chunk pendingDrain; + private ByteBuffer pendingDrain; BodyChunkPipe(int depth, int chunkSize) { this(depth, chunkSize, "-"); @@ -94,14 +99,15 @@ enum State { } /** - * Producer side: acquire a chunk to fill. Blocks if all chunks are currently in flight. - * Returns {@code null} only if the pipe was aborted while the producer was waiting. + * Producer side: acquire a buffer in write mode (position=0, limit=capacity). Blocks if all + * buffers are currently in flight. Returns {@code null} only if the pipe was aborted while the + * producer was waiting. * - *

Allocates the chunk's backing byte[] on first use up to the configured depth. This keeps the - * per-request footprint minimal until the producer actually starts pumping (i.e., until after the - * CRT stream has been acquired). + *

Allocates the buffer on first use up to the configured depth. This keeps the per-request + * footprint minimal until the producer actually starts pumping (i.e., until after the CRT stream + * has been acquired). */ - Chunk acquireForFill() throws InterruptedException { + ByteBuffer acquireForFill() throws InterruptedException { synchronized (freeLock) { while (true) { State s = state.get(); @@ -109,13 +115,13 @@ Chunk acquireForFill() throws InterruptedException { LOG.debug(() -> tag + "acquireForFill returning null, state=" + s); return null; } - Chunk c = free.pollFirst(); - if (c != null) { - return c; + ByteBuffer bb = free.pollFirst(); + if (bb != null) { + return bb; } if (allocated < depth) { allocated++; - return new Chunk(chunkSize); + return ByteBuffer.allocate(chunkSize); } freeLock.wait(ACQUIRE_WAIT_TIMEOUT_MS); } @@ -123,15 +129,15 @@ Chunk acquireForFill() throws InterruptedException { } /** - * Producer side: publish a filled chunk to the consumer. Caller must have set - * {@link Chunk#len(int)} before calling. + * Producer side: publish a filled buffer to the consumer. Caller must have called + * {@link ByteBuffer#flip()} so the buffer is in read mode (position=0, limit=N). * - *

If the chunk is empty (zero-length read), it is recycled back to the free deque rather than - * pushed to the ready queue: an empty chunk would otherwise be leaked from the bounded pool, and - * the consumer would interpret it as a no-op anyway. + *

If the buffer has no remaining bytes (zero-length read), it is recycled back to the free + * deque rather than pushed to the ready queue: an empty buffer would otherwise be leaked from + * the bounded pool, and the consumer would interpret it as a no-op anyway. */ - void publish(Chunk chunk) throws InterruptedException { - if (chunk.len() == 0) { + void publish(ByteBuffer chunk) throws InterruptedException { + if (!chunk.hasRemaining()) { recycle(chunk); return; } @@ -203,8 +209,7 @@ int pollDrain(ByteBuffer dst) { // signalEof/Error/abort (volatile state CAS). Once we observe the volatile state // transition here, the producer's prior ready.put is guaranteed visible to a // subsequent poll on this thread. Re-poll once to drain any chunk that landed in - // ready before the producer's terminal CAS - otherwise we'd drop it, causing CRT - // to fire AWS_ERROR_HTTP_OUTGOING_STREAM_LENGTH_INCORRECT. + // ready before the producer's terminal CAS. pendingDrain = ready.poll(); if (pendingDrain != null) { continue; @@ -220,15 +225,19 @@ int pollDrain(ByteBuffer dst) { return totalBytesConsumed; } } - int n = Math.min(dst.remaining(), pendingDrain.len() - pendingDrain.pos()); - dst.put(pendingDrain.data(), pendingDrain.pos(), n); - pendingDrain.pos(pendingDrain.pos() + n); + int n = Math.min(dst.remaining(), pendingDrain.remaining()); + // Cap source's limit so dst.put(src) only consumes n bytes (avoids BufferOverflowException + // when pendingDrain.remaining() > dst.remaining()). + int srcOrigLimit = pendingDrain.limit(); + pendingDrain.limit(pendingDrain.position() + n); + dst.put(pendingDrain); + pendingDrain.limit(srcOrigLimit); totalBytesConsumed += n; - if (pendingDrain.pos() >= pendingDrain.len()) { - // The chunk has been fully copied into dst, so we return it to the free deque - // (and notify the producer in case it was waiting). This is what bounds the pool: - // chunks only re-enter the producer pool after the consumer has drained them. - Chunk drained = pendingDrain; + if (!pendingDrain.hasRemaining()) { + // Buffer fully copied into dst, return it to the free deque (and notify the producer + // in case it was waiting). This is what bounds the pool: buffers only re-enter the + // producer pool after the consumer has drained them. + ByteBuffer drained = pendingDrain; pendingDrain = null; recycle(drained); } @@ -245,9 +254,9 @@ State state() { } /** - * Visible-for-test / test-only helper: number of {@link Chunk} buffers minted so far. The pipe - * lazily allocates chunks on the producer's first {@link #acquireForFill()}, so this is 0 until - * the producer starts pumping and grows up to {@code depth}. + * Visible-for-test / test-only helper: number of buffers minted so far. The pipe lazily allocates + * buffers on the producer's first {@link #acquireForFill()}, so this is 0 until the producer + * starts pumping and grows up to {@code depth}. */ @SdkTestInternalApi int allocatedForTest() { @@ -256,10 +265,10 @@ int allocatedForTest() { } } - private void recycle(Chunk c) { + private void recycle(ByteBuffer bb) { synchronized (freeLock) { - c.reset(); - free.push(c); + bb.clear(); + free.push(bb); freeLock.notifyAll(); } } diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/Chunk.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/Chunk.java deleted file mode 100644 index 0797b8cbc67a..000000000000 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/Chunk.java +++ /dev/null @@ -1,63 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"). - * You may not use this file except in compliance with the License. - * A copy of the License is located at - * - * http://aws.amazon.com/apache2.0 - * - * or in the "license" file accompanying this file. This file is distributed - * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either - * express or implied. See the License for the specific language governing - * permissions and limitations under the License. - */ - -package software.amazon.awssdk.http.crt.internal.request; - -import software.amazon.awssdk.annotations.SdkInternalApi; - -/** - * A reusable byte buffer carrying request body data through the {@link BodyChunkPipe}. - * - *

Internal to this package; used only by {@link BodyChunkPipe} and {@link SyncRequestBodyPump}. - * - *

{@code pos} and {@code len} are intentionally non-volatile: hand-off between the producer and - * consumer always goes through the {@code ArrayBlockingQueue} (or the {@code freeLock} monitor on - * recycle), both of which provide release/acquire happens-before for the field writes. - */ -@SdkInternalApi -final class Chunk { - private final byte[] data; - private int pos; - private int len; - - Chunk(int chunkSize) { - this.data = new byte[chunkSize]; - } - - byte[] data() { - return data; - } - - int pos() { - return pos; - } - - void pos(int pos) { - this.pos = pos; - } - - int len() { - return len; - } - - void len(int len) { - this.len = len; - } - - void reset() { - this.pos = 0; - this.len = 0; - } -} diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java index 254b4127ea3e..4b7a87b5c3e6 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java @@ -17,6 +17,7 @@ import java.io.IOException; import java.io.InputStream; +import java.nio.ByteBuffer; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.http.ContentStreamProvider; import software.amazon.awssdk.utils.Logger; @@ -52,14 +53,14 @@ public void pump() throws IOException { LOG.info(() -> tag + "pump() entered"); try (InputStream in = contentStreamProvider.newStream()) { while (true) { - Chunk chunk = pipe.acquireForFill(); + ByteBuffer chunk = pipe.acquireForFill(); if (chunk == null) { LOG.info(() -> tag + "pump() exiting due to abort (acquireForFill returned null)"); return; } int read; try { - read = in.read(chunk.data(), 0, chunk.data().length); + read = in.read(chunk.array(), chunk.arrayOffset() + chunk.position(), chunk.remaining()); } catch (IOException ioe) { LOG.info(() -> tag + "pump() exiting due to error: " + ioe.getMessage()); pipe.signalError(ioe); @@ -70,8 +71,8 @@ public void pump() throws IOException { pipe.signalEof(); return; } - chunk.pos(0); - chunk.len(read); + chunk.position(chunk.position() + read); + chunk.flip(); pipe.publish(chunk); } } catch (InterruptedException ie) { diff --git a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipeTest.java b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipeTest.java index 72f29278589b..99097487a29b 100644 --- a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipeTest.java +++ b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipeTest.java @@ -51,12 +51,11 @@ void pollDrain_afterEofWithEmptyQueue_returnsMinusOne() { @Test void publish_thenDrain_consumerSeesProducerBytes() throws Exception { BodyChunkPipe pipe = new BodyChunkPipe(2, 8); - Chunk c = pipe.acquireForFill(); + ByteBuffer bb = pipe.acquireForFill(); byte[] payload = {1, 2, 3, 4, 5}; - System.arraycopy(payload, 0, c.data(), 0, payload.length); - c.pos(0); - c.len(payload.length); - pipe.publish(c); + bb.put(payload); + bb.flip(); + pipe.publish(bb); pipe.signalEof(); ByteBuffer dst = ByteBuffer.allocate(16); @@ -84,9 +83,10 @@ void signalError_pollDrainThrows() { @Test void abort_emptiesReadyAndChangesState() throws Exception { BodyChunkPipe pipe = new BodyChunkPipe(2, 8); - Chunk c = pipe.acquireForFill(); - c.len(4); - pipe.publish(c); + ByteBuffer bb = pipe.acquireForFill(); + bb.put(new byte[]{1, 2, 3, 4}); + bb.flip(); + pipe.publish(bb); pipe.abort(); @@ -97,11 +97,11 @@ void abort_emptiesReadyAndChangesState() throws Exception { @Test void pollDrain_signalErrorWithQueuedChunks_drainsThenThrows() throws Exception { BodyChunkPipe pipe = new BodyChunkPipe(2, 8); - Chunk c = pipe.acquireForFill(); + ByteBuffer bb = pipe.acquireForFill(); byte[] payload = {7, 8, 9}; - System.arraycopy(payload, 0, c.data(), 0, payload.length); - c.len(payload.length); - pipe.publish(c); + bb.put(payload); + bb.flip(); + pipe.publish(bb); pipe.signalError(new RuntimeException("boom")); ByteBuffer dst = ByteBuffer.allocate(payload.length); @@ -120,11 +120,11 @@ void pollDrain_signalErrorWithQueuedChunks_drainsThenThrows() throws Exception { @Test void pollDrain_signalEofWithQueuedChunks_drainsThenReturnsMinusOne() throws Exception { BodyChunkPipe pipe = new BodyChunkPipe(2, 8); - Chunk c = pipe.acquireForFill(); + ByteBuffer bb = pipe.acquireForFill(); byte[] payload = {10, 20, 30}; - System.arraycopy(payload, 0, c.data(), 0, payload.length); - c.len(payload.length); - pipe.publish(c); + bb.put(payload); + bb.flip(); + pipe.publish(bb); pipe.signalEof(); ByteBuffer dst = ByteBuffer.allocate(payload.length); @@ -153,11 +153,11 @@ void abort_afterSignalEof_leavesStateAsEof() { @Test void abort_afterSignalEofWithQueuedChunks_doesNotClearReady() throws Exception { BodyChunkPipe pipe = new BodyChunkPipe(2, 8); - Chunk c = pipe.acquireForFill(); + ByteBuffer bb = pipe.acquireForFill(); byte[] payload = {1, 2, 3}; - System.arraycopy(payload, 0, c.data(), 0, payload.length); - c.len(payload.length); - pipe.publish(c); + bb.put(payload); + bb.flip(); + pipe.publish(bb); pipe.signalEof(); pipe.abort(); @@ -172,9 +172,10 @@ void abort_afterSignalEofWithQueuedChunks_doesNotClearReady() throws Exception { @Test void recycle_intoEofPipe_doesNotThrowAndDoesNotCorruptPool() throws Exception { BodyChunkPipe pipe = new BodyChunkPipe(2, 8); - Chunk c = pipe.acquireForFill(); - c.len(4); - pipe.publish(c); + ByteBuffer bb = pipe.acquireForFill(); + bb.put(new byte[]{1, 2, 3, 4}); + bb.flip(); + pipe.publish(bb); pipe.signalEof(); ByteBuffer dst = ByteBuffer.allocate(8); @@ -189,11 +190,11 @@ void recycle_intoEofPipe_doesNotThrowAndDoesNotCorruptPool() throws Exception { @Test void recycle_intoAbortedPipe_doesNotThrow() throws Exception { BodyChunkPipe pipe = new BodyChunkPipe(2, 8); - Chunk c = pipe.acquireForFill(); + ByteBuffer bb = pipe.acquireForFill(); pipe.abort(); - c.len(0); - pipe.publish(c); + bb.flip(); + pipe.publish(bb); assertThat(pipe.state()).isEqualTo(BodyChunkPipe.State.ABORTED); } @@ -201,11 +202,11 @@ void recycle_intoAbortedPipe_doesNotThrow() throws Exception { @Test void recycle_intoErrorPipe_doesNotThrow() throws Exception { BodyChunkPipe pipe = new BodyChunkPipe(2, 8); - Chunk c = pipe.acquireForFill(); + ByteBuffer bb = pipe.acquireForFill(); pipe.signalError(new RuntimeException("boom")); - c.len(0); - pipe.publish(c); + bb.flip(); + pipe.publish(bb); assertThat(pipe.state()).isEqualTo(BodyChunkPipe.State.ERROR); } @@ -221,24 +222,27 @@ void constructor_doesNotAllocateChunks() { void acquireForFill_firstCall_allocatesOneChunk() throws Exception { BodyChunkPipe pipe = new BodyChunkPipe(4, 16); - Chunk c = pipe.acquireForFill(); + ByteBuffer bb = pipe.acquireForFill(); - assertThat(c).isNotNull(); - assertThat(c.data()).hasSize(16); + assertThat(bb).isNotNull(); + assertThat(bb.capacity()).isEqualTo(16); + assertThat(bb.position()).isZero(); + assertThat(bb.limit()).isEqualTo(16); assertThat(pipe.allocatedForTest()).isEqualTo(1); } @Test void acquireForFill_uniqueChunksUpToDepth_thenStopsAllocating() throws Exception { BodyChunkPipe pipe = new BodyChunkPipe(3, 8); - Chunk c1 = pipe.acquireForFill(); - Chunk c2 = pipe.acquireForFill(); - Chunk c3 = pipe.acquireForFill(); + ByteBuffer c1 = pipe.acquireForFill(); + ByteBuffer c2 = pipe.acquireForFill(); + ByteBuffer c3 = pipe.acquireForFill(); - c1.len(1); + c1.put((byte) 1); + c1.flip(); pipe.publish(c1); pipe.pollDrain(ByteBuffer.allocate(8)); - Chunk reused = pipe.acquireForFill(); + ByteBuffer reused = pipe.acquireForFill(); assertThat(c1).isNotSameAs(c2).isNotSameAs(c3); assertThat(c2).isNotSameAs(c3); @@ -249,14 +253,15 @@ void acquireForFill_uniqueChunksUpToDepth_thenStopsAllocating() throws Exception @Test void acquireForFill_recycledChunkReused_noNewAllocation() throws Exception { BodyChunkPipe pipe = new BodyChunkPipe(2, 8); - Chunk c = pipe.acquireForFill(); - c.len(3); - pipe.publish(c); + ByteBuffer bb = pipe.acquireForFill(); + bb.put(new byte[]{1, 2, 3}); + bb.flip(); + pipe.publish(bb); pipe.pollDrain(ByteBuffer.allocate(8)); - Chunk reused = pipe.acquireForFill(); + ByteBuffer reused = pipe.acquireForFill(); - assertThat(reused).isSameAs(c); + assertThat(reused).isSameAs(bb); assertThat(pipe.allocatedForTest()).isEqualTo(1); } @@ -265,9 +270,9 @@ void acquireForFill_afterAbort_returnsNull() throws Exception { BodyChunkPipe pipe = new BodyChunkPipe(2, 8); pipe.abort(); - Chunk c = pipe.acquireForFill(); + ByteBuffer bb = pipe.acquireForFill(); - assertThat(c).isNull(); + assertThat(bb).isNull(); } @Test @@ -275,9 +280,9 @@ void acquireForFill_afterSignalError_returnsNull() throws Exception { BodyChunkPipe pipe = new BodyChunkPipe(2, 8); pipe.signalError(new RuntimeException("boom")); - Chunk c = pipe.acquireForFill(); + ByteBuffer bb = pipe.acquireForFill(); - assertThat(c).isNull(); + assertThat(bb).isNull(); } @Test @@ -363,18 +368,19 @@ void signalError_concurrentPollDrain_consumerNeverSeesNullCause() throws Excepti @Test void acquireForFill_blocksUntilConsumerRecycles_thenWakesAndCompletes() throws Exception { BodyChunkPipe pipe = new BodyChunkPipe(1, 8); - Chunk first = pipe.acquireForFill(); - first.len(4); + ByteBuffer first = pipe.acquireForFill(); + first.put(new byte[]{1, 2, 3, 4}); + first.flip(); pipe.publish(first); CountDownLatch producerEntered = new CountDownLatch(1); - AtomicReference reused = new AtomicReference<>(); + AtomicReference reused = new AtomicReference<>(); AtomicReference producerError = new AtomicReference<>(); Thread producer = new Thread(() -> { try { producerEntered.countDown(); - Chunk c = pipe.acquireForFill(); - reused.set(c); + ByteBuffer bb = pipe.acquireForFill(); + reused.set(bb); } catch (Throwable t) { producerError.set(t); } diff --git a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/PipeBackedRequestBodyStreamTest.java b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/PipeBackedRequestBodyStreamTest.java index 1add84bb4cdf..6122adbae2d8 100644 --- a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/PipeBackedRequestBodyStreamTest.java +++ b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/request/PipeBackedRequestBodyStreamTest.java @@ -38,11 +38,11 @@ void sendRequestBody_emptyOpenPipe_returnsFalseAndCopiesNothing() { @Test void sendRequestBody_afterEofAndDrained_returnsTrue() throws Exception { BodyChunkPipe pipe = new BodyChunkPipe(2, 8); - Chunk c = pipe.acquireForFill(); + ByteBuffer bb = pipe.acquireForFill(); byte[] payload = {1, 2, 3}; - System.arraycopy(payload, 0, c.data(), 0, payload.length); - c.len(payload.length); - pipe.publish(c); + bb.put(payload); + bb.flip(); + pipe.publish(bb); pipe.signalEof(); PipeBackedRequestBodyStream stream = new PipeBackedRequestBodyStream(pipe); @@ -97,11 +97,11 @@ void resetPosition_returnsFalse() { @Test void sendRequestBody_destinationSmallerThanChunk_drainsAcrossMultipleCalls() throws Exception { BodyChunkPipe pipe = new BodyChunkPipe(2, 16); - Chunk c = pipe.acquireForFill(); + ByteBuffer bb = pipe.acquireForFill(); byte[] payload = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10}; - System.arraycopy(payload, 0, c.data(), 0, payload.length); - c.len(payload.length); - pipe.publish(c); + bb.put(payload); + bb.flip(); + pipe.publish(bb); pipe.signalEof(); PipeBackedRequestBodyStream stream = new PipeBackedRequestBodyStream(pipe); From 28b65324cf848bb7ca144142acb4984cbd33d8c7 Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Sun, 14 Jun 2026 14:17:06 -0700 Subject: [PATCH 11/12] Remove diagnostic logging added during Catapult investigation Strips the per-request reqId correlation, lifecycle LOG.info calls in the sync CRT request path, and the test-side thread-dump-on-timeout helper that were added to chase the Catapult hang. --- .../amazon/awssdk/checkstyle-suppressions.xml | 5 - .../awssdk/http/crt/AwsCrtHttpClient.java | 53 ++------ .../http/crt/internal/CrtRequestContext.java | 12 -- .../http/crt/internal/CrtRequestExecutor.java | 14 +-- .../crt/internal/request/BodyChunkPipe.java | 14 +-- .../internal/request/CrtRequestAdapter.java | 5 +- .../internal/request/SyncRequestBodyPump.java | 15 +-- .../http/LongRunningRequestTestSupport.java | 113 +----------------- ...HttpClientLongRunningRequestTestSuite.java | 58 +++++++-- 9 files changed, 70 insertions(+), 219 deletions(-) diff --git a/build-tools/src/main/resources/software/amazon/awssdk/checkstyle-suppressions.xml b/build-tools/src/main/resources/software/amazon/awssdk/checkstyle-suppressions.xml index 8988a3bcc74a..4e81373b0be3 100644 --- a/build-tools/src/main/resources/software/amazon/awssdk/checkstyle-suppressions.xml +++ b/build-tools/src/main/resources/software/amazon/awssdk/checkstyle-suppressions.xml @@ -72,9 +72,4 @@ - - - diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java index 1fa66cb940a6..10574489a190 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java @@ -22,7 +22,6 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.ExecutionException; -import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.function.Consumer; @@ -43,7 +42,6 @@ import software.amazon.awssdk.http.crt.internal.request.SyncRequestBodyPump; import software.amazon.awssdk.utils.AttributeMap; import software.amazon.awssdk.utils.CompletableFutureUtils; -import software.amazon.awssdk.utils.Logger; /** * An implementation of {@link SdkHttpClient} that uses the AWS Common Runtime (CRT) Http Client to communicate with @@ -105,100 +103,76 @@ public ExecutableHttpRequest prepareRequest(HttpExecuteRequest request) { * request) */ HttpStreamManager streamManager = getOrCreateConnectionPool(poolKey(request.httpRequest())); - // Tests may override via x-aws-sdk-test-id so surefire output can be grep'd by request. - String reqId = request.httpRequest() - .firstMatchingHeader("x-aws-sdk-test-id") - .orElseGet(() -> String.format("%08x", ThreadLocalRandom.current().nextInt())); CrtRequestContext context = CrtRequestContext.builder() .streamManager(streamManager) .readBufferSize(this.readBufferSize) .request(request) .connectionAcquisitionTimeoutMillis(this.connectionAcquisitionTimeout) - .reqId(reqId) .build(); return new CrtHttpRequest(context); } private static final class CrtHttpRequest implements ExecutableHttpRequest { - private static final Logger LOG = Logger.loggerFor(CrtHttpRequest.class); - private final CrtRequestContext context; - private final String reqId; - private final String tag; private volatile CompletableFuture responseFuture; private volatile SyncRequestBodyPump pump; private CrtHttpRequest(CrtRequestContext context) { this.context = context; - this.reqId = context.reqId(); - this.tag = "[reqId=" + reqId + "] "; } @Override public HttpExecuteResponse call() throws IOException { HttpExecuteResponse.Builder builder = HttpExecuteResponse.builder(); - boolean hasBody = context.sdkRequest().contentStreamProvider().isPresent(); - LOG.info(() -> tag + "call() entered, hasBody=" + hasBody); try { CrtRequestExecutor.Result result = new CrtRequestExecutor().execute(context); responseFuture = result.responseFuture(); pump = result.pump(); - LOG.info(() -> tag + "call() executor.execute() returned, streamFuture pending, pump=" - + (pump != null ? "non-null" : "null")); + // Wake a parked producer when CRT signals request failure via responseFuture so the + // pump's blocked acquireForFill() returns instead of holding the caller thread. if (pump != null) { SyncRequestBodyPump pumpRef = pump; responseFuture.whenComplete((r, t) -> { if (t != null) { - LOG.info(() -> tag + "responseFuture hook: invoking pump.abort() (cause=" - + t.getClass().getSimpleName() + ")"); pumpRef.abort(); } }); } - LOG.info(() -> tag + "call() entering waitForStreamAcquired, timeoutMillis=" - + context.connectionAcquisitionTimeoutMillis()); boolean streamAcquired = waitForStreamAcquired(result.streamFuture(), context.connectionAcquisitionTimeoutMillis()); - LOG.info(() -> tag + "call() waitForStreamAcquired returned " + streamAcquired); if (pump != null) { if (streamAcquired) { - LOG.info(() -> tag + "call() entering pump.pump()"); try { pump.pump(); - LOG.info(() -> tag + "call() pump.pump() returned"); } catch (IOException ioe) { - LOG.info(() -> tag + "call() pump.pump() threw IOException: " + ioe.getMessage()); responseFuture.completeExceptionally(ioe); throw ioe; } } else { - LOG.info(() -> tag + "call() invoking pump.abort() (post-wait, streamAcquired=false)"); pump.abort(); } } - LOG.info(() -> tag + "call() entering joinInterruptibly(responseFuture)"); SdkHttpFullResponse response = CompletableFutureUtils.joinInterruptibly(responseFuture); - LOG.info(() -> tag + "call() responseFuture joined: success"); builder.response(response); builder.responseBody(response.content().orElse(null)); - LOG.info(() -> tag + "call() exiting normally"); return builder.build(); } catch (CompletionException e) { Throwable cause = e.getCause(); - LOG.info(() -> tag + "call() catch CompletionException, cause=" - + (cause == null ? "" : cause.getClass().getName() + ": " + cause.getMessage())); + // Complete the future exceptionally to trigger connection cleanup in the response handler. + // Handles the thread-interrupt case where joinInterruptibly throws due to + // InterruptedException, ensuring closeConnection() is invoked to prevent leaking the + // connection from the pool. if (responseFuture != null) { responseFuture.completeExceptionally(cause != null ? cause : e); } if (pump != null) { - LOG.info(() -> tag + "call() catch invoking pump.abort()"); pump.abort(); } @@ -220,38 +194,25 @@ public HttpExecuteResponse call() throws IOException { @Override public void abort() { - LOG.info(() -> tag + "abort() called externally"); if (responseFuture != null) { responseFuture.completeExceptionally(new IOException("Request was cancelled")); } if (pump != null) { - LOG.info(() -> tag + "abort() invoking pump.abort()"); pump.abort(); } } private boolean waitForStreamAcquired(CompletableFuture streamFuture, long timeoutMillis) { if (streamFuture == null) { - LOG.info(() -> tag + "waitForStreamAcquired: streamFuture==null, returning false"); return false; } - LOG.info(() -> tag + "waitForStreamAcquired: starting, timeout=" + timeoutMillis + "ms"); try { streamFuture.get(timeoutMillis, TimeUnit.MILLISECONDS); - LOG.info(() -> tag + "waitForStreamAcquired: streamFuture completed normally"); return true; } catch (InterruptedException e) { Thread.currentThread().interrupt(); - LOG.info(() -> tag + "waitForStreamAcquired: interrupted"); - return false; - } catch (TimeoutException e) { - LOG.warn(() -> tag + "waitForStreamAcquired: timed out after " + timeoutMillis - + "ms - streamFuture still pending"); return false; - } catch (ExecutionException e) { - Throwable cause = e.getCause(); - LOG.info(() -> tag + "waitForStreamAcquired: streamFuture completed exceptionally: " - + (cause == null ? e.getMessage() : cause.getClass().getName() + ": " + cause.getMessage())); + } catch (TimeoutException | ExecutionException e) { return false; } } diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestContext.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestContext.java index ced8fd555082..76771f859d0c 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestContext.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestContext.java @@ -27,7 +27,6 @@ public final class CrtRequestContext { private final HttpStreamManager streamManager; private final MetricCollector metricCollector; private final long connectionAcquisitionTimeoutMillis; - private final String reqId; private CrtRequestContext(Builder builder) { this.request = builder.request; @@ -35,7 +34,6 @@ private CrtRequestContext(Builder builder) { this.streamManager = builder.streamManager; this.metricCollector = request.metricCollector().orElse(null); this.connectionAcquisitionTimeoutMillis = builder.connectionAcquisitionTimeoutMillis; - this.reqId = builder.reqId; } public static Builder builder() { @@ -62,16 +60,11 @@ public long connectionAcquisitionTimeoutMillis() { return connectionAcquisitionTimeoutMillis; } - public String reqId() { - return reqId; - } - public static final class Builder { private HttpExecuteRequest request; private long readBufferSize; private HttpStreamManager streamManager; private long connectionAcquisitionTimeoutMillis; - private String reqId; private Builder() { } @@ -96,11 +89,6 @@ public Builder connectionAcquisitionTimeoutMillis(long connectionAcquisitionTime return this; } - public Builder reqId(String reqId) { - this.reqId = reqId; - return this; - } - public CrtRequestContext build() { return new CrtRequestContext(this); } diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java index 9c1514774009..e69225083357 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java @@ -28,17 +28,14 @@ import software.amazon.awssdk.http.crt.internal.response.InputStreamAdaptingHttpStreamResponseHandler; import software.amazon.awssdk.metrics.MetricCollector; import software.amazon.awssdk.metrics.NoOpMetricCollector; -import software.amazon.awssdk.utils.Logger; @SdkInternalApi public final class CrtRequestExecutor { - private static final Logger LOG = Logger.loggerFor(CrtRequestExecutor.class); public Result execute(CrtRequestContext executionContext) { CompletableFuture requestFuture = new CompletableFuture<>(); MetricCollector metricCollector = executionContext.metricCollector(); boolean shouldPublishMetrics = metricCollector != null && !(metricCollector instanceof NoOpMetricCollector); - String tag = "[reqId=" + executionContext.reqId() + "] "; // get acquireStartTime as early as possible for the concurrency timer, but only when metrics are // enabled since clock_gettime() is a full sys call barrier (multiple mutexes and a hw interrupt). @@ -48,40 +45,33 @@ public Result execute(CrtRequestContext executionContext) { InputStreamAdaptingHttpStreamResponseHandler crtResponseHandler = new InputStreamAdaptingHttpStreamResponseHandler(requestFuture); SyncCrtRequest syncCrtRequest = CrtRequestAdapter.toCrtRequest(executionContext); - LOG.info(() -> tag + "execute() acquireStream invoked"); CompletableFuture streamFuture = executionContext.streamManager().acquireStream(syncCrtRequest.httpRequest(), crtResponseHandler); // Evict the connection from the pool on failure so it is not reused. requestFuture.whenComplete((r, t) -> { if (t != null) { - LOG.info(() -> tag + "execute() requestFuture exceptional: closeConnection() (cause=" - + t.getClass().getSimpleName() + ")"); crtResponseHandler.closeConnection(); } }); long finalAcquireStartTime = acquireStartTime; streamFuture.whenComplete((streamBase, throwable) -> { + // Only notify the response handler when stream acquisition succeeded; passing a null + // streamBase from a failed acquisition would NPE inside the handler. if (throwable == null) { - LOG.info(() -> tag + "execute() streamFuture.whenComplete fired with success"); crtResponseHandler.onAcquireStream(streamBase); - } else { - LOG.info(() -> tag + "execute() streamFuture.whenComplete fired with throwable=" - + throwable.getClass().getName() + ": " + throwable.getMessage()); } if (shouldPublishMetrics) { reportMetrics(executionContext.streamManager(), metricCollector, finalAcquireStartTime); } if (throwable != null) { - LOG.info(() -> tag + "execute() completing requestFuture exceptionally from streamFuture failure"); requestFuture.completeExceptionally(wrapCrtException(throwable)); } }); return new Result(requestFuture, syncCrtRequest.pump(), streamFuture); } catch (Throwable t) { - LOG.info(() -> tag + "execute() outer catch, completing requestFuture exceptionally: " + t.getClass().getName()); requestFuture.completeExceptionally(t); return new Result(requestFuture, null, null); } diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java index 3e6f84ad41d0..6fc436d5b896 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/BodyChunkPipe.java @@ -69,7 +69,6 @@ enum State { private final ArrayBlockingQueue ready; private final Deque free; private final AtomicReference state = new AtomicReference<>(State.OPEN); - private final String tag; /** * Guards the free deque, allocated counter, and producer wait/notify protocol. Kept private * so external code cannot synchronize on the pipe instance and stall the producer. @@ -81,10 +80,6 @@ enum State { private ByteBuffer pendingDrain; BodyChunkPipe(int depth, int chunkSize) { - this(depth, chunkSize, "-"); - } - - BodyChunkPipe(int depth, int chunkSize, String reqId) { if (depth < 1) { throw new IllegalArgumentException("depth must be >= 1"); } @@ -95,7 +90,6 @@ enum State { this.chunkSize = chunkSize; this.ready = new ArrayBlockingQueue<>(depth); this.free = new ArrayDeque<>(depth); - this.tag = "[reqId=" + reqId + "] "; } /** @@ -112,7 +106,7 @@ ByteBuffer acquireForFill() throws InterruptedException { while (true) { State s = state.get(); if (s == State.ABORTED || s == State.ERROR) { - LOG.debug(() -> tag + "acquireForFill returning null, state=" + s); + LOG.debug(() -> "acquireForFill returning null, state=" + s); return null; } ByteBuffer bb = free.pollFirst(); @@ -151,7 +145,7 @@ void publish(ByteBuffer chunk) throws InterruptedException { */ void signalEof() { if (state.compareAndSet(State.OPEN, State.EOF)) { - LOG.debug(() -> tag + "state OPEN -> EOF"); + LOG.debug(() -> "state OPEN -> EOF"); } } @@ -165,7 +159,7 @@ void signalError(Throwable t) { // harmless if the CAS later loses (idempotent signal). error = t; if (state.compareAndSet(State.OPEN, State.ERROR)) { - LOG.debug(() -> tag + "state OPEN -> ERROR (" + t.getClass().getSimpleName() + ")"); + LOG.debug(() -> "state OPEN -> ERROR (" + t.getClass().getSimpleName() + ")"); } freeLock.notifyAll(); } @@ -177,7 +171,7 @@ void signalError(Throwable t) { void abort() { synchronized (freeLock) { if (state.compareAndSet(State.OPEN, State.ABORTED)) { - LOG.debug(() -> tag + "state OPEN -> ABORTED"); + LOG.debug(() -> "state OPEN -> ABORTED"); ready.clear(); } freeLock.notifyAll(); diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/CrtRequestAdapter.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/CrtRequestAdapter.java index 734c5a352227..4a3991c5d44a 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/CrtRequestAdapter.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/CrtRequestAdapter.java @@ -100,10 +100,9 @@ public static SyncCrtRequest toCrtRequest(CrtRequestContext request) { return new SyncCrtRequest(new HttpRequest(method, finalEncodedPath, crtHeaderArray, null), null); } - String reqId = request.reqId(); - BodyChunkPipe pipe = new BodyChunkPipe(PIPE_DEPTH, CHUNK_SIZE, reqId); + BodyChunkPipe pipe = new BodyChunkPipe(PIPE_DEPTH, CHUNK_SIZE); PipeBackedRequestBodyStream bodyStream = new PipeBackedRequestBodyStream(pipe); - SyncRequestBodyPump pump = new SyncRequestBodyPump(providerOpt.get(), pipe, reqId); + SyncRequestBodyPump pump = new SyncRequestBodyPump(providerOpt.get(), pipe); HttpRequest crtRequest = new HttpRequest(method, finalEncodedPath, crtHeaderArray, bodyStream); return new SyncCrtRequest(crtRequest, pump); } diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java index 4b7a87b5c3e6..fa237045ede2 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/request/SyncRequestBodyPump.java @@ -20,7 +20,6 @@ import java.nio.ByteBuffer; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.http.ContentStreamProvider; -import software.amazon.awssdk.utils.Logger; /** * Caller-thread producer that reads from the customer's {@link InputStream} and publishes chunks to a @@ -29,20 +28,13 @@ */ @SdkInternalApi public final class SyncRequestBodyPump { - private static final Logger LOG = Logger.loggerFor(SyncRequestBodyPump.class); private final ContentStreamProvider contentStreamProvider; private final BodyChunkPipe pipe; - private final String tag; SyncRequestBodyPump(ContentStreamProvider contentStreamProvider, BodyChunkPipe pipe) { - this(contentStreamProvider, pipe, "-"); - } - - SyncRequestBodyPump(ContentStreamProvider contentStreamProvider, BodyChunkPipe pipe, String reqId) { this.contentStreamProvider = contentStreamProvider; this.pipe = pipe; - this.tag = "[reqId=" + reqId + "] "; } /** @@ -50,24 +42,21 @@ public final class SyncRequestBodyPump { * event-loop thread. On EOF signals the pipe normally; on {@link IOException} signals an error and rethrows. */ public void pump() throws IOException { - LOG.info(() -> tag + "pump() entered"); try (InputStream in = contentStreamProvider.newStream()) { while (true) { ByteBuffer chunk = pipe.acquireForFill(); if (chunk == null) { - LOG.info(() -> tag + "pump() exiting due to abort (acquireForFill returned null)"); + // pipe was aborted while we were waiting; stop without signaling EOF. return; } int read; try { read = in.read(chunk.array(), chunk.arrayOffset() + chunk.position(), chunk.remaining()); } catch (IOException ioe) { - LOG.info(() -> tag + "pump() exiting due to error: " + ioe.getMessage()); pipe.signalError(ioe); throw ioe; } if (read < 0) { - LOG.info(() -> tag + "pump() exiting due to eof"); pipe.signalEof(); return; } @@ -76,7 +65,6 @@ public void pump() throws IOException { pipe.publish(chunk); } } catch (InterruptedException ie) { - LOG.info(() -> tag + "pump() exiting due to interrupt"); pipe.abort(); Thread.currentThread().interrupt(); throw new IOException("Interrupted while writing request body", ie); @@ -87,7 +75,6 @@ public void pump() throws IOException { * Abort the underlying pipe (e.g., when the caller's {@code call()} is cancelled). */ public void abort() { - LOG.info(() -> tag + "pump.abort() called"); pipe.abort(); } } diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/LongRunningRequestTestSupport.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/LongRunningRequestTestSupport.java index 21d04a5c9446..723869ad8525 100644 --- a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/LongRunningRequestTestSupport.java +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/LongRunningRequestTestSupport.java @@ -20,21 +20,11 @@ import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo; import com.github.tomakehurst.wiremock.junit5.WireMockExtension; -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.io.UncheckedIOException; -import java.lang.management.ManagementFactory; -import java.lang.management.ThreadInfo; -import java.lang.management.ThreadMXBean; -import java.net.URI; -import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; -import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; -import software.amazon.awssdk.utils.Logger; /** * Shared helpers for the long-running request test suites. @@ -46,8 +36,6 @@ public final class LongRunningRequestTestSupport { public static final Duration TIME_BOUND_SAFETY_MARGIN = Duration.ofSeconds(10); public static final Duration HANG_DELAY = Duration.ofMinutes(1); - private static final Logger log = Logger.loggerFor(LongRunningRequestTestSupport.class); - private LongRunningRequestTestSupport() { } @@ -84,117 +72,22 @@ public static void stubHanging(WireMockExtension mockServer) { .withFixedDelay((int) HANG_DELAY.toMillis()))); } - /** - * Async-executes a POST against {@code mockServer} on a worker thread. A per-call random - * {@code testReqId} is logged BEFORE the request is dispatched and propagated to the SDK via the - * {@code x-aws-sdk-test-id} header so the SDK can prefix its lifecycle logs with the same id. - */ - public static TestRequestExecution executeAsync(SdkHttpClient client, WireMockExtension mockServer) { - String testReqId = "test-" + String.format("%08x", ThreadLocalRandom.current().nextInt()); - log.info(() -> "TEST REQUEST ID: " + testReqId + " (dispatching)"); - CompletableFuture future = CompletableFuture.supplyAsync(() -> { - executeRequest(client, mockServer, testReqId); - return null; - }); - return new TestRequestExecution(testReqId, future); - } - - private static void executeRequest(SdkHttpClient client, WireMockExtension mockServer, String testReqId) { - URI uri = URI.create("http://localhost:" + mockServer.getPort()); - SdkHttpFullRequest request = SdkHttpFullRequest.builder() - .uri(uri) - .method(SdkHttpMethod.POST) - .putHeader("Host", uri.getHost()) - .putHeader("Content-Length", "4") - .putHeader("x-aws-sdk-test-id", testReqId) - .contentStreamProvider(() -> new ByteArrayInputStream( - "Body".getBytes(StandardCharsets.UTF_8))) - .build(); - try { - HttpExecuteResponse response = client.prepareRequest(HttpExecuteRequest.builder() - .request(request) - .contentStreamProvider( - request.contentStreamProvider() - .orElse(null)) - .build()) - .call(); - response.responseBody().ifPresent(body -> { - try { - while (body.read() != -1) { - // drain body so mid-body timeouts surface - } - body.close(); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - }); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } - - public static void assertFailsWithinTimeBound(TestRequestExecution execution, Duration expectedTimeout) { - assertFailsWithinTimeBound(execution.future(), execution.testReqId(), expectedTimeout); - } - public static void assertFailsWithinTimeBound(CompletableFuture future, Duration expectedTimeout) { - assertFailsWithinTimeBound(future, "(unknown)", expectedTimeout); - } - - private static void assertFailsWithinTimeBound(CompletableFuture future, String testReqId, Duration expectedTimeout) { Duration maxWait = expectedTimeout.plus(TIME_BOUND_SAFETY_MARGIN); try { future.get(maxWait.toMillis(), TimeUnit.MILLISECONDS); - throw new AssertionError("Expected request " + testReqId - + " to throw an exception but it completed successfully"); + throw new AssertionError("Expected request to throw an exception but it completed successfully"); } catch (TimeoutException e) { - // Bookend the thread dump with the test reqId so surefire output can be grep'd by request. - log.error(() -> "TEST REQUEST ID: " + testReqId + " (timed out, dumping threads)"); - log.error(() -> dumpAllThreads()); future.cancel(true); throw new AssertionError( - "Expected request " + testReqId + " to fail within " + maxWait - + " but it was still running - client appears to hang", + "Expected request to fail within " + maxWait + " but it was still running - client appears to hang", e); } catch (InterruptedException e) { Thread.currentThread().interrupt(); - throw new AssertionError("Unexpected interruption while waiting for request " + testReqId + " to fail", e); + throw new AssertionError("Unexpected interruption while waiting for request to fail", e); } catch (ExecutionException e) { // expected } } - - /** - * Bundles a worker future with the per-call test reqId so the assertion helper can reference it in - * failure messages. - */ - public static final class TestRequestExecution { - private final String testReqId; - private final CompletableFuture future; - - TestRequestExecution(String testReqId, CompletableFuture future) { - this.testReqId = testReqId; - this.future = future; - } - - public String testReqId() { - return testReqId; - } - - public CompletableFuture future() { - return future; - } - } - - static String dumpAllThreads() { - ThreadMXBean tmx = ManagementFactory.getThreadMXBean(); - ThreadInfo[] infos = tmx.dumpAllThreads(true, true); - StringBuilder sb = new StringBuilder("=== THREAD DUMP ===\n"); - for (ThreadInfo info : infos) { - sb.append(info.toString()).append('\n'); - } - sb.append("=== END THREAD DUMP ===\n"); - return sb.toString(); - } } diff --git a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientLongRunningRequestTestSuite.java b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientLongRunningRequestTestSuite.java index 4403db37a425..0e52eb19eb49 100644 --- a/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientLongRunningRequestTestSuite.java +++ b/test/http-client-tests/src/main/java/software/amazon/awssdk/http/SdkHttpClientLongRunningRequestTestSuite.java @@ -19,15 +19,19 @@ import static software.amazon.awssdk.http.LongRunningRequestTestSupport.CONFIGURED_TIMEOUT; import static software.amazon.awssdk.http.LongRunningRequestTestSupport.HANG_DELAY; import static software.amazon.awssdk.http.LongRunningRequestTestSupport.assertFailsWithinTimeBound; -import static software.amazon.awssdk.http.LongRunningRequestTestSupport.executeAsync; import static software.amazon.awssdk.http.LongRunningRequestTestSupport.stubHanging; import static software.amazon.awssdk.http.LongRunningRequestTestSupport.stubLongPolling; import static software.amazon.awssdk.http.LongRunningRequestTestSupport.stubStreamingWithPauses; import com.github.tomakehurst.wiremock.junit5.WireMockExtension; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.util.concurrent.CompletableFuture; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; -import software.amazon.awssdk.http.LongRunningRequestTestSupport.TestRequestExecution; import software.amazon.awssdk.utils.AttributeMap; /** @@ -52,7 +56,7 @@ public void executeWhenReadTimeoutAndServerDelaysResponseFailsWithinTimeoutBound CONFIGURED_TIMEOUT) .build()); try { - assertFailsWithinTimeBound(executeAsync(client, mockServer), CONFIGURED_TIMEOUT); + assertFailsWithinTimeBound(executeAsync(client), CONFIGURED_TIMEOUT); } finally { client.close(); } @@ -67,7 +71,7 @@ public void executeWhenReadTimeoutAndStreamingResponsePausesFailsWithinTimeoutBo CONFIGURED_TIMEOUT) .build()); try { - assertFailsWithinTimeBound(executeAsync(client, mockServer), CONFIGURED_TIMEOUT); + assertFailsWithinTimeBound(executeAsync(client), CONFIGURED_TIMEOUT); } finally { client.close(); } @@ -85,14 +89,54 @@ public void executeWhenConnectionAcquireTimeoutAndPoolExhaustedFailsWithinTimeou CONFIGURED_TIMEOUT) .build()); try { - TestRequestExecution firstRequest = executeAsync(client, mockServer); + CompletableFuture firstRequest = executeAsync(client); Thread.sleep(500); - assertFailsWithinTimeBound(executeAsync(client, mockServer), CONFIGURED_TIMEOUT); + assertFailsWithinTimeBound(executeAsync(client), CONFIGURED_TIMEOUT); - firstRequest.future().cancel(true); + firstRequest.cancel(true); } finally { client.close(); } } + + private CompletableFuture executeAsync(SdkHttpClient client) { + return CompletableFuture.supplyAsync(() -> { + executeRequest(client); + return null; + }); + } + + private void executeRequest(SdkHttpClient client) { + URI uri = URI.create("http://localhost:" + mockServer.getPort()); + SdkHttpFullRequest request = SdkHttpFullRequest.builder() + .uri(uri) + .method(SdkHttpMethod.POST) + .putHeader("Host", uri.getHost()) + .putHeader("Content-Length", "4") + .contentStreamProvider(() -> new ByteArrayInputStream( + "Body".getBytes(StandardCharsets.UTF_8))) + .build(); + try { + HttpExecuteResponse response = client.prepareRequest(HttpExecuteRequest.builder() + .request(request) + .contentStreamProvider( + request.contentStreamProvider() + .orElse(null)) + .build()) + .call(); + response.responseBody().ifPresent(body -> { + try { + while (body.read() != -1) { + // drain body so mid-body timeouts surface + } + body.close(); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + }); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } } From a55fd780d79bd05834abfe7c6fdd9f2957114df2 Mon Sep 17 00:00:00 2001 From: Zoe Wang <33073555+zoewangg@users.noreply.github.com> Date: Sun, 14 Jun 2026 19:42:30 -0700 Subject: [PATCH 12/12] Add more tests --- .../awssdk/http/crt/AwsCrtHttpClient.java | 2 + .../http/crt/internal/CrtRequestExecutor.java | 14 +- .../crt/AwsCrtHttpClientWireMockTest.java | 128 ++++++++++++++++++ 3 files changed, 136 insertions(+), 8 deletions(-) diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java index 10574489a190..937e4d5dd1a2 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/AwsCrtHttpClient.java @@ -206,6 +206,8 @@ private boolean waitForStreamAcquired(CompletableFuture streamFu if (streamFuture == null) { return false; } + // The eventual exception is propagated by the executor's streamFuture.whenComplete via + // requestFuture.completeExceptionally; here we only decide whether to run the body pump. try { streamFuture.get(timeoutMillis, TimeUnit.MILLISECONDS); return true; diff --git a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java index e69225083357..f47b59e7a868 100644 --- a/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java +++ b/http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/CrtRequestExecutor.java @@ -57,16 +57,14 @@ public Result execute(CrtRequestContext executionContext) { long finalAcquireStartTime = acquireStartTime; streamFuture.whenComplete((streamBase, throwable) -> { - // Only notify the response handler when stream acquisition succeeded; passing a null - // streamBase from a failed acquisition would NPE inside the handler. - if (throwable == null) { - crtResponseHandler.onAcquireStream(streamBase); - } - if (shouldPublishMetrics) { - reportMetrics(executionContext.streamManager(), metricCollector, finalAcquireStartTime); - } if (throwable != null) { requestFuture.completeExceptionally(wrapCrtException(throwable)); + + } else { + crtResponseHandler.onAcquireStream(streamBase); + if (shouldPublishMetrics) { + reportMetrics(executionContext.streamManager(), metricCollector, finalAcquireStartTime); + } } }); diff --git a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/AwsCrtHttpClientWireMockTest.java b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/AwsCrtHttpClientWireMockTest.java index 5171451a4f04..aad5103b287c 100644 --- a/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/AwsCrtHttpClientWireMockTest.java +++ b/http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/AwsCrtHttpClientWireMockTest.java @@ -40,6 +40,7 @@ import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.concurrent.Callable; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; @@ -321,6 +322,133 @@ public void serverResetsConnection_connectionReturnedToPool_subsequentRequestSuc } } + @Test + public void interruptDuringCall_connectionReturnedToPool_subsequentRequestSucceeds() throws Exception { + try (SdkHttpClient client = AwsCrtHttpClient.builder() + .maxConcurrency(1) + .connectionAcquisitionTimeout(Duration.ofSeconds(10)) + .build()) { + URI uri = URI.create("http://localhost:" + mockServer.port()); + stubFor(any(urlPathEqualTo("/")).willReturn(aResponse().withFixedDelay(2000).withBody("hello"))); + stubFor(put(urlPathEqualTo("/sink")).willReturn(aResponse().withStatus(200))); + + SdkHttpRequest delayedRequest = createRequest(uri); + HttpExecuteRequest delayedExecute = HttpExecuteRequest.builder() + .request(delayedRequest) + .contentStreamProvider(() -> new ByteArrayInputStream(new byte[0])) + .build(); + + CountDownLatch workerDone = new CountDownLatch(1); + AtomicReference workerError = new AtomicReference<>(); + ExecutorService worker = Executors.newSingleThreadExecutor(); + try { + Future inFlight = worker.submit(() -> { + try { + client.prepareRequest(delayedExecute).call(); + } catch (Throwable t) { + workerError.set(t); + } finally { + workerDone.countDown(); + } + }); + + // Give call() time to enter joinInterruptibly() before we interrupt. + Thread.sleep(100); + inFlight.cancel(true); + + assertThat(workerDone.await(10, TimeUnit.SECONDS)).isTrue(); + assertThat(workerError.get()) + .isInstanceOf(IOException.class) + .hasMessageContaining("cancelled"); + } finally { + worker.shutdownNow(); + worker.awaitTermination(5, TimeUnit.SECONDS); + } + + // If the interrupt leaked the connection, this second call() would block on acquire and fail + // when connectionAcquisitionTimeout (10s above) elapses. + String body = "after-interrupt"; + byte[] bodyBytes = body.getBytes(StandardCharsets.UTF_8); + SdkHttpFullRequest okRequest = SdkHttpFullRequest.builder() + .uri(uri) + .method(SdkHttpMethod.PUT) + .encodedPath("/sink") + .putHeader("Host", uri.getHost()) + .putHeader("Content-Length", Integer.toString(bodyBytes.length)) + .build(); + HttpExecuteRequest okExecute = HttpExecuteRequest.builder() + .request(okRequest) + .contentStreamProvider(() -> new ByteArrayInputStream(bodyBytes)) + .build(); + HttpExecuteResponse response = client.prepareRequest(okExecute).call(); + assertThat(response.httpResponse().statusCode()).isEqualTo(200); + } + } + + @Test + public void acquireTimeoutThenHolderCancelled_connectionReturnedToPool_subsequentRequestSucceeds() throws Exception { + try (SdkHttpClient client = AwsCrtHttpClient.builder() + .maxConcurrency(1) + .connectionAcquisitionTimeout(Duration.ofSeconds(2)) + .build()) { + URI uri = URI.create("http://localhost:" + mockServer.port()); + stubFor(any(urlPathEqualTo("/")).willReturn(aResponse().withFixedDelay(60_000).withBody("hello"))); + + SdkHttpRequest holderRequest = createRequest(uri); + HttpExecuteRequest holderExecute = HttpExecuteRequest.builder() + .request(holderRequest) + .contentStreamProvider(() -> new ByteArrayInputStream(new byte[0])) + .build(); + ExecutableHttpRequest holder = client.prepareRequest(holderExecute); + + SdkHttpRequest racerRequest = createRequest(uri); + HttpExecuteRequest racerExecute = HttpExecuteRequest.builder() + .request(racerRequest) + .contentStreamProvider(() -> new ByteArrayInputStream(new byte[0])) + .build(); + + ExecutorService pool = Executors.newFixedThreadPool(2); + try { + Future holderFuture = pool.submit(holder::call); + // Give the holder time to acquire the only slot before the racer tries. + Thread.sleep(500); + + Future racerFuture = pool.submit(() -> client.prepareRequest(racerExecute).call()); + // CRT surfaces the acquire-timeout as HttpException; CrtHttpRequest.call() rethrows + // it directly (does not wrap in IOException). + assertThatThrownBy(() -> racerFuture.get(5, TimeUnit.SECONDS)) + .hasMessageContaining("acquire"); + + // Release the slot via the same closeConnection path the other leak tests exercise. + holder.abort(); + assertThatThrownBy(() -> holderFuture.get(5, TimeUnit.SECONDS)) + .hasCauseInstanceOf(IOException.class); + } finally { + pool.shutdownNow(); + pool.awaitTermination(5, TimeUnit.SECONDS); + } + + // If the slot didn't reclaim, this third call() blocks on acquire and fails when the + // 2s connectionAcquisitionTimeout above elapses. + stubFor(put(urlPathEqualTo("/sink")).willReturn(aResponse().withStatus(200))); + byte[] okBytes = "ok".getBytes(StandardCharsets.UTF_8); + SdkHttpFullRequest okRequest = SdkHttpFullRequest.builder() + .uri(uri) + .method(SdkHttpMethod.PUT) + .encodedPath("/sink") + .putHeader("Host", uri.getHost()) + .putHeader("Content-Length", Integer.toString(okBytes.length)) + .build(); + HttpExecuteRequest okExecute = HttpExecuteRequest.builder() + .request(okRequest) + .contentStreamProvider(() -> new ByteArrayInputStream(okBytes)) + .build(); + + HttpExecuteResponse response = client.prepareRequest(okExecute).call(); + assertThat(response.httpResponse().statusCode()).isEqualTo(200); + } + } + /** * Regression test for the deadlock the pull-pump fix addresses. On master, the request body's * {@code InputStream.read(...)} ran on the CRT event-loop thread (via the body callback), which