RFR: 8284585: PushPromiseContinuation test fails intermittently in timeout
Conor Cleary
ccleary at openjdk.java.net
Wed May 4 13:35:21 UTC 2022
On Wed, 4 May 2022 11:13:26 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:
>> **Issue**
>> A recent fix for [JDK-8263031](https://bugs.openjdk.java.net/browse/JDK-8263031), which addressed the httpclient being unable to properly process Http/2 PushPromise frames followed by continuations caused intermittent failures of the test. This was cause by two seperate problems.
>>
>> - Firstly, on the client side, `Http2Connection.processFrame` did not a check for whether or not a Continuation was to be expected early on in the method resulting in frames other than the expected Continuation Frame being incorrectly processed. In Http/2, when a Continuation Frame is expected, no other type of frame should be processed without a connection error of type PROTOCOL_ERROR being thrown.
>> - Secondly, `Http2TestConnection.handlePush` had no guards around response headers and data frames being sent before a continuation is sent which resulted in the intermittent nature of this timeout.
>>
>> **Proposed Solution**
>> The test class `OutgoingPushPromise` was modified to contain a list of Continuation Frames as required rather than the Continuations being scheduled to send in the test itself. This prevents the situation of unexpected frames being sent before a Continuation Frame was meant to arrive.
>>
>> In addition to the above, Http2Connection was modified to ensure that a connection error of type PROTOCOL_ERROR is thrown if a frame other than a Continuation arrives at the client unexpectedly.
>
> test/jdk/java/net/httpclient/http2/server/Http2TestServerConnection.java line 949:
>
>> 947: pp.streamid(op.parentStream);
>> 948: writeFrame(pp);
>> 949: if (pp.getFlags() != HeadersFrame.END_HEADERS && op.hasContinuations()) {
>
> Maybe you could just drop `op.hasContinuations()`
I had it like this to account for a case where a test could incorrectly add continuations to an OutgoingPushPromise without correctly setting the END_HEADERS flag on the PushPromiseFrame. This could possibly be handled in OutgoingPushPromise though.
-------------
PR: https://git.openjdk.java.net/jdk/pull/8518
More information about the net-dev
mailing list