On 2010-07-28, Grant Edwards <grant.b.edwa...@gmail.com> wrote:
> On 2010-07-27, Grant Edwards <grant.b.edwa...@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

Reply via email to