Thanks Roger.
On 2019/11/1 11:14 PM, Roger Riggs wrote:
Hi Hamlin,
I would expect endPoint().getPort() to be non-zero except during the
narrow window of the initialization before the server socket is opened.
See TCPEndPoint.listen().
I assumed you mean TCPTransport.listen().
If that's true, then either there is no server socket to close or
that part of the condition is always true and could be removed.
TCPTransport.listen() is synced against TCPTransport instance, and
decrementExportCount() is also synced against TCPTransport instance, so
I agree we could just remove getListenPort()/getPort() check.
So I think we'd need to understand if it was intentional to leave
an application specified port open, while closing one that
the application has no (specific) knowledge of.
I think it was intentional, reasons are:
1. check TCPTransport.setDefaultPort(...) ).
2. in code "r1 = LocateRegistry.createRegistry(0);
UnicastRemoteObject.unexportObject(r1, true); r2 =
LocateRegistry.createRegistry(0);", r1 and r1 use the same server socket
port.
But I think we can/should change this behavior, reasons are:
1. it's not necessary, it's still a resource leak if I don't use port 0
anymore.
2. it's not stated in the API doc.
3. if it's not stated in the API doc, then this behavior lead to
inconsistency between 0 and other ports.
4. more important, this implementation makes some test scenarios hard to
be stable (like JDK-8233105
<https://bugs.openjdk.java.net/browse/JDK-8233105>).
Thank you
-Hamlin
$.02, Roger
BTW, verifyPortFree() would easier to understand and code with if it
returned a boolean
instead throwing an exception. The non-local exception handling makes
the code harder to follow.
On 11/1/19 12:47 AM, Hamlin Li wrote:
Would you please to review the following patch?
bug: https://bugs.openjdk.java.net/browse/JDK-8233313
webrev: http://cr.openjdk.java.net/~mli/8233313/webrev.00/
With following code, port used behind is not closed after unexportObject
/Registry registry = LocateRegistry.createRegistry(0);//
//boolean b = UnicastRemoteObject.unexportObject(registry, true);/
But, the port can be closed if pass in an explicit port number to
createRegistry.
I think is not right and is a resource leak.
Thank you
-Hamlin
