On Tue, 7 Dec 2021 14:28:01 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:

>> Jaikiran Pai has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   minor - rename variable in test
>
> Generally it would be good if the test did not make any assumption on the 
> presence - or absence - of either IPv4 or IPv6 on the tested machine.
> The IPSupport test library class has methods that allows a test to inquire 
> whether IPv6 or IPv4 are available - I would recommend using them to figure 
> out which test cases can be tested on the machine the test runs on.

> @dfuch Hello Daniel, on a slightly unrelated note I see there's a PR which 
> tries to address too many HTTPClient instances resulting in odd failures on 
> Windows #7263.
> 
> In this current PR of mine, I have a new test `HttpClientLocalAddrTest` which 
> will end up creating (atmost) 20 HTTPClient instances. In my runs so far, I 
> haven't seen any failures due to the number of instances, but I don't have a 
> Windows setup to verify if these many clients can end up creating any issues 
> on them. Unlike the change in that other PR which reduces the HTTPClient 
> instance to just one, we won't be able to use that trick here since this 
> whole test intentionally attempts to create multiple different instance of 
> the HTTPClient. I hope this won't cause any issues on those Windows setups.

Hi Jaikiran. First sorry for the delay in reviewing this change. Hopefully I'll 
be able to dedicate some more time to it in the coming weeks. WRT to your 
concerns with the number of HttpClient instances created by your test, I don't 
think this will be an issue (until proven false). Michael has a PR out 
(https://git.openjdk.java.net/jdk/pull/7302) that should solve the issue by 
using Unix Domain Socket in the pipe implementation of the selector. 

Even though that might also remove the need for my PR I'm still intending to 
proceed with it - as it should be safer to backport a test-only change to jdk 
18 / jdk 17. Your test will be in the main line only - so it can benefit from 
Michael's PR.

-------------

PR: https://git.openjdk.java.net/jdk/pull/6690

Reply via email to