From 111337f1ef7d3bc3e8589496896a508a4d514442 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Wed, 8 Jan 2025 14:07:34 +0100 Subject: [PATCH] fix BlockingContentProducer staying blocked on semaphore when an error occurs Signed-off-by: Ludovic Orban --- .../eclipse/jetty/ee10/servlet/AsyncContentProducer.java | 6 ++++++ .../eclipse/jetty/ee10/servlet/BlockingContentProducer.java | 5 ++++- .../java/org/eclipse/jetty/ee10/servlet/BlockingTest.java | 2 +- .../eclipse/jetty/ee11/servlet/AsyncContentProducer.java | 6 ++++++ .../eclipse/jetty/ee11/servlet/BlockingContentProducer.java | 5 ++++- .../java/org/eclipse/jetty/ee11/servlet/BlockingTest.java | 2 +- 6 files changed, 22 insertions(+), 4 deletions(-) diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContentProducer.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContentProducer.java index beaf5440a134..113824b079e4 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContentProducer.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContentProducer.java @@ -395,6 +395,12 @@ void release() _condition.signal(); } + void fail() + { + _permits = Integer.MAX_VALUE; + _condition.signal(); + } + @Override public String toString() { diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/BlockingContentProducer.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/BlockingContentProducer.java index e0ea0e34e328..8865c2ba4d35 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/BlockingContentProducer.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/BlockingContentProducer.java @@ -153,8 +153,9 @@ public boolean onContentProducible() // Calling _asyncContentProducer.onContentProducible() changes the channel state from WAITING to WOKEN which // would prevent the async error thread from noticing that a redispatching is needed. boolean unready = _asyncContentProducer.isUnready(); + boolean error = _asyncContentProducer.isError(); if (LOG.isDebugEnabled()) - LOG.debug("onContentProducible releasing semaphore {} unready={}", _semaphore, unready); + LOG.debug("onContentProducible releasing semaphore {} unready={} error={}", _semaphore, unready, error); // Do not release the semaphore if we are not unready, as certain protocols may call this method // just after having received the request, not only when they have read all the available content. if (unready) @@ -163,6 +164,8 @@ public boolean onContentProducible() _asyncContentProducer.getServletChannel().getServletRequestState().onReadIdle(); _semaphore.release(); } + if (error) + _semaphore.fail(); return false; } } diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/BlockingTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/BlockingTest.java index 0a35130b4e1b..bcbae1731d85 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/BlockingTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/BlockingTest.java @@ -161,7 +161,7 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) throws StackTraceElement[] stackTrace = threadRef.get().getStackTrace(); for (StackTraceElement stackTraceElement : stackTrace) { - System.out.println(stackTraceElement); + System.err.println(stackTraceElement); } } assertTrue(await); diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/AsyncContentProducer.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/AsyncContentProducer.java index 84fb051a6a09..c38cd632fda7 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/AsyncContentProducer.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/AsyncContentProducer.java @@ -395,6 +395,12 @@ void release() _condition.signal(); } + void fail() + { + _permits = Integer.MAX_VALUE; + _condition.signal(); + } + @Override public String toString() { diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/BlockingContentProducer.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/BlockingContentProducer.java index 073869abc0f1..1e4fb3d14e5e 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/BlockingContentProducer.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/BlockingContentProducer.java @@ -153,8 +153,9 @@ public boolean onContentProducible() // Calling _asyncContentProducer.onContentProducible() changes the channel state from WAITING to WOKEN which // would prevent the async error thread from noticing that a redispatching is needed. boolean unready = _asyncContentProducer.isUnready(); + boolean error = _asyncContentProducer.isError(); if (LOG.isDebugEnabled()) - LOG.debug("onContentProducible releasing semaphore {} unready={}", _semaphore, unready); + LOG.debug("onContentProducible releasing semaphore {} unready={} error={}", _semaphore, unready, error); // Do not release the semaphore if we are not unready, as certain protocols may call this method // just after having received the request, not only when they have read all the available content. if (unready) @@ -163,6 +164,8 @@ public boolean onContentProducible() _asyncContentProducer.getServletChannel().getServletRequestState().onReadIdle(); _semaphore.release(); } + if (error) + _semaphore.fail(); return false; } } diff --git a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/BlockingTest.java b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/BlockingTest.java index 4a64b5c1a9ff..5a29ab50a702 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/BlockingTest.java +++ b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/BlockingTest.java @@ -161,7 +161,7 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) throws StackTraceElement[] stackTrace = threadRef.get().getStackTrace(); for (StackTraceElement stackTraceElement : stackTrace) { - System.out.println(stackTraceElement); + System.err.println(stackTraceElement); } } assertTrue(await);