RFR [12] 8211092: test/jdk/sun/net/www/http/HttpClient/MultiThreadTest.java fails intermittently when cleaning up

[Daniel Fuchs]

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.hegarty at 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.fuchs at 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
>>>
>>
>