Dropped https client connection doesn't drop backend proxy_pass connection

[Maxim Dounin]

Hello!

On Tue, Mar 19, 2013 at 03:45:10PM +1100, Robert Mueller wrote:

> 
> > > When an https client drops it's connection, the upstream http proxy
> > > connection is not dropped. If nginx can't detect an https client
> > > disconnect properly, that must mean it's leaking connection information
> > > internally doesn't it?
> > 
> > No.  It just can't say if a connection was closed or not as there 
> > are pending data in the connection, and it can't read data (there 
> > may be a pipelined request).  Therefore in this case, being on the 
> > safe side, it assumes the connection isn't closed and doesn't try 
> > to abort upstream request.
> 
> Oh right I see now.
> 
> So the underlying problem is that the nginx stream layer abstraction
> isn't clean enough to handle low level OS events and then map them
> through the SSL layer to read/write/eof conceptual events as needed.
> Instead you need an OS level "eof" event, which you then assume maps
> through the SSL abstraction layer to a SSL stream eof event.

Not exactly.  The underlying problem is that BSD sockets API 
doesn't provide standard means to detect EOF without reading all 
pending data, and hence OS-specific extensions have to be used to 
reliably detect pending EOFs.

> Ok, so I had a look at the kqueue eof handling, and what's needed for
> epoll eof handling, and created a quick patch that seems to work.
> 
> Can you check this out, and see if it looks right. If so, any chance you
> can incorporate it upstream?
> 
> http://robm.fastmail.fm/downloads/nginx-epoll-eof.patch
> 
> If there's anything you want changed, let me know and I'll try and fix
> it up.
 
I don't really like what you did in ngx_http_upstream.c.  And 
there are more places where ev->pending_eof is used, and probably 
at least some of these places should be adapted, too.

Additionally, poll_ctl(2) manpage claims the EPOLLRDHUP only 
available since Linux 2.6.17, and this suggests it needs some 
configure tests/conditional compilation.

Valentin is already worked on this, and I believe he'll be able to 
provide a little bit more generic patch.

-- 
Maxim Dounin
http://nginx.org/en/donation.html