CPS-chained subrequests with I/O interceptions no longer work in nginx 0.8.21 (Was Re: Custom event + timer regressions caused by the new release)

[agentzh]

On Tue, Oct 27, 2009 at 9:41 PM, Maxim Dounin <mdounin at mdounin.ru> wrote:
> Could you please try to produce reduced code that triggers the issue?
>

Essentially, say, I have two subrequests A and B. And then:

1. I start subrequest A in my content handler this way:

    ngx_http_post_subrequest_t          *psr;

    psr->handler = post_subrequest_handler;
    psr->data = ctx;

    rc = ngx_http_subrequest(r, &location, &query_string, &sr, psr, 0);
    if (rc != NGX_OK) {
        return NGX_ERROR;
    }

    return NGX_OK;

2. In A's post_subrequest's handler, i.e., the
"post_subrequest_handler" function, issues subrequest B to a location,
say, /foo, that involves I/O operations, like this:

    location /foo {
       proxy_pass "http://bar.com/baz";
    }

Then after subrequests A and B are done, the whole connection hangs
forever and will never terminate (at least it seems so).

> I don't really have much time to investigate your module code,
> but I believe the problem is that you use subrequests and recall
> content handlers in mostly arbitrary way.
>  E.g. it looks like you
> call content handler again from post_subrequest handler.  This
> isn't supposed to work - they are normally called in quite a
> different contexts and have very different expectations.

I just reused my content handler's C function to avoid code
duplication in my "echo" module :) It could be a separate handler
function given above. The key issue is that whether the content
handler function and the post_subrequest handler has the same
semantics in their ngx_int_t retval, given that they happen to have
exactly the same prototype.

> Probably
> some changes in 0.8.21 rendered your code to be completely
> invalid.
>

I've just tracked down, yes, eventually, that it is the following line
of code newly added to 0.8.21 causes all my issues:

--- nginx-0.8.20/src/http/ngx_http_request.c	2009-10-02 19:30:47.000000000 +0800
+++ nginx-0.8.21/src/http/ngx_http_request.c	2009-10-22 17:48:42.000000000 +0800
@@ -2235,6 +2248,8 @@
	    ngx_log_debug2(NGX_LOG_DEBUG_HTTP, wev->log, 0,
	                   "http writer done: \"%V?%V\"", &r->uri, &r->args);
	
+	    r->write_event_handler = ngx_http_request_empty_handler;
+	
	    ngx_http_finalize_request(r, rc);
	}

By manually commenting out the write_event_handler assignment line,
*ALL* my tests are passing with nginx 0.8.21! :D

I have no idea regarding the reason behind that line's addition given
my limited knowledge of the nginx internals. Is there a way to
work-around it in nginx 0.8.21?

I'm guessing that nobody has done such things before, but I do believe
this is really useful for doing component-style programming in nginx,
because we don't have to rely on output filters (that are both costly
and *very* verbose to write) to hack in sequential subrequests.

Hopefully the nginx developers, especially Igor, will keep this trick
working in future versions or even bless it as a standard approach
rather than an evil hack or something.

Thanks!
-agentzh