Request constructor cannot call set_close_after_request because it has not been registered with Server yet

[Darren Warner]

The following code in Connection.read_record()...

         m_server.add_request(
            new Request(this, m_server.get_abort_func(), r));

...constructs a new Request before it is added to the server. However, the Request constructor calls Connection.set_close_after_request...

      if((m_flags & RequestFlags.KeepConnection) != RequestFlags.KeepConnection\
) {
        m_conn.set_close_after_request(m_id);

...which checks Server to see if the request exists...

      if(m_server.has_request(request_id)) {
 m_closing_after_finished_request_id = request_id;

... which it doesn't and an exception is thrown.

[Darren Warner (launchpad-dazwin)]

This problem gets a little complex. Moving the set_close_after_request() call into Connection (so that it can be registered after it exists in Server) still has the problem of when the request is created and destroyed within the call to Server.add_request(), which is the current case with Server.default_on_new_request(). The other solution is to remove the check in set_close_after_request() - this makes things work but there exists the potential for race conditions. Really, the only solution would be to make adding the new request and registering it in Connection an atomic operation - ugly as things currently stand.

However, I think the real solution is that list of current request id's need to be managed in Connection, not Server. This is especially true when you consider that different web servers may all be communicating with a single libvfcgi-based application server, so that there exists the very likely possibility that we need to handle duplicate request id's simultaneously (the FGCI spec also aludes to this - "...the application keeps track of the current state of each request ID on a _given_ transport connection...").

[Darren Warner (launchpad-dazwin)]

Committed a proposed fix to http://bazaar.launchpad.net/~launchpad-dazwin/libvfcgi/fixes/revision/34

[Michael B. Trausch (mtrausch)]

On Tue, Dec 15, 2009 at 11:45 PM, Darren Warner <email address hidden> wrote:
> This problem gets a little complex. Moving the set_close_after_request()
> call into Connection (so that it can be registered after it exists in
> Server) still has the problem of when the request is created and
> destroyed within the call to Server.add_request(), which is the current
> case with Server.default_on_new_request(). The other solution is to
> remove the check in set_close_after_request() - this makes things work
> but there exists the potential for race conditions. Really, the only
> solution would be to make adding the new request and registering it in
> Connection an atomic operation - ugly as things currently stand.
>
> However, I think the real solution is that list of current request id's
> need to be managed in Connection, not Server. This is especially true
> when you consider that different web servers may all be communicating
> with a single libvfcgi-based application server, so that there exists
> the very likely possibility that we need to handle duplicate request
> id's simultaneously (the FGCI spec also aludes to this - "...the
> application keeps track of the current state of each request ID on a
> _given_ transport connection...").

My understanding is that Apache doesn't do things that way, though it
has been a long time since I have checked out the behavior of Apache
against a FastCGI back-end process. However, Apache doesn't support
the multiplexing of requests on a single connection and therefore
every new connection that is a back-end for an Apache FastCGI
front-end would have its own request namespace, and that doesn't make
sense to me unless Apache holds open transport connections and itself
separates the namespace for each connection. The latter assertion I
don't believe I have tested, though.

My idea by keeping things in the FastCGI.Server object was to make it
clear that one FastCGI.Server represents a single request namespace.
However, the more that I think about that, the more I think that
notion may be just as flawed as the method by which I understand
Apache to work (and thus my recollection is quite likely to be in
error, otherwise Apache is simply unsuitable for use in a production
FastCGI environment).

The good news is that libvfcgi isn't promising API or ABI stability
for a while yet, so if any of this is wrong it can be changed later.