8231259: (dc) DatagramChannel::disconnect re-binds socket to the wildcard address (macOS)

[Daniel Fuchs]

Thanks Alan!

That looks good to me now. I agree the try-finally provides
a better guarantee.

best regards,

-- daniel

On 18/11/2019 20:35, Alan Bateman wrote:
> On 18/11/2019 15:00, Daniel Fuchs wrote:
>> :
>>
>> DatagramChannelImpl:
>>
>> I wonder however if:
>>
>> 1177                     InetSocketAddress isa = Net.localAddress(fd);
>>
>> should be
>>
>>                          InetSocketAddress isa = localAddress =
>> Net.localAddress(fd);
>>
>> In case repairSocket() later fails. Although we already specified
>> that "the socket is an unspecified state", it would leave
>> the socket is in an unspecified state closer to what it was
>> before the fix.
> That is a good point, there should be a try-finally around the repair so 
> that the localAddress is set even when it fails.
> 
> 
>>
>> 1222             Object value = getOption(option);
>> 1223             if (value != null) {
>> 1224                 map.put(option, value);
>> 1225             }
>>
>> Should we catch and ignore IOException thrown by getOption(),
>> there - as you did in the test?
> No, I think we want to fail in this case. The test catches IOException 
> here because there are extended socket options on Solaris that cannot be 
> read after the socket is bound.
> 
> 
>>
>> MembershipRegistry:
>>
>>  123     interface ThrowingConsumer<T, X extends Throwable> {...
>>
>> should that be annotated with @FunctionalInterface?
> Okay.
> 
> I've put an updated webrev here:
>    http://cr.openjdk.java.net/~alanb/8231259/1/webrev/
> 
> This has the try-finally around the repair and as @FI to the non-public 
> interface. No other changes.
> 
> -Alan
> 
>