Issue with Response OutputStream#close() rethrowing same EofException instance #11736
Assignees
Labels
Bug
For general bugs on Jetty side
Comments
|
This is the output of running the above test case: |
|
This seems to overlap with another issue that is currently being worked on. |
|
I've reviewed that issue, I'm not sure I see an overlap other than they both relate to EOFException conditions, I think this may be a distinct scenario. |
|
While the initial description of the issue isn't the same, the root cause, and the fix, seem to be the same. See PR |
|
I cloned package org.eclipse.jetty.server;
import org.eclipse.jetty.io.Content;
import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.util.Callback;
import org.junit.jupiter.api.Test;
import java.io.IOException;
import java.net.http.HttpClient;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
import java.nio.charset.StandardCharsets;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
public class EofDuringResponseWriteTest {
private static void sleep(final int seconds) {
try {
TimeUnit.SECONDS.sleep(seconds);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
}
@Test
public void testClientAbortsRequest() throws Exception {
final var server = new Server();
final var connector = new ServerConnector(server, 1, 1);
server.addConnector(connector);
final var context = new ContextHandler("/");
final var clientResponse = new AtomicReference<CompletableFuture<HttpResponse<String>>>();
context.setHandler(
new Handler.Abstract() {
@Override
public boolean handle(Request request, Response response, Callback callback) {
try (final var body = Content.Sink.asOutputStream(response)) {
// simulate the client hanging up
if (clientResponse.get() != null) {
clientResponse.get().cancel(true);
}
sleep(2);
// Raises EofException because client hung up
body.write("Hello World".getBytes(StandardCharsets.UTF_8));
callback.succeeded();
} catch (IOException e) {
// Does NOT reach here, because the OutputStream#close()
// rethrows same EofException instance, which causes
// try with resources block call to: Throwable#addSuppresed() to
// fail with java.lang.IllegalArgumentException: Self-suppression not
// permitted
callback.failed(e);
}
return true;
}
});
server.setHandler(context);
server.start();
clientResponse.set(
HttpClient.newHttpClient()
.sendAsync(
HttpRequest.newBuilder().GET().uri(server.getURI()).build(),
HttpResponse.BodyHandlers.ofString()));
sleep(2);
server.stop();
}
}This is the output of running the test: In other words the issue reported above continues to occur, even with the changes in |
|
I think the core issue is that I think |
|
I don't think I agree with your statement that But it seems that you have found a real issue with our implementation of |
lorban
added a commit
that referenced
this issue
May 7, 2024
Signed-off-by: Ludovic Orban <[email protected]>
lorban
added a commit
that referenced
this issue
May 7, 2024
Signed-off-by: Ludovic Orban <[email protected]>
lorban
added a commit
that referenced
this issue
May 13, 2024
Signed-off-by: Ludovic Orban <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Jetty version(s)
Jetty 12.0.7
Jetty Environment
core
Java version/vendor
(use: java -version)java version "21.0.1" 2023-10-17 LTS
Java(TM) SE Runtime Environment (build 21.0.1+12-LTS-29)
Java HotSpot(TM) 64-Bit Server VM (build 21.0.1+12-LTS-29, mixed mode, sharing)
OS type/version
ProductName: macOS
ProductVersion: 14.4.1
BuildVersion: 23E224
Description
In the case where a client hangs up before a Jetty Handler has written a response, an
EofExceptionis raised when attempting to write to the response output stream.This is expected.
It is best practice to acquire and release the response output stream using with the try-with-resources idiom:
Note that the try with resources block is translated into a regular try block per the Java Language Specification, so when the block completes the
OutputStream#close()method will be invoked.The generated try block has the following form:
{ final {VariableModifierNoFinal} R Identifier = Expression; Throwable #primaryExc = null; try ResourceSpecification_tail Block catch (Throwable #t) { #primaryExc = #t; throw #t; } finally { if (Identifier != null) { if (#primaryExc != null) { try { Identifier.close(); } catch (Throwable #suppressedExc) { #primaryExc.addSuppressed(#suppressedExc); } } else { Identifier.close(); } } } }#primaryExc.addSuppressed(#suppressedExc);The logic of
Throwable#addSuppressedis:thisthen raise anIlegalArgumentException(to prevent an exception referring to itself as suppressed)In Jetty 12 (to the best of my recollection this was not the case in earlier Jetty versions, but have not confirmed this), the
OutputStream#close()call rethrows the exact same instance ofEofExceptionas was raised when the call toOutputStream#write()was invoked.This causes the generated try block to end up raising an
IllegalArgumentExceptiondue toThrowable#addSuppressed()refusing to add itself as a suppressed exception.Whether intentional or not, the generated try block excludes the possibility that the
close()method may throw the exact same exception instance which originally caused the try with resources block to be exited.I cannot think of a good reason for the
close()method to rethrow the same exception instance. I think a failure in theclose()method is semantically different to the failure in thewrite()method and should result in a different exception instance (if any) being thrown.In this specific case, where the client has hung-up, I think the appropriate behaviour of
close()is to complete without raising any error.How to reproduce?
See the test case below:
The text was updated successfully, but these errors were encountered: