RFR 8195059: Update java.net Socket and DatagramSocket implementations to use Cleaner

Chris Hegarty chris.hegarty at oracle.com
Fri Feb 2 16:30:03 UTC 2018


Roger,

On 01/02/18 21:29, Roger Riggs wrote:
> Hi Chris,
> 
> Thanks for the review and suggestion.
> 
> Webrev updated:
> http://cr.openjdk.java.net/~rriggs/webrev-net-cleanup-8195059/

This looks good to me, just a few small comments:

1) windows SocketImpl.c in the comments:
      java_net_AbstractPlainSocketImpl -> 
java_net_SocketCleanable_cleanupClose0

2) SocketCleanable.java needs a copyright header

-Chris.

>   * Refactored SocketCleanup into a java.net package private SocketCleanable
>   * Moved register and unregister into static methods in SocketCleanable
>   * Simplified the test by retaining the checking for GC reclaimed
>     objects but
>     removed checking fd counts, since they were unreliable in testing
>     and not consistent across OS's
> 
> Thanks, Roger
> 
> On 2/1/2018 10:11 AM, Chris Hegarty wrote:
>> Hi Roger,
>>
>>> On 31 Jan 2018, at 15:52, Roger Riggs<Roger.Riggs at oracle.com>  wrote:
>>>
>>> Addingnet-dev at openjdk.java.net
>>>
>>> On 1/30/2018 5:08 PM, Roger Riggs wrote:
>>>> Please review changes to replace finalizers in socket, datagram, and multicast networking
>>>> with Cleaner based release of the raw file descriptors.  Each FileDescriptor is registered
>>>> for cleanup after the raw fd (or handle) is assigned.  Normal calls to close unregister the
>>>> cleaner before using the current logic to close the raw fd/handle. Windows networking
>>>> uses fd's with the Windows socket_ API requiring a special cased Cleanable.
>>>>
>>>> The tests check that the implementation objects including FileDescriptors are reclaimed
>>>> and for Linux that the raw fd counts are reduced as expected.
>>>>
>>>> Webrev:
>>>>     http://cr.openjdk.java.net/~rriggs/webrev-net-cleanup-8195059/
>> I think this is good. One small comment; could SocketCleanup be a
>> top-level package-private class, since it is shared by the three
>> different socket types.
>>
>> I didn’t look too hard at the tests, other than to note that they seem
>> to verify expected behaviour, which is good.
>>
>> -Chris.
> 


More information about the core-libs-dev mailing list