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?