Issue #5794 ensuring serverChannel is closed in the event of a failed bind to ensure proper resources are cleaned

[joewitt]

Issue #5794 ensuring serverChannel is closed in the event of a failed bind to ensure proper resources are cleaned

Signed-off-by: Joe Witt joewitt@apache.org

[sbordet]

Can you please rework this fix to use try-with-resources, so there is no need of an explicit close?

[joewitt]

We only want the serverChannel closed if the BindException occurs. Otherwise it needs to be returned to the caller in an open state. I don't believe try-with-resources is what we want here but let me know if I misunderstood.

[gregw]

@sbordet try-with-resources doesn't work in this case as we use the socket in the successful case... actually probably should null out the field... just in case....

[gregw]

Actually no fields were set, but there was one already in a try-with-resources, so backed the change out of that one.

[joewitt]

Looks like the serverChannel is within the method scope so nothing to null out. I looked at the caller scope or otherwise to see if there was a better place to adjust the close (such as a try with resources). And I'm not seeing that. For the specific issue observed the change as I have it closes that problem. If this is part of a larger concern I'm not sure I follow. Do you need any change from the patch as-is?

[sbordet]

Re: using try-with-resources: doh :)

@gregw #5802 is the same fix? @joewitt can you use IO.close() instead? Also, I think we want to catch everything that may possibly be throws after ServerSocketChannel.open(), so I would widen the try scope.

[sbordet]

Incorporating this change within #5802.

[sbordet]

Re: using try-with-resources: doh :)

@gregw #5802 is the same fix? @joewitt can you use IO.close() instead? Also, I think we want to catch everything that may possibly be throws after ServerSocketChannel.open(), so I would widen the try scope.

[gregw]

Oops I got confused and did #5802. Resolving now....