On Fri, 19 Dec 2025 12:11:11 GMT, EunHyunsu <[email protected]> wrote:

>> ### Problem
>> 
>>   When the HTTP/2 client receives a GOAWAY frame with a non-zero error code, 
>> the current implementation discards both the error code and debug data. 
>> Users only see generic "Connection closed by peer" errors without any 
>> information about why the server terminated the connection.
>> 
>>   ### Solution
>> 
>>   Per [RFC 9113 
>> §5.4.1](https://www.rfc-editor.org/rfc/rfc9113.html#section-5.4.1), a GOAWAY 
>> frame with a non-zero error code indicates a connection error requiring 
>> immediate closure. This fix:
>> 
>>   1. **Distinguishes** graceful shutdown (NO_ERROR) from connection errors
>>   2. **Preserves** error code and debug data in exception messages
>>   3. **Categorizes** streams based on `lastStreamId`:
>>      - Streams with ID > `lastStreamId`: Marked as unprocessed for automatic 
>> retry
>>      - Streams with ID ≤ `lastStreamId`: Failed with detailed error 
>> information
>> 
>>   ### Changes
>> 
>>   **Core Implementation** (`Http2Connection.java`):
>>   - Modified `handleGoAway()` to check error code and route appropriately
>>   - Added `handleGoAwayWithError()` method that:
>>     - Extracts error code and debug data from GOAWAY frame
>>     - Creates meaningful error messages with error name, hex code, and debug 
>> data
>>     - Properly categorizes streams for retry or failure
>> 
>>   **Test Infrastructure**:
>>   - `Http2TestServerConnection.sendGoAway(int, int, byte[])`: Supports 
>> custom error codes
>>   - `Http2TestExchangeImpl.getServerConnection()`: Accessor for test handlers
>>   - `GoAwayWithErrorTest`: Verifies proper error propagation
>> 
>>   ### Example
>> 
>>   **Before:**
>>   IOException: Connection closed by peer
>> 
>>   **After:**
>>   IOException: Received GOAWAY with error code Protocol error (0x1): Invalid 
>> HEADERS frame
>> 
>>   ### Testing
>> 
>>   - New `GoAwayWithErrorTest` passes
>>   - Existing HTTP/2 tests unaffected (NO_ERROR path unchanged)
>>   - Backward compatible (no public API changes)
>
> EunHyunsu has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8371903: Use stateLock to synchronize stream iteration and connection 
> closure

Well with af092399cd5d188fdec4061d33528f6dc0f3a7f0 the `GoAwayWithErrorTest` 
deadlocks sometimes, with the following stack traces:
Thread 1:

              
"java.base/java.util.concurrent.locks.ReentrantLock.lock(ReentrantLock.java:323)",
              
"java.net.http/jdk.internal.net.http.Http2Connection.tryReserveForPoolCheckout(Http2Connection.java:1604)",
              
"java.net.http/jdk.internal.net.http.Http2ClientImpl.getConnectionFor(Http2ClientImpl.java:109)",
              
"java.net.http/jdk.internal.net.http.ExchangeImpl.attemptHttp2Exchange(ExchangeImpl.java:143)",
              
"java.net.http/jdk.internal.net.http.ExchangeImpl.get(ExchangeImpl.java:122)",
              
"java.net.http/jdk.internal.net.http.Exchange.establishExchange(Exchange.java:409)",
              
"java.net.http/jdk.internal.net.http.Exchange.responseAsyncImpl(Exchange.java:624)",
              
"java.net.http/jdk.internal.net.http.Exchange.responseAsync(Exchange.java:447)",
              
"java.net.http/jdk.internal.net.http.MultiExchange.responseAsyncImpl(MultiExchange.java:483)",
              
"java.net.http/jdk.internal.net.http.MultiExchange.lambda$responseAsync0$0(MultiExchange.java:402)",

Thread 2:

              
"java.base/java.util.concurrent.locks.ReentrantLock.lock(ReentrantLock.java:323)",
              
"java.net.http/jdk.internal.net.http.Http2ClientImpl.removeFromPool(Http2ClientImpl.java:225)",
              
"java.net.http/jdk.internal.net.http.Http2Connection$Terminator.doTerminate(Http2Connection.java:2135)",
              
"java.net.http/jdk.internal.net.http.Http2Connection$Terminator.terminate(Http2Connection.java:2099)",
              
"java.net.http/jdk.internal.net.http.Http2Connection.close(Http2Connection.java:899)",
              
"java.net.http/jdk.internal.net.http.Http2Connection.handleGoAwayWithError(Http2Connection.java:1481)",
              
"java.net.http/jdk.internal.net.http.Http2Connection.handleGoAway(Http2Connection.java:1411)",

connectionPoolLock and stateLock are acquired in a different order, leading to 
the deadlock.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/28632#issuecomment-3718222666

Reply via email to