On Sun, 17 Jan 2021 02:02:54 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

>> As noted by the reporter in the comments of 
>> https://bugs.openjdk.java.net/browse/JDK-7146776, the `URLStreamHandler` in 
>> its `getHostAddress` is incorrectly synchronizing on the `URLStreamHandler` 
>> instance, instead of synchronizing on the `URL` instance that is being 
>> passed to that method.
>> 
>> The implementation of `URLStreamHandler#getHostAddress` checks for the 
>> (package private) `hostAddress` member of the passed `URL` and if it isn't 
>> set then does a DNS lookup to get the address and finally sets the 
>> `hostAddress` member of the `URL` to this returned address. The access to 
>> `hostAddress` of `URL` needs to be `synchronized` since it can be updated 
>> (in a different thread) in the `URL#set` methods.
>> 
>> The commit here removes the synchronization from the 
>> `URLStreamHandler#getHostAddress` and moves the entire implementation into a 
>> new (package private) `URL#getHostAddress` method and then calls this new 
>> method from the `URLStreamHandler#getHostAddress`. I decided to do it this 
>> way instead of just leaving the implementation in 
>> `URLStreamHandler#getHostAddress` and synchronizing on the passed `URL` 
>> instance, since IMO, this makes it much more easier to see (in one place 
>> within the `URL` class) what the synchronization requirements are when 
>> dealing with the `hostAddress`. Furthermore, this now allows the 
>> `hostAddress` to be made a `private` member (which I have done in this 
>> commit) instead of the previous package private access. I checked that no 
>> other code tries to access this package private `URL#hostAddress`.
>> 
>> I can't think of an easy way to add a jtreg test for this change, so I 
>> haven't added one.
>
> Jaikiran Pai has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Javadoc update

Looks good to me.

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

Marked as reviewed by chegar (Reviewer).

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

Reply via email to