RFR 8156504/9, java/net/URLPermission/nstest/lookup.sh fails intermittently
Daniel Fuchs
daniel.fuchs at oracle.com
Wed Nov 2 14:12:26 UTC 2016
On 02/11/16 11:18, Chris Hegarty wrote:
> Thanks for doing this Felix.
>
> I have some comments about the style and the use of AccessControlContext.
> Rather than list them I put an alternative version at the following location, please
> take a look.
>
> http://cr.openjdk.java.net/~chegar/8156504_alt/
>
> Note: specifically it is good practice to read off the HTTP request before
> closing the socket ( on the server socket ). Not doing so can lead to reset
> issues.
+ 1 for getting rid of the AccessControlContext.
I was not very comfortable with that either.
cheers,
-- daniel
>
> -Chris.
>
>> On 2 Nov 2016, at 07:10, Felix Yang <felix.yang at oracle.com> wrote:
>>
>> Hi Amy,
>>
>> thanks for the comments. Updated webrev:
>>
>> http://cr.openjdk.java.net/~xiaofeya/8156504/webrev.01/
>>
>> -Felix
>> On 2016/11/2 14:18, Amy Lu wrote:
>>> Good to see one more script test be changed to java, thank you Felix.
>>>
>>> I'm not official reviewer, but some minor comments.
>>>
>>> 30 * @library /lib/testlibrary
>>> 31 * @build jdk.testlibrary.*
>>> 32 * @compile LookupTest.java
>>> I noticed test requires testlibrary, but seems test do not actually depend on that lib, these lines could be removed.
>>>
>>> 77 is.close();
>>> Is it better to do the close in the finally block?
>>>
>>> 128 if (serverSocket == null) {
>>> 129 serverSocket.close();
>>> 130 }
>>> Typo here?
>>>
>>> Even more minor...
>>> - private static void addMappingToHostsFile (String host,
>>> - String addr,
>>> - String hostsFileName,
>>> - boolean append)
>>> - throws Exception {
>>> + private static void addMappingToHostsFile(String host, String addr,
>>> + String hostsFileName, boolean append) throws Exception {
>>> This might be reformatted automatically by IDE, but just feel previous one is more easy to read.
>>>
>>> Thanks,
>>> Amy
>>>
>>>
>>> On 11/2/16 10:39 AM, Felix Yang wrote:
>>>> Hi there,
>>>>
>>>> please review the following patch for an intermittent failing test. Converted it into plain java test and avoid free port anti-pattern.
>>>>
>>>> Bug:
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8156504
>>>>
>>>> Webrev:
>>>>
>>>> http://cr.openjdk.java.net/~xiaofeya/8156504/webrev.00/
>>>>
>>>> Thanks,
>>>>
>>>> Felix
>>>>
>>>
>>
>
More information about the net-dev
mailing list