Re: Segfaults in ConnectionHander

[Gregory (Grisha) Trubetskoy]

This may be a good question to post to dev@httpd.apache.org

Grisha

On Mon, 30 Jan 2006, Graham Dumpleton wrote:

Getting a bit closer now, have next part of puzzle worked out.

Graham Dumpleton wrote ..

This is starting to look really ugly.

In _conn_read(), it first creates a bucket brigade from the connection
objects pool object. No chance of this being destroyed prematurely
as a result.

    bb = apr_brigade_create(c->pool, c->bucket_alloc);

From what I understand, it then makes a call which links the bucket

brigade to the actual source of data.

    rc = ap_get_brigade(c->input_filters, bb, mode, APR_BLOCK_READ, bufsize);

Under normal circumstances this would also have the side effect of
performing the first actual read of data off the socket connection which
the client created to Apache.

When ap_get_brigade() is called, it is actually calling through to the
function core_input_filter() in Apache (server/core.c). In that function, it
ultimately hits the code:

       e = APR_BRIGADE_FIRST(ctx->b);
       rv = apr_bucket_read(e, &str, &len, block);

       if (APR_STATUS_IS_EAGAIN(rv)) {
           return APR_SUCCESS;
       }

Tracking down into apr_bucket_read() it ends up calling the function
socket_bucket_read() containg the code:

   *str = NULL;
   *len = APR_BUCKET_BUFF_SIZE;
   buf = apr_bucket_alloc(*len, a->list); /* XXX: check for failure? */

   rv = apr_socket_recv(p, buf, len);

   if (block == APR_NONBLOCK_READ) {
       apr_socket_timeout_set(p, timeout);
   }

   if (rv != APR_SUCCESS && rv != APR_EOF) {
       apr_bucket_free(buf);
       return rv;
   }

The apr_socket_recv() is what is doing the initial read of data from the
socket connection. This should block until the first data is received.

What is happening though is that it is returning -1 with errno set to
EAGAIN. Thus it frees the temporary bucket it created and returns
EAGAIN as the result.

If you note the code in the core_input_filter() it has:

       if (APR_STATUS_IS_EAGAIN(rv)) {
           return APR_SUCCESS;
       }

Thus, when EAGAIN is encountered, it simply returns success and does
not do anything else.

Returning back up to _conn_read() in mod_python source code, we have
where core_input_filter() was called ap_get_brigade():

   Py_BEGIN_ALLOW_THREADS;
   rc = ap_get_brigade(c->input_filters, bb, mode, APR_BLOCK_READ, bufsize);
   Py_END_ALLOW_THREADS;

   if (! APR_STATUS_IS_SUCCESS(rc)) {
       PyErr_SetObject(PyExc_IOError,
                       PyString_FromString("Connection read error"));
       return NULL;
   }

Since APR_SUCCESS was returned and assigned to "rc", no problem is detected.

The code which follows then assumes that the first bucket in the bucket
brigade actually contains valid data, when in fact the first bucket is actually
crap as nothing was done to set up a valid bucket since EAGAIN was returned.
As a consequence it crashes.

Thus in summary, _conn_read() doesn't cater in any way for the possibility
that the initial socket read may have failed because of EAGAIN and thus
the bucket is bogus. The problem is, how is it mean't to know this if the
value APR_SUCCESS is returned by ap_get_brigade().

At this point, seems a bit of research is needed of other examples of
connection handlers for Apache to see how they handle the initial startup
sequence and processing of initial data. What is in mod_python now does
not appear to be reliable in the face of an EAGAIN error occuring.

Graham