On Mon, Oct 03, 2016 at 11:36:19PM +1100, Lawrence Stewart wrote:
> Hi Ruslan,
>
> On Mon, Oct 3, 2016 at 6:45 PM, Ruslan Ermilov <ru@nginx.com> wrote:
>
> > On Mon, Oct 03, 2016 at 03:37:38PM +1100, Lawrence Stewart wrote:
> > > Hi all,
> > >
> > > I need to pull kernel data using getsockopt() for a connection prior to
> > the
> > > socket being closed. A pool cleanup handler registered with the
> > > ngx_connection_t's pool would seem to be the right way to do this, but
> > > ngx_http_close_connection() calls ngx_close_connection() prior to
> > > ngx_destroy_pool(), which means my cleanup handler sees a closed socket.
> > >
> > > A look through ngx_close_connection() and the functions it calls would
> > lead
> > > me to believe it should be safe (and possible even desirable in an
> > ordering
> > > sense) to destroy the connection's pool prior to calling
> > > ngx_close_connection().
> >
> > If we talk about client connections, then c->log is allocated from
> > c->pool, and c->read->log and c->write->log also use this log object,
> > see ngx_event_accept().
> >
> > A look through ngx_close_connection() shows that nearly every call
> > it makes: ngx_log_error, ngx_del_timer, ngx_del_conn, ngx_del_event,
> > ngx_delete_posted_event, and ngx_reusable_connection need c->log
> > which would be destroyed if c->pool was destroyed.
> >
>
> Thanks, I missed this log allocation detail.
>
>
> >
> > > There is an existence proof in the nginx tree that
> > > this is reasonable to do (see [1]) and there is also a comment in
> > > ngx_close_connection() that implies it expects that the connection's pool
> > > has already been destroyed i.e.
> > >
> > > /* we use ngx_cycle->log because c->log was in c->pool */
> >
> > This comment seems outdated since rev. c52408583801 which removed
> > the ngx_destroy_pool(c->pool) call from ngx_close_connection():
> >
> >
> Ah, ok.
>
>
> > : @@ -388,8 +387,6 @@ void ngx_close_connection(ngx_connection
> > : c->fd = (ngx_socket_t) -1;
> > : c->data = NULL;
> > :
> > : - ngx_destroy_pool(c->pool);
> > : -
> > : if (ngx_close_socket(fd) == -1) {
> > :
> > : /* we use ngx_cycle->log because c->log was in c->pool */
> >
> > > Are there some subtleties I'm missing, or is it correct and reasonable to
> > > always call ngx_destroy_pool() on a connection's pool prior to calling
> > > ngx_close_connection()?
> >
> > On the contrary, the upstream connection's pool is destroyed
> > before calling ngx_close_connection(), see ngx_http_upstream_next()
> > and ngx_http_upstream_finalize_request(). This is safe because
> > the upstream connection reuses the client connection's log, see
> > ngx_http_upstream_connect().
> >
>
> Ah, I missed this detail too. Thanks for the pointer.
>
> So given that my idea is a fail, what is the recommended way to hook
> connection tear down from a module so that I can achieve my goal?

See if ngx_http_cleanup_add() fits your needs.

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel