SSL FD Leak

[Igor Sysoev]

On Thu, Jan 03, 2008 at 12:38:14PM -0500, Ben Maurer wrote:

> Hi Igor,
> 
> I hope you had a good vacation.
> 
> Ben Maurer wrote:
> >Ben Maurer wrote:
> >>Some progress on debugging this -- it may have to to do with the 
> >>deferred setting.
> >>
> >>I've managed to get straces like this:
> >>
> >>accept(6, {sa_family=AF_INET, sin_port=htons(35327), 
> >>sin_addr=inet_addr("127.0.0.1")}, [16]) = 92
> >>ioctl(92, FIONBIO, [1])                 = 0
> >>recv(92, 0xbf9c6c2b, 1, MSG_PEEK)       = -1 EAGAIN (Resource 
> >>temporarily unavailable)
> >>
> >>by using:
> >>
> >>ab -c500 -n2000 https://localhost:8095/
> >>
> >>and aborting in the middle. It seems that these straces are the ones 
> >>that result in leaked FDs. The trace really doesn't make much sense to 
> >>me. Deferred accept promises that the socket only goes into accept 
> >>once it has data or if it's ready to be closed. Neither of these 
> >>should result in an EAGAIN. Regardless, it seems the problem is that 
> >>the FD never gets added to epoll at this point.
> >
> >It seems like commenting out the check for HTTP requests on the socket 
> >made everything work. There's probably a way to do this more correctly 
> >(eg, get the event added back into the epoll structure). With that said, 
> >maybe it'd be possible to avoid the MSG_PEEK call completely. Openssl is 
> > good at detecting this error:
> 
> 
> I tracked down one more FD leak -- it seems that if openssl returns a 
> WANT_READ or a WANT_WRITE during the handshake and the client stops 
> responding to packets that the FDs will leak. I think the solution to 
> this is to add a ngx_add_timer(c->read, 30000); when WANT_READ is 
> returned in ngx_ssl_handshake.
> 
> I'm a bit worried about how easy it is for a connection to not have any 
> timer and thus be leaked when the client completely loses connectivity. 
> Is it worth thinking about some safeguard against this. For example, by 
> enforcing that every connection has some sort of timer guarding it.

The attached patch should fix the leak.


-- 
Igor Sysoev
http://sysoev.ru/en/
-------------- next part --------------
Index: src/http/ngx_http_request.c
===================================================================
--- src/http/ngx_http_request.c	(revision 1148)
+++ src/http/ngx_http_request.c	(working copy)
@@ -490,6 +490,11 @@
             rc = ngx_ssl_handshake(c);
 
             if (rc == NGX_AGAIN) {
+
+                if (!rev->timer_set) {
+                    ngx_add_timer(rev, c->listening->post_accept_timeout);
+                }
+
                 c->ssl->handler = ngx_http_ssl_handshake_handler;
                 return;
             }