Re: Segfaults in ConnectionHander (Possible Solution)

2006-01-30 Thread 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


Re: Segfaults in ConnectionHander (Possible Solution)

2006-01-30 Thread Graham Dumpleton
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)

2006-01-30 Thread Jim Gallacher

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)

2006-01-31 Thread Volodya
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)

2006-01-31 Thread Jim Gallacher

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)

2006-01-31 Thread Jim Gallacher

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