Skip to content

Commit 43ecc9f

Browse files
committed
Handle Junit failures
1 parent 333c026 commit 43ecc9f

4 files changed

Lines changed: 26 additions & 3 deletions

File tree

http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/response/CrtResponseAdapter.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ public int onResponseBody(HttpStreamBase stream, byte[] bodyBytesIn) {
107107

108108
@Override
109109
public void onResponseComplete(HttpStreamBase stream, int errorCode) {
110+
responseHandlerHelper.onResponseComplete();
110111
if (errorCode == CRT.AWS_CRT_SUCCESS) {
111112
onSuccessfulResponseComplete();
112113
} else {

http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/response/InputStreamAdaptingHttpStreamResponseHandler.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ public int onResponseBody(HttpStreamBase stream, byte[] bodyBytesIn) {
109109

110110
@Override
111111
public void onResponseComplete(HttpStreamBase stream, int errorCode) {
112+
responseHandlerHelper.onResponseComplete();
112113
if (errorCode == CRT.AWS_CRT_SUCCESS) {
113114
onSuccessfulResponseComplete();
114115
} else {

http-clients/aws-crt-client/src/main/java/software/amazon/awssdk/http/crt/internal/response/ResponseHandlerHelper.java

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ public class ResponseHandlerHelper {
3232
private final SdkHttpResponse.Builder responseBuilder;
3333
private HttpStreamBase stream;
3434
private boolean streamClosed;
35+
private boolean streamCompleted;
3536
private final Object streamLock = new Object();
3637

3738
public ResponseHandlerHelper(SdkHttpResponse.Builder responseBuilder) {
@@ -81,16 +82,33 @@ public void releaseConnection() {
8182
}
8283
}
8384

85+
/**
86+
* Called when CRT fires onResponseComplete. After this, {@link #closeConnection()} skips
87+
* {@code cancel()} to avoid a use-after-free in the native layer: on the GOAWAY path,
88+
* {@code thread_data.state} is {@code HALF_CLOSED_LOCAL} (not {@code CLOSED}), so the
89+
* cancel cross-thread task's state check passes and dereferences a freed connection → SIGSEGV.
90+
*/
91+
public void onResponseComplete() {
92+
synchronized (streamLock) {
93+
streamCompleted = true;
94+
}
95+
}
96+
8497
/**
8598
* Cancel and close the stream, forcing the underlying connection to shut down rather than be returned to the
8699
* connection pool. This should be called on error paths or when the stream is aborted before the response is
87100
* fully consumed. {@code cancel()} must be invoked before {@code close()} per the CRT contract.
101+
* <p>
102+
* If CRT has already completed the stream via {@link #onResponseComplete()}, {@code cancel()} is skipped
103+
* to avoid a native use-after-free, but {@code close()} is still called to release the Java-side handle.
88104
*/
89105
public void closeConnection() {
90106
synchronized (streamLock) {
91107
if (!streamClosed && stream != null) {
92108
streamClosed = true;
93-
stream.cancel();
109+
if (!streamCompleted) {
110+
stream.cancel();
111+
}
94112
stream.close();
95113
}
96114
}

http-clients/aws-crt-client/src/test/java/software/amazon/awssdk/http/crt/internal/BaseHttpStreamResponseHandlerTest.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,15 +91,18 @@ void nonServerError_shouldCloseStream(int statusCode) {
9191
}
9292

9393
@Test
94-
void failedToGetResponse_shouldCancelAndCloseStream() {
94+
void failedToGetResponse_shouldCloseStreamWithoutCancel() {
9595
HttpHeader[] httpHeaders = getHttpHeaders();
9696
responseHandler.onResponseHeaders(httpStream, 200, HttpHeaderBlock.MAIN.getValue(),
9797
httpHeaders);
9898

9999
responseHandler.onResponseComplete(httpStream, 1);
100100
assertThatThrownBy(() -> requestFuture.join()).hasRootCauseInstanceOf(HttpException.class);
101101
InOrder inOrder = Mockito.inOrder(httpStream);
102-
inOrder.verify(httpStream).cancel();
102+
// cancel() is skipped when CRT has already completed the stream (streamCompleted=true).
103+
// Per HttpStreamBase.cancel() javadoc: "if the stream is already completing, this call will have no effect."
104+
// For HTTP/2, calling cancel() on an already-completed stream triggers premature native resource cleanup.
105+
verify(httpStream, never()).cancel();
103106
inOrder.verify(httpStream).close();
104107
}
105108

0 commit comments

Comments
 (0)