On 28/08/2020 09:54, Daniel Fuchs wrote:
Hi Michael,
On 28/08/2020 09:46, Michael McMahon wrote:
Daniel,
I wonder if the new Cancelable interface could be simplified to
remove the "mayInterruptIfRunning" parameter? It seems like the
cancellation operation has no effect if the parameter is false..
The interface mimics the Future::cancel and CompletableFuture::cancel
API. IMO it's easier to delegate the handling of the boolean parameter
at the implementation site (MultiExchange) rather than at the calling
site (CompletableFuture). At one point in the history of the fix I also
had MinimalFuture implement Cancellable. I stepped back from that after
reflection but it could still become handy.
Okay, that's fine. It was just an observation really
Thanks
Michael.
Otherwise, I am happy with the change.
Thanks Michael!
best regards,
-- daniel
Thanks
Michael
On 31/07/2020 13:00, Michael McMahon wrote:
Hi Daniel,
Good to see that produced useful results. I think it would be a good
idea
to separate the two additional fixes if possible. That would certainly
make back-porting a lot easier.
Thanks,
Michael.
On 31/07/2020 10:46, Daniel Fuchs wrote:
Thanks for suggesting this course of action Michael!
This flushed out two additional bugs in our stack, which I believe
are probably at the root of some of the rare intermittent failures
we have been observing in the Throwing* tests:
- SocketTube: I found an issue where the scheduler might not
be restarted if resuming/pausing event from within
the scheduler loop (that runs in the selector manager
thread) failed due to the socket being asynchronously
closed by another thread.
That could cause some tests to fail in timeout.
- Http2Connection/Stream: there was an issue where DataFrames
could be sent after a ResetFrame was sent. That caused the
server to close down the connection. The next test would
start opening a new stream on the same connection while
the server was concurrently closing it, and the test
would eventually fail - sometimes with a message saying
"EOF reached while reading".
The following webrev includes these two additional fixes, and I have
now very stable test runs. I wonder if I should try to extract those
two fixes though - as it might be worthwile to backport
them independently:
http://cr.openjdk.java.net/~dfuchs/webrev_8245462/webrev.01/index.html
best regards,
-- daniel
On 28/07/2020 15:19, Daniel Fuchs wrote:
On 28/07/2020 15:04, Michael McMahon wrote:
The code is technically racy on the GET test, but it's often the
case when you want
something to be racy then it turns out not to be in practice, 99
times out of a 100 anyway
(figures made up). I was thinking you could put a random sleep on
the client side before calling
cancel (say between 1ms and the SERVER_LATENCY constant). Print
out the random value too
in case it finds a problem.
Oh - that's a good point. Let me try that.
best regards,
-- daniel
