Re: RFR 8162521, java/net/Authenticator/B4933582.sh fails intermittently with BindException
On 01/12/16 14:40, Felix Yang wrote: Hi Daniel, please review the new webrev: http://cr.openjdk.java.net/~xiaofeya/8162521/webrev.01/ Looks good Felix! -- daniel Thanks, Felix On 2016/12/1 18:58, Daniel Fuchs wrote: Hi Felix, Good to see one more script converted to java. I wonder if the fact that the test use to run two separate JVMs was significant. Hopefully it's not. According original bug (https://bugs.openjdk.java.net/browse/JDK-4933582), I didn't find out a strong reason to run with two different JVMs. But original TestHttpServe.terminate() cannot free ports on Windows, that is why the shutdown section was adjusted. This could be a possible reason. -Felix The changes in TestHttpServer seem OK - this class is shared by many other tests - so any modifications here is to be taken with care. I trust you ran all the jdk_net tests? Yes, tested on all platforms. -Felix In B4933582.java: maybe MyAuthenticator.count should be made volatile (or AtomicInteger). I don't think it's strictly necessary as I believe our implementation will only call it in the client thread (which is always the main thread here) - but it may be a better idea to use AtomicInteger anyway: just make no assumption :-) I'd also suggest to use try-with-resource to close in CacheImpl (int port) and CacheImpl::writeMap to make sure the file is always properly closed. Updated all the history codes, as you suggested Thanks, Felix best regards, -- daniel On 01/12/16 09:47, Felix Yang wrote: Hi, please review the following patch. The code was converted from shell script to plain Java. Since it is by-design to bind on a prior-used port, add a few of re-tries for possible BindException. Bug: https://bugs.openjdk.java.net/browse/JDK-8162521 Webrev: http://cr.openjdk.java.net/~xiaofeya/8162521/webrev.00/ Thanks, Felix
Re: RFR 8162521, java/net/Authenticator/B4933582.sh fails intermittently with BindException
Hi Daniel, please review the new webrev: http://cr.openjdk.java.net/~xiaofeya/8162521/webrev.01/ Thanks, Felix On 2016/12/1 18:58, Daniel Fuchs wrote: Hi Felix, Good to see one more script converted to java. I wonder if the fact that the test use to run two separate JVMs was significant. Hopefully it's not. According original bug (https://bugs.openjdk.java.net/browse/JDK-4933582), I didn't find out a strong reason to run with two different JVMs. But original TestHttpServe.terminate() cannot free ports on Windows, that is why the shutdown section was adjusted. This could be a possible reason. -Felix The changes in TestHttpServer seem OK - this class is shared by many other tests - so any modifications here is to be taken with care. I trust you ran all the jdk_net tests? Yes, tested on all platforms. -Felix In B4933582.java: maybe MyAuthenticator.count should be made volatile (or AtomicInteger). I don't think it's strictly necessary as I believe our implementation will only call it in the client thread (which is always the main thread here) - but it may be a better idea to use AtomicInteger anyway: just make no assumption :-) I'd also suggest to use try-with-resource to close in CacheImpl (int port) and CacheImpl::writeMap to make sure the file is always properly closed. Updated all the history codes, as you suggested Thanks, Felix best regards, -- daniel On 01/12/16 09:47, Felix Yang wrote: Hi, please review the following patch. The code was converted from shell script to plain Java. Since it is by-design to bind on a prior-used port, add a few of re-tries for possible BindException. Bug: https://bugs.openjdk.java.net/browse/JDK-8162521 Webrev: http://cr.openjdk.java.net/~xiaofeya/8162521/webrev.00/ Thanks, Felix
Re: RFR 8162521, java/net/Authenticator/B4933582.sh fails intermittently with BindException
Hi Felix, Good to see one more script converted to java. I wonder if the fact that the test use to run two separate JVMs was significant. Hopefully it's not. The changes in TestHttpServer seem OK - this class is shared by many other tests - so any modifications here is to be taken with care. I trust you ran all the jdk_net tests? In B4933582.java: maybe MyAuthenticator.count should be made volatile (or AtomicInteger). I don't think it's strictly necessary as I believe our implementation will only call it in the client thread (which is always the main thread here) - but it may be a better idea to use AtomicInteger anyway: just make no assumption :-) I'd also suggest to use try-with-resource to close in CacheImpl (int port) and CacheImpl::writeMap to make sure the file is always properly closed. best regards, -- daniel On 01/12/16 09:47, Felix Yang wrote: Hi, please review the following patch. The code was converted from shell script to plain Java. Since it is by-design to bind on a prior-used port, add a few of re-tries for possible BindException. Bug: https://bugs.openjdk.java.net/browse/JDK-8162521 Webrev: http://cr.openjdk.java.net/~xiaofeya/8162521/webrev.00/ Thanks, Felix
RFR 8162521, java/net/Authenticator/B4933582.sh fails intermittently with BindException
Hi, please review the following patch. The code was converted from shell script to plain Java. Since it is by-design to bind on a prior-used port, add a few of re-tries for possible BindException. Bug: https://bugs.openjdk.java.net/browse/JDK-8162521 Webrev: http://cr.openjdk.java.net/~xiaofeya/8162521/webrev.00/ Thanks, Felix
