I figured it out. It's a bug in the actual code, not the test. The code
checks for response data on non-SSL connections by calling
`inputStream.available()`, as originally suggested here:

https://www.mail-archive.com/[email protected]/msg09913.html

Unfortunately, `InputStream::available` only returns an *estimate*; the
contract for the method is basically that an attempt to read or skip
`available()` bytes will not trigger blocking behavior. The inverse is not
true: `available() == 0` does not imply that there is no data to be read,
and in fact `return 0` is a legal and common implementation of
`available()`.

Apparently, on some Java versions, on some operating systems, on some test
runs, the server's response never shows up as available() bytes. This
behavior is non-deterministic, but easily reproduced with `@RepeatedTest`:
I see `available()` repeatedly returning 0, but a subsequent call to
`connection.isDataAvailable(Timeout.ONE_MILLISECOND)` shows that data is in
fact available to be read. (This is very similar to the problem of
implementing validateAfterInactivity in the classic client, but complicated
by some very inconvenient non-determinism.) I went to considerable lengths
to confirm that this is not just a race condition in our code; it's some
sort of quirk of Java sockets, and/or the underlying OS.

Since this is an opt-in feature for users who presumably are willing to
take the documented performance hit in exchange for improved correctness,
I'm going to change the code to always perform a blocking read in order to
check for response data.

On Sat, Dec 27, 2025 at 2:13 PM Ryan Schmitt <[email protected]> wrote:

