In the module I’m developing, I have the possibility of encountering an
error after the response headers were already sent. As the headers were
already sent (with status 200) the only way for me to signal the error
to
the client would be to close the connection. I tried calling
ngx_http_finalize_request with both NGX_ERROR and NGX_HTTP_CLOSE and the
connection is not closed.
After debugging it, I found it has to do with the ‘if
(mr->write_event_handler)’ in ngx_http_terminate_request. I’m not sure
what
is the purpose of this if, but in my case write_event_handler has the
value
ngx_http_request_empty_handler, so the if evaluates to true and the
connection is not terminated. When I forcefully change
write_event_handler
to NULL with GDB, I see the connection is closed as expected.
I searched the code for ‘write_event_handler =’ and could not find a
single
place where this member gets a value of NULL (it always gets a pointer
to
some function).
Can anyone confirm if this is really a bug ? maybe the if should be
updated
to ‘if (mr->write_event_handler != ngx_http_request_empty_handler)’ ?
On Mon, Sep 01, 2014 at 05:25:56PM -0400, erankor2 wrote:
is the purpose of this if, but in my case write_event_handler has the value
ngx_http_request_empty_handler, so the if evaluates to true and the
connection is not terminated. When I forcefully change write_event_handler
to NULL with GDB, I see the connection is closed as expected.
I searched the code for ‘write_event_handler =’ and could not find a single
place where this member gets a value of NULL (it always gets a pointer to
some function).
The r->write_event_handler is set to NULL on initial creation of a
request. If write_event_handler is not NULL during a request
termination, nginx posts an event instead of freeing the request
request directly (this is done to avoid use-after-free when
processing posted subrequests). The request is still freed
though.
To clarify - the problem is not about freeing the request (I don’t think
there’s a resource leak here), the problem is that the connection is
just
left hanging until the client closes it / the server is restarted.
It is true that write_event_handler gets initialized to zero when the
request is allocated, but it is set to ngx_http_request_empty_handler
before
the first line of my code even runs. In ngx_http_core_content_phase
there’s:
where r->content_handler is the entry point to my code. So, unless I
explicitly reset it back to NULL (something that I never saw in any
other
nginx module) write_event_handler will not be null and the connection
will
be left hanging.
I forked some sample hello world module and modified it to reproduce the
problem, please check it out here:
In that code, I’m sending the response headers and then trigger a timer
for
1 second. In the timer callback I close the request with NGX_ERROR, but
the
connection remains active (I used a timer here since that’s the easiest
solution to defer the execution, in my real project I’m performing
asynchronous file I/O)
On Wed, Sep 03, 2014 at 10:10:01AM -0400, erankor2 wrote:
if (r->content_handler) {
I forked some sample hello world module and modified it to reproduce the
problem, please check it out here:
In that code, I’m sending the response headers and then trigger a timer for
1 second. In the timer callback I close the request with NGX_ERROR, but the
connection remains active (I used a timer here since that’s the easiest
solution to defer the execution, in my real project I’m performing
asynchronous file I/O)
The problem is that you use your own timer, and don’t run posted
requests after it - but the request termination code relies on
posted requests being run.
If you are using your own events, you should do processing similar
to ngx_http_request_handler(), see src/http/ngx_http_request.c.
Notably, you have to call the ngx_http_run_posted_requests()
function after the code which calls ngx_http_finalize_request().
In this particular case, trivial fix is to do something like:
On Thu, Sep 04, 2014 at 04:38:41PM -0400, erankor2 wrote:
it myself.
So my questions are:
Is there a simple rule that dictates when I should call
ngx_http_run_posted_requests and when I should not ?
I can work around the problem by doing something like ‘if called from the
aio callback call ngx_http_run_posted_requests’ otherwise, don’t call, but
that doesn’t feel like the correct solution.
You should call ngx_http_run_posted_requests() after low-level
events. That is, there is no need to call it after http-related
events, as ngx_http_run_posted_requests() will be called by
ngx_http_request_handler(). But if your own low-level events are
triggered, like the timer in your code, it’s your responsibility
to call ngx_http_request_handler() after them.
A simpler solution that seems to work for me, is to call
ngx_http_run_posted_requests only when I see that r->main->count is one
(before finalize). But I don’t know if that solution makes any sense - is
there any relation between these posted_requests and the request count ?
No, it doesn’t make sense to check r->main->count.
My understanding (before reading your reply :)) was that whenever I do
something asynchronous I must increment the count, whenever I call
finalize_request the count is decremented and once the count reaches zero
the request is terminated. So, what I’m missing is why the termination of
the request has to be deferred via posted requests - couldn’t
ngx_http_terminate_request just close the request only when it sees the
count reached zero instead ?
Termination on errors which you are doing isn’t expected to wait
for r->main->count to reach zero. It is expected to proactively
stop all ongoing activity by executing cleanup handlers, and then
free the request.
However, I bumped into a new problem, I use 2 different types of
asyncronous
operations in my code - file I/O and http requests. When I call
ngx_http_run_posted_requests from the aio callback it works well, but
when I
call it from the HTTP completion callback it actually makes the request
hang.
I can see that it calls ngx_http_upstream_check_broken_connection in
r->write_event_handler(r). I guess that in this case I should let
ngx_http_run_posted_requests run from the upstream module instead of
calling
it myself.
So my questions are:
Is there a simple rule that dictates when I should call
ngx_http_run_posted_requests and when I should not ?
I can work around the problem by doing something like ‘if called from
the
aio callback call ngx_http_run_posted_requests’ otherwise, don’t call,
but
that doesn’t feel like the correct solution.
A simpler solution that seems to work for me, is to call
ngx_http_run_posted_requests only when I see that r->main->count is one
(before finalize). But I don’t know if that solution makes any sense -
is
there any relation between these posted_requests and the request count ?
My understanding (before reading your reply :)) was that whenever I do
something asynchronous I must increment the count, whenever I call
finalize_request the count is decremented and once the count reaches
zero
the request is terminated. So, what I’m missing is why the termination
of
the request has to be deferred via posted requests - couldn’t
ngx_http_terminate_request just close the request only when it sees the
count reached zero instead ?
Thanks again for all your help, I really appreciate it !