<div dir="ltr">Hi Ruslan,<br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Oct 3, 2016 at 6:45 PM, Ruslan Ermilov <span dir="ltr"><<a href="mailto:ru@nginx.com" target="_blank">ru@nginx.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Mon, Oct 03, 2016 at 03:37:38PM +1100, Lawrence Stewart wrote:<br>
> Hi all,<br>
><br>
> I need to pull kernel data using getsockopt() for a connection prior to the<br>
> socket being closed. A pool cleanup handler registered with the<br>
> ngx_connection_t's pool would seem to be the right way to do this, but<br>
> ngx_http_close_connection() calls ngx_close_connection() prior to<br>
> ngx_destroy_pool(), which means my cleanup handler sees a closed socket.<br>
><br>
> A look through ngx_close_connection() and the functions it calls would lead<br>
> me to believe it should be safe (and possible even desirable in an ordering<br>
> sense) to destroy the connection's pool prior to calling<br>
> ngx_close_connection().<br>
<br>
</span>If we talk about client connections, then c->log is allocated from<br>
c->pool, and c->read->log and c->write->log also use this log object,<br>
see ngx_event_accept().<br>
<br>
A look through ngx_close_connection() shows that nearly every call<br>
it makes: ngx_log_error, ngx_del_timer, ngx_del_conn, ngx_del_event,<br>
ngx_delete_posted_event, and ngx_reusable_connection need c->log<br>
which would be destroyed if c->pool was destroyed.<br></blockquote><br>Thanks, I missed this log allocation detail.<br><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> There is an existence proof in the nginx tree that<br>
> this is reasonable to do (see [1]) and there is also a comment in<br>
> ngx_close_connection() that implies it expects that the connection's pool<br>
> has already been destroyed i.e.<br>
><br>
> /* we use ngx_cycle->log because c->log was in c->pool */<br>
<br>
</span>This comment seems outdated since rev. c52408583801 which removed<br>
the ngx_destroy_pool(c->pool) call from ngx_close_connection():<br>
<br></blockquote><div><br></div><div>Ah, ok.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
: @@ -388,8 +387,6 @@ void ngx_close_connection(ngx_<wbr>connection<br>
:      c->fd = (ngx_socket_t) -1;<br>
:      c->data = NULL;<br>
:<br>
: -    ngx_destroy_pool(c->pool);<br>
: -<br>
:      if (ngx_close_socket(fd) == -1) {<br>
:<br>
:          /* we use ngx_cycle->log because c->log was in c->pool */<br>
<span class=""><br>
> Are there some subtleties I'm missing, or is it correct and reasonable to<br>
> always call ngx_destroy_pool() on a connection's pool prior to calling<br>
> ngx_close_connection()?<br>
<br>
</span>On the contrary, the upstream connection's pool is destroyed<br>
before calling ngx_close_connection(), see ngx_http_upstream_next()<br>
and ngx_http_upstream_finalize_<wbr>request().  This is safe because<br>
the upstream connection reuses the client connection's log, see<br>
ngx_http_upstream_connect().<br></blockquote><div><br></div><div>Ah, I missed this detail too. Thanks for the pointer.<br><br></div><div>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?<br><br></div><div>Cheers,<br></div><div>Lawrence<br></div></div></div></div>