> I'm already looking at this. Something very strange is happening; I have a
> TCP stream open in Wireshark right now showing that the server sent back a
> 400, but the client never stopped writing its request entity. Either the
> client isn't seeing the server's response somehow (I can see from logging
> breakpoints that `inputStream.available()` just keeps returning 0), or the
> `400 Bad Request` is not triggering a `ResponseOutOfOrderException`, or the
> exception is somehow getting swallowed.
>
> On Sat, Dec 27, 2025 at 11:39 AM Oleg Kalnichevski <[email protected]>
> wrote:
>
>> Hi Ryan
>>
>> The test still fails in some CI environments.
>>
>>
>> https://github.com/apache/httpcomponents-core/actions/runs/20535102490/job/58992266978
>>
>> Any theory as to why it has started failing out of the sudden? The test
>> was contributed in 2020 and I do not remember it causing any trouble.
>>
>> Oleg
>>
>>
>> On 12/27/2025 06:48, [email protected] wrote:
>> > This is an automated email from the ASF dual-hosted git repository.
>> >
>> > rschmitt pushed a commit to branch master
>> > in repository
>> https://gitbox.apache.org/repos/asf/httpcomponents-core.git
>> >
>> >
>> > The following commit(s) were added to refs/heads/master by this push:
>> >       new 9d954702b
>> MonitoringResponseOutOfOrderStrategyIntegrationTest: Fix deadlock
>> > 9d954702b is described below
>> >
>> > commit 9d954702b2527dc0eac5adf8ca9284b0150bb21e
>> > Author: Ryan Schmitt <[email protected]>
>> > AuthorDate: Sat Dec 27 00:16:39 2025 -0500
>> >
>> >      MonitoringResponseOutOfOrderStrategyIntegrationTest: Fix deadlock
>> >
>> >      This test can deadlock: both the client and the server can fill up
>> their
>> >      respective output buffers and block indefinitely while
>> simultaneously
>> >      trying to write to each other.
>> >
>> >      It's clear why the client in this test sends a large request
>> entity, but
>> >      I'm not able to identify a reason why the test server is also
>> sending a
>> >      large response entity. To prevent the deadlock, I've changed the
>> test
>> >      server to send a small response instead, one that will trivially
>> fit in
>> >      the output buffer.
>> >
>> >      I've also added an assertion to verify that the client's request
>> was
>> >      truncated upon receipt of the server's response, since this seems
>> to be
>> >      the actual functionality we are testing. This new assertion fails
>> if
>> >      `MonitoringResponseOutOfOrderStrategy` is not provided.
>> > ---
>> >   ...ringResponseOutOfOrderStrategyIntegrationTest.java | 19
>> ++++++++-----------
>> >   1 file changed, 8 insertions(+), 11 deletions(-)
>> >
>> > diff --git
>> a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/classic/MonitoringResponseOutOfOrderStrategyIntegrationTest.java
>> b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/classic/MonitoringResponseOutOfOrderStrategyIntegrationTest.java
>> > index 81ff0e804..dddd39696 100644
>> > ---
>> a/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/classic/MonitoringResponseOutOfOrderStrategyIntegrationTest.java
>> > +++
>> b/httpcore5-testing/src/test/java/org/apache/hc/core5/testing/classic/MonitoringResponseOutOfOrderStrategyIntegrationTest.java
>> > @@ -47,6 +47,7 @@
>> >   import org.apache.hc.core5.http.io.SocketConfig;
>> >   import org.apache.hc.core5.http.io.entity.AbstractHttpEntity;
>> >   import org.apache.hc.core5.http.io.entity.EntityUtils;
>> > +import org.apache.hc.core5.http.io.entity.StringEntity;
>> >   import org.apache.hc.core5.http.message.BasicClassicHttpRequest;
>> >   import org.apache.hc.core5.http.protocol.HttpCoreContext;
>> >   import org.apache.hc.core5.testing.SSLTestContexts;
>> > @@ -55,12 +56,10 @@
>> >   import org.apache.hc.core5.util.Timeout;
>> >   import org.junit.jupiter.api.Assertions;
>> >   import org.junit.jupiter.api.Test;
>> > +import org.junit.jupiter.api.Timeout.ThreadMode;
>> >   import org.junit.jupiter.api.extension.RegisterExtension;
>> >
>> >   abstract class MonitoringResponseOutOfOrderStrategyIntegrationTest {
>> > -
>> > -    // Use a 16k buffer for consistent results across systems
>> > -    private static final int BUFFER_SIZE = 16 * 1024;
>> >       private static final Timeout TIMEOUT = Timeout.ofSeconds(3);
>> >
>> >       private final URIScheme scheme;
>> > @@ -78,14 +77,12 @@ public
>> MonitoringResponseOutOfOrderStrategyIntegrationTest(final URIScheme schem
>> >                   .setSslContext(scheme == URIScheme.HTTPS ?
>> SSLTestContexts.createServerSSLContext() : null)
>> >                   .setSocketConfig(SocketConfig.custom()
>> >                           .setSoTimeout(TIMEOUT)
>> > -                        .setSndBufSize(BUFFER_SIZE)
>> > -                        .setRcvBufSize(BUFFER_SIZE)
>> >                           .setSoKeepAlive(false)
>> >                           .build())
>> >
>>  .setRequestRouter(RequestRouter.<HttpRequestHandler>builder()
>> >                           .addRoute(RequestRouter.LOCAL_AUTHORITY, "*",
>> (request, response, context) -> {
>> >                               response.setCode(400);
>> > -                            response.setEntity(new
>> AllOnesHttpEntity(200000));
>> > +                            response.setEntity(new
>> StringEntity("stop"));
>> >                           })
>> >
>>  .resolveAuthority(RequestRouter.LOCAL_AUTHORITY_RESOLVER)
>> >                           .build()));
>> > @@ -95,8 +92,6 @@ public
>> MonitoringResponseOutOfOrderStrategyIntegrationTest(final URIScheme schem
>> >
>>  .setSslContext(SSLTestContexts.createClientSSLContext())
>> >                   .setSocketConfig(SocketConfig.custom()
>> >                           .setSoTimeout(TIMEOUT)
>> > -                        .setRcvBufSize(BUFFER_SIZE)
>> > -                        .setSndBufSize(BUFFER_SIZE)
>> >                           .setSoKeepAlive(false)
>> >                           .build())
>> >
>>  .setConnectionFactory(DefaultBHttpClientConnectionFactory.builder()
>> > @@ -105,7 +100,7 @@ public
>> MonitoringResponseOutOfOrderStrategyIntegrationTest(final URIScheme schem
>> >       }
>> >
>> >       @Test
>> > -    @org.junit.jupiter.api.Timeout(value = 1, unit =
>> TimeUnit.MINUTES)// Failures may hang
>> > +    @org.junit.jupiter.api.Timeout(value = 1, unit = TimeUnit.MINUTES,
>> threadMode = ThreadMode.SEPARATE_THREAD)
>> >       void testResponseOutOfOrderWithDefaultStrategy() throws Exception
>> {
>> >           final HttpServer server = serverResource.start();
>> >           final HttpRequester requester = clientResource.start();
>> > @@ -114,16 +109,18 @@ void testResponseOutOfOrderWithDefaultStrategy()
>> throws Exception {
>> >           final HttpHost host = new HttpHost(scheme.id, "localhost",
>> server.getLocalPort());
>> >
>> >           final ClassicHttpRequest post = new
>> BasicClassicHttpRequest(Method.POST, "/");
>> > -        post.setEntity(new AllOnesHttpEntity(200000));
>> > +        final AllOnesHttpEntity requestEntity = new
>> AllOnesHttpEntity(20 * 1024 * 1024);
>> > +        post.setEntity(requestEntity);
>> >
>> >           try (final ClassicHttpResponse response =
>> requester.execute(host, post, TIMEOUT, context)) {
>> >               Assertions.assertEquals(400, response.getCode());
>> >               EntityUtils.consumeQuietly(response.getEntity());
>> >           }
>> > +        Assertions.assertTrue(requestEntity.remaining > 0, "Client
>> should have stopped sending data");
>> >       }
>> >
>> >       private static final class AllOnesHttpEntity extends
>> AbstractHttpEntity {
>> > -        private long remaining;
>> > +        long remaining;
>> >
>> >           protected AllOnesHttpEntity(final long length) {
>> >               super(ContentType.APPLICATION_OCTET_STREAM, null, true);
>> >
>>
>>

Reply via email to