Re: Segfaults in ConnectionHander (Possible Solution)
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
Re: Segfaults in ConnectionHander (Possible Solution)
Graham Dumpleton wrote .. > 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. Okay, this may work, but the EAGAIN propogating backup as an empty string to Python can cause a tight loop to occur where calls are going out and back into Python code. This will occur until something is read or an error occurs. To avoid the back and forth, another option may be: while (APR_BRIGADE_EMPTY(bb)) { 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; } } What doesn't make sense to me is that on my Mac OS X box where this problem only occurs when you have two listener ports, even when you have already read some input from the connection, it tight loops with the lowest level read always returning EAGAIN. Ie., it doesn't block at all. Thus something really bad is happening on on Mac OS X. Unless Apache is setting some strange ioctl options on the socket to inadvertently cause this, it looks to me like Mac OS X is broken in some way. I am still on Mac OS X (10.3). I'll have to try it on my 10.4 box and see if it makes any difference. Graham
Re: Segfaults in ConnectionHander (Possible Solution)
Graham Dumpleton wrote: 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. Note that APR_STATUS_IS_SUCCESS has been removed from apr 1.x, which is one of the issues in getting mod_python to run in Apache 2.2. It looks like we should just check if rc != 0. This is according to discussion here featuring Greg Stein and Ryan Bloom: http://www.mail-archive.com/dev@httpd.apache.org/msg21757.html I'll update MODPYTHON-78 regarding Apache 2.2 with details on this and apr_sockaddr_port_get which has also been removed in apr 1.x. Jim
Re: Segfaults in ConnectionHander (Possible Solution)
On Mon, Jan 30, 2006 at 09:40:39PM -0500, Graham Dumpleton wrote: > Graham Dumpleton wrote .. > > 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. > > Okay, this may work, but the EAGAIN propogating backup as an empty > string to Python can cause a tight loop to occur where calls are going > out and back into Python code. This will occur until something is read > or an error occurs. > > To avoid the back and forth, another option may be: > > while (APR_BRIGADE_EMPTY(bb)) { > 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; > } > } > Graham, this code runs smoothly, i.e. no segfaults, all tests passed: FreeBSD 4.9: Apache/2.0.50 (prefork) Python/2.3.4 Apache/2.0.55 (prefork) Python/2.4.2 Thanks!
Re: Segfaults in ConnectionHander (Possible Solution)
Volodya wrote: On Mon, Jan 30, 2006 at 09:40:39PM -0500, Graham Dumpleton wrote: Graham Dumpleton wrote .. 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. Okay, this may work, but the EAGAIN propogating backup as an empty string to Python can cause a tight loop to occur where calls are going out and back into Python code. This will occur until something is read or an error occurs. To avoid the back and forth, another option may be: while (APR_BRIGADE_EMPTY(bb)) { 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; } } Graham, this code runs smoothly, i.e. no segfaults, all tests passed: FreeBSD 4.9: That's good news. I still wonder why we are seeing this problem in 3.2 and 3.1.4 though. Jim
Re: Segfaults in ConnectionHander (Possible Solution)
Jim Gallacher wrote: Volodya wrote: On Mon, Jan 30, 2006 at 09:40:39PM -0500, Graham Dumpleton wrote: Graham Dumpleton wrote .. 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. Okay, this may work, but the EAGAIN propogating backup as an empty string to Python can cause a tight loop to occur where calls are going out and back into Python code. This will occur until something is read or an error occurs. To avoid the back and forth, another option may be: while (APR_BRIGADE_EMPTY(bb)) { 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; } } Graham, this code runs smoothly, i.e. no segfaults, all tests passed: FreeBSD 4.9: That's good news. I still wonder why we are seeing this problem in 3.2 and 3.1.4 though. And what I meant to say was "and *NOT* in 3.1.4". Jim