[ECOS] Re: Socket leak when accept() is aborted by TCP RST?

[Grant Edwards <grant dot b dot edwards at gmail dot com>]

On 2010-07-28, Grant Edwards <grant.b.edwards@gmail.com> wrote:
> On 2010-07-27, Grant Edwards <grant.b.edwards@gmail.com> wrote:
>
>> I'm seeing what appears to me to be a socket leak in the accept()
>> operation provided by the "new" BSD network stack.
>
> The stack is definitely leaking sockets.  I can now reliably reproduce
> the problem by opening a TCP connection and then immediately causing a
> TCP reset.

AFAICT, the leak occurs in kern/sockio.c in bsd_accept():

   289	static int 
   290	bsd_accept(cyg_file *fp, cyg_file *new_fp,
   291	           struct sockaddr *name, socklen_t *anamelen)
   292	{
   293	    socklen_t namelen = 0;
   294	    int error = 0, s;
   295	    struct socket *head, *so;
   296	    struct sockaddr *sa;
  [...]
   334	    /*
   335	     * At this point we know that there is at least one connection
   336	     * ready to be accepted. Remove it from the queue prior to
   337	     * allocating the file descriptor for it since falloc() may
   338	     * block allowing another process to accept the connection
   339	     * instead.
   340	     */
   341	    so = TAILQ_FIRST(&head->so_comp);
   342	    TAILQ_REMOVE(&head->so_comp, so, so_list);
   343	    head->so_qlen--;

The socket has been removed from the queue at this point.
   
  [...]

   382	    error = soaccept(so, &sa);

soaccept() calls tcp_usr_accept(), which checks the socket state and
upon discovering it's not connected it sets errno=353 and returns an
error.
   
   383	    if (error) {
   384	        /*
   385	         * return a namelen of zero for older code which might
   386	         * ignore the return value from accept.
   387	         */	
   388	        if (name != NULL) {
   389	            *anamelen = 0;
   390	        }
   391	        goto noconnection;
   392	    }
  [...]
  
   413	noconnection:
   414	
   415	#if 0 // FIXME
  [...]
   437	#else
   438	 done:
   439	    splx(s);
   440	    if (sa)
   441	        FREE(sa, M_SONAME);
   442	#endif
   443	    
   444	    return (error);
   445	}

The socket structure pointed to by 'so' is not freed.

When an error is returned, the accept() function in
io/fileio//socket.cxx frees the file pointer and file descriptor but
not the socket:

     1	//==========================================================================
     2	//
     3	//      socket.cxx
  [...]     
   198	__externC int	accept (int s, struct sockaddr *sa, socklen_t *addrlen)
   199	{
   200	    SOCKET_ENTRY();
   
  [...]
  
   235	        err = ops->accept( fp, new_fp, sa, addrlen );
   236	
   237	        UNLOCK_SOCKET( fp );
   238	
   239	    }
   240	    else err = EBADF;
   241	
   242	    if( err != 0 )
   243	    {
   244	        cyg_fp_free( fp );
   245	        cyg_fd_free(fd);
   246	        cyg_file_free(new_fp);        
   247	        SOCKET_RETURN( err );
   248	    }
  [...]


So, nowhere does the socket get freed.  

I've verified that when connections are closed or reset _after_
accept() returns OK, the values of "so" retrieved from the queue at
line 341 get re-used over and over again.  When the connection is
immediately reset causing tcp_usr_accept() to return an error, the
values of "so" keep going up (you never see a repeated value) and
eventually you run out of sockets.
  
Where should the socket be freed?  In bsd_accept() where it is removed
from the queue?  Or in socket.cxx when the fd/fp are freed?
  
-- 
Grant Edwards               grant.b.edwards        Yow! I want a VEGETARIAN
                                  at               BURRITO to go ... with
                              gmail.com            EXTRA MSG!!


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss