Re: Segfaults in ConnectionHander (Possible Solution)

[Graham Dumpleton]

Graham Dumpleton wrote ..
> 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().

Extending the above code as:

    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;
    }

    /* Return empty string if no buckets. Can be caused by EAGAIN. */
    if (APR_BRIGADE_EMPTY(bb)) {
        return PyString_FromString("");
    }

seems to fix the problem. Ie., use call to APR_BRIGADE_EMPTY(bb) to check
whether any new buckets added and returning empty string if not.

Can someone else seeing this issue try this fix and see if the tests then
work.

Graham