RFR [9] 8180155: WebSocket secure connection get stuck after onOpen

[Pavel Rappo]

Hi Daniel,

Thanks a lot for looking into this! I agree there's an issue exactly where you
pointed at. I'm sure it would be a good idea to write a test that covers this.
Unfortunately it would be quite a challenging task.

I tried to rewrite the state machine the way that both fixes this bug and makes
the code easier to follow (I'm biased here obviously).

Please have a look at the updated webrev and tell me if you're generally happy
with this approach:

    http://cr.openjdk.java.net/~prappo/8180155/webrev.01/

So here are a couple of points to keep in mind while reasoning about the code:

  1. There's at most one channel readiness event registered at any given time
  2. There's at most one thread executing `pushContinuously` at any given time
     (enforced by CooperativeHandler)

Thanks!

> On 31 May 2017, at 12:13, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
> 
> Hi Pavel,
> 
> Receiver.java:
> 
> 120     private void pushContinuously() {
> 121         while (true) {
> 122             if (!readable.get()) {
> 123                 if (!initialized) {
> 124                     initialized = true;
> 125                     try {
> 126                         channel.registerEvent(event);
> 127                     } catch (IOException e) {
> 128                         messageConsumer.onError(e);
> 129                     }
> 130                 }
> 131                 break;
> 132             } else if (demand.get() > 0 && !handler.isStopped()) {
> 133                 pushOnce();
> 134             } else {
> 135                 break;
> 136             }
> 137         }
> 138     }
> 
> I think you might have an issue here, if the initialBuffer
> already contains all the data that there is to be read, then
> the readable.get() will be false, initialized will be false,
> pushOnce() will not be called, and you're going to register
> an event that will never be triggered.
> 
> best regards,
> 
> -- daniel
> 
> On 31/05/2017 11:30, Pavel Rappo wrote:
>> Hello,
>> Please review the following change:
>>  http://cr.openjdk.java.net/~prappo/8180155/webrev.00/
>> This change addresses 2 separate issues:
>>     https://bugs.openjdk.java.net/browse/JDK-8180155
>>     https://bugs.openjdk.java.net/browse/JDK-8156518
>> The first one is a busy-wait for data on the socket which manifests itself as
>> non-returning invocation of WebSocket.request(long).
>> The second one is an issue with a timeout for an asynchronous HTTP request.
>> This issue affects both HttpClient and WebSocket implementations.
>> Thanks,
>> -Pavel
>