I believe you're referring to the close() in the catch clause following the 
call to getImpl().bind.  The problem I encountered was when the Datagram.bind 
threw an exception before it got that far.  In my case, the checkListen was 
throwing a SecurityException, but any of the earlier throws would cause the 
same problem.  The SecurityException wouldn't have been caught by the catch 
addressed by the CR in any case.  We encountered this while running the TCK.  
One of its tests tries to create lots of sockets, all of them getting security 
violations until we hit a limit on the number of open sockets.

    public synchronized void bind(SocketAddress addr) throws SocketException {
        if (isClosed())
            throw new SocketException("Socket is closed");
        if (isBound())
            throw new SocketException("already bound");
        if (addr == null)
            addr = new InetSocketAddress(0);
        if (!(addr instanceof InetSocketAddress))
            throw new IllegalArgumentException("Unsupported address type!");
        InetSocketAddress epoint = (InetSocketAddress) addr;
        if (epoint.isUnresolved())
            throw new SocketException("Unresolved address");
        InetAddress iaddr = epoint.getAddress();
        int port = epoint.getPort();
        checkAddress(iaddr, "bind");
        SecurityManager sec = System.getSecurityManager();
        if (sec != null) {
            sec.checkListen(port);                   <<<<<<<<<<<<<<<<<<<<<<<<<< 
This throws a SecurityException
        }
        try {
            getImpl().bind(port, iaddr);
        } catch (SocketException e) {
            getImpl().close();
            throw e;
        }
        bound = true;
    }

Tom.


-----Original Message-----
From: Chris Hegarty [mailto:chris.hega...@oracle.com] 
Sent: Monday, August 29, 2011 1:56 PM
To: Salter, Thomas A
Cc: net-dev@openjdk.java.net
Subject: Re: Datagram socket leak

[take two!]

Tom,

This specific area of code was changed recently due to CR 7035556 [1], 
changeset [2], and this issue was discussed during the code review [3].

Essentially, bind() already closes the impl internally before throwing 
the exception. Does this resolve the issue for you? Or do you still see 
potential to leak?

-Chris

[1] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7035556
[2] http://hg.openjdk.java.net/jdk8/tl/jdk/rev/07a12583d4ea
[3] http://mail.openjdk.java.net/pipermail/net-dev/2011-July/003318.html

On 08/29/11 03:27 PM, Salter, Thomas A wrote:
> There appears to be a socket leak in both DatagramSocket and
> MulticastSocket constructors. Both classes have constructors that create
> a socket and then attempt to bind. The bind can fail with a variety of
> exceptions none of which are caught by the constructor. Thus, the actual
> system socket that was allocated by impl.create() is never closed.
>
> My fix was to wrap a try-finally around the bind call and call close()
> if isBound is false.
>
> public DatagramSocket(SocketAddress bindaddr) throws SocketException {
>
> // create a datagram socket.
>
> createImpl();
>
> if (bindaddr != null) {
>
> try {
>
> bind(bindaddr);
>
> } finally {
>
> if( !isBound() )
>
> close();
>
> }
>
> }
>
> }
>
> Tom Salter
>
> Unisys Corporation
>

Reply via email to