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.fu...@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
>
