Hi Chris,
Good finding!
I agree with the principle, but I think that strictly speaking
it would be more correct if the server's workers ArrayList was
changed into a ConcurrentLinkedQueue.
The main could then poll the queue & join until it's empty
(or alternatively use forEach)
best regards,
-- daniel
On 27/09/2018 11:28, Chris Hegarty wrote:
Reviving this RFR ...
In fact we don’t need to wait for jtreg b14, we can change the test now
and remove it from the problem list.
The issue with the test is that is creates non-daemon threads that
remain blocked in socket read until the HTTP keep-alive times out on the
client-side. This is fine, and exactly what the test is trying to
assert. The problem occurs because the test, run in samevm / agentvm
mode, returns from its `main` method when the client requests complete,
this can be up to 15 seconds before the other non-daemon “worker”
threads complete.
From jtreg’s perspective, the test is finished when the `main` method
returns. Jtreg then attempts to cleanup ( terminate / interrupt ) any
threads that the test has left behind. Jtreg fails to cleanup the non-
daemon worker threads as they are blocked in socket read. Jtreg then
gives up and issues an error.
The test’s main method should not return until all of its worker threads
have completed. This validates the assertions in the tests, as well as
not putting jtreg under pressure to do cleanup.
http://cr.openjdk.java.net/~chegar/8211092.01/
-Chris.
On 25 Sep 2018, at 14:49, Chris Hegarty <chris.hega...@oracle.com> wrote:
Thanks Daniel,
Let’s postpone any potential change to the test until jtreg b14
is promoted. See
http://mail.openjdk.java.net/pipermail/net-dev/2018-September/011775.html
-Chris.
On 25 Sep 2018, at 10:17, Daniel Fuchs <daniel.fu...@oracle.com> wrote:
Hi Chris,
I was looking at that test too - especially WRT to this comment
at the beginning:
/*
* This tests keep-alive behavior using chunkedinputstreams
* It checks that keep-alive connections are used and also
* that requests are not being repeated (due to errors)
*
* It also checks that the keepalive connections are closed eventually
* because the test will not terminate if the connections
* are not closed by the keep-alive timer.
*/
I think that with your proposed change then the test will no
longer check that the `keepalive connections are closed eventually`.
Maybe that's OK - in which case that comment should be corrected.
best regards,
-- daniel
On 25/09/2018 10:01, Chris Hegarty wrote:
This is a test-only change to allow the test to terminate its
worker threads more timely, rather than leaving it to jtreg
( which has caused some issues, see 8208690 and
7902259 ). The test now closes the worker's socket so
that the worker thread, that blocked may be blocked in a
socket read, can exit more timely.
http://cr.openjdk.java.net/~chegar/8211092/
-Chris.
[1] https://bugs.openjdk.java.net/browse/JDK-8208690
[2] https://bugs.openjdk.java.net/browse/CODETOOLS-7902259
