Hi,

So far, I really like mod_python -- it's a great piece of
software. But I think I've found a small bug.

I've noticed that adding a handler using add_handler prompts apache to
segfault if no statically registered (e.g., specified in the apache
conf files) handlers of the same category (e.g., PythonLogHandler,
PythonTransHandler) are present. I've tested this in both the release
version (314) and version 325b (albeit more thoroughly in the former).

Here's how I triggered the bug:

Add this to apache2.conf:
PythonPath "sys.path+[(the path goes here)]"
PythonTransHandler crashbug

crashbug.py contains:

from mod_python import apache

def transhandler(req):
req.add_handler("PythonLogHandler","foo::loghandler")
return apache.OK

foo contains:

from mod_python import apache

def loghandler(req):
import sys
sys.stderr.write("foo: saw request type:" + req.method + "\n")
return apache.OK

If I run apache

sudo   /usr/sbin/apache2 -X

and tickle the server:

echo -e "GET / HTTP/1.0\n\n" | nc localhost 8080

...apache segfaults.

On the other hand, if I add the line

PythonLogHandler notfoo

to apache2.conf, where notfoo.py is identical to foo except that
"foo:" is replaced with "notfoo", I get the expected two lines in the
log, and no crash.

Here's the trace:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread -1214269216 (LWP 21283)]
0xb76dddb3 in hlist_copy (p=0x8158fa8, hle=0x0) at hlist.c:91
91          head->handler = apr_pstrdup(p, hle->handler);
(gdb) bt
#0  0xb76dddb3 in hlist_copy (p=0x8158fa8, hle=0x0) at hlist.c:91
#1  0xb76dd9aa in MpHList_FromHLEntry (hle=0x0) at hlistobject.c:49
#2  0xb76e59b6 in python_handler (req=0x8238480, phase=0xb76e804c "PythonLog
Handler") at mod_python.c:1029
#3  0x08086086 in ap_run_log_transaction ()
#4  0x0806963e in ap_process_request ()
#5  0x08064b19 in _start ()


I suspect the problem is in this part of mod_python.c:

Here's my understanding (please correct me if I'm wrong).  If hle is
NULL (i.e., there were no handlers of this type registered in the
apache conf), but dynhle is not null (we registered a dynamic
handler), we continue processing...

if (! (hle || dynhle)) {
/* nothing to do here */
return DECLINED;
}

/* determine interpreter to use */
interp_name = select_interp_name(req, NULL, conf, hle, NULL, 0);

/* get/create interpreter */
idata = get_interpreter(interp_name, req->server);

if (!idata)
return HTTP_INTERNAL_SERVER_ERROR;

/* create/acquire request object */
request_obj = get_request_object(req, interp_name, phase);

/* remember the extension if any. used by publisher */
if (ext)
request_obj->extension = apr_pstrdup(req->pool, ext);

But then, we try to create an MpHList from hle, which is NULL -- prompting a
 crash a few levels down.

/* create a hahdler list object */
request_obj->hlo = (hlistobject *)MpHList_FromHLEntry(hle);


Here's how I fixed it:

--- /home/jdb/dl/mpsvn/mod_python-3.1.4/src/mod_python.c	2005-01-29 16:25:28
.000000000 -0500
+++ mod_python.c	2006-01-04 16:59:13.000000000 -0500
@@ -1026,13 +1026,18 @@
request_obj->extension = apr_pstrdup(req->pool, ext);

/* create a hahdler list object */
-    request_obj->hlo = (hlistobject *)MpHList_FromHLEntry(hle);
-
-    /* add dynamically registered handlers, if any */
-    if (dynhle) {
+    if (hle) {
+      request_obj->hlo = (hlistobject *)MpHList_FromHLEntry(hle);
+      /* add dynamically registered handlers, if any */
+      if (dynhle) {
MpHList_Append(request_obj->hlo, dynhle);
+      }
}
-
+    else {
+      /* hle was NULL; so create hlo from dynhle */
+      request_obj->hlo = (hlistobject *)MpHList_FromHLEntry(dynhle);
+    }
+
/*
* Here is where we call into Python!
* This is the C equivalent of

AFAIK, this works -- but I'm new at this.

best regards,

Joe

[ Post a follow-up to this message ]

You are a few weeks too late. :-)

See:

http://issues.apache.org/jira/browse/MODPYTHON-98

There are whole bunch of little issues with adding handlers using
req.add_handler(). For this one you seem to have tread much the
same path as I did.

Graham

Joseph Barillari wrote ..
> Hi,
>
> So far, I really like mod_python -- it's a great piece of
> software. But I think I've found a small bug.
>
> I've noticed that adding a handler using add_handler prompts apache to
> segfault if no statically registered (e.g., specified in the apache
> conf files) handlers of the same category (e.g., PythonLogHandler,
> PythonTransHandler) are present. I've tested this in both the release
> version (314) and version 325b (albeit more thoroughly in the former).
>
> Here's how I triggered the bug:
>
> Add this to apache2.conf:
> PythonPath "sys.path+[(the path goes here)]"
> PythonTransHandler crashbug
>
> crashbug.py contains:
>
> from mod_python import apache
>
> def transhandler(req):
>     req.add_handler("PythonLogHandler","foo::loghandler")
>     return apache.OK
>
> foo contains:
>
> from mod_python import apache
>
> def loghandler(req):
>     import sys
>     sys.stderr.write("foo: saw request type:" + req.method + "\n")
>     return apache.OK
>
> If I run apache
>
> sudo   /usr/sbin/apache2 -X
>
> and tickle the server:
>
> echo -e "GET / HTTP/1.0\n\n" | nc localhost 8080
>
> ...apache segfaults.
>
> On the other hand, if I add the line
>
> PythonLogHandler notfoo
>
> to apache2.conf, where notfoo.py is identical to foo except that
> "foo:" is replaced with "notfoo", I get the expected two lines in the
> log, and no crash.
>
> Here's the trace:
>
> Program received signal SIGSEGV, Segmentation fault.
> [Switching to Thread -1214269216 (LWP 21283)]
> 0xb76dddb3 in hlist_copy (p=0x8158fa8, hle=0x0) at hlist.c:91
> 91          head->handler = apr_pstrdup(p, hle->handler);
> (gdb) bt
> #0  0xb76dddb3 in hlist_copy (p=0x8158fa8, hle=0x0) at hlist.c:91
> #1  0xb76dd9aa in MpHList_FromHLEntry (hle=0x0) at hlistobject.c:49
> #2  0xb76e59b6 in python_handler (req=0x8238480, phase=0xb76e804c "PythonL
ogHandler")
> at mod_python.c:1029
> #3  0x08086086 in ap_run_log_transaction ()
> #4  0x0806963e in ap_process_request ()
> #5  0x08064b19 in _start ()
>
>
> I suspect the problem is in this part of mod_python.c:
>
> Here's my understanding (please correct me if I'm wrong).  If hle is
> NULL (i.e., there were no handlers of this type registered in the
> apache conf), but dynhle is not null (we registered a dynamic
> handler), we continue processing...
>
>     if (! (hle || dynhle)) {
>         /* nothing to do here */
>         return DECLINED;
>     }
>
>     /* determine interpreter to use */
>     interp_name = select_interp_name(req, NULL, conf, hle, NULL, 0);
>
>     /* get/create interpreter */
>     idata = get_interpreter(interp_name, req->server);
>
>     if (!idata)
>         return HTTP_INTERNAL_SERVER_ERROR;
>
>     /* create/acquire request object */
>     request_obj = get_request_object(req, interp_name, phase);
>
>     /* remember the extension if any. used by publisher */
>     if (ext)
>         request_obj->extension = apr_pstrdup(req->pool, ext);
>
> But then, we try to create an MpHList from hle, which is NULL -- prompting
> a crash a few levels down.
>
>     /* create a hahdler list object */
>     request_obj->hlo = (hlistobject *)MpHList_FromHLEntry(hle);
>
>
> Here's how I fixed it:
>
> --- /home/jdb/dl/mpsvn/mod_python-3.1.4/src/mod_python.c	2005-01-29 16:25:
28.000000000
> -0500
> +++ mod_python.c	2006-01-04 16:59:13.000000000 -0500
> @@ -1026,13 +1026,18 @@
>          request_obj->extension = apr_pstrdup(req->pool, ext);
>
>      /* create a hahdler list object */
> -    request_obj->hlo = (hlistobject *)MpHList_FromHLEntry(hle);
> -
> -    /* add dynamically registered handlers, if any */
> -    if (dynhle) {
> +    if (hle) {
> +      request_obj->hlo = (hlistobject *)MpHList_FromHLEntry(hle);
> +      /* add dynamically registered handlers, if any */
> +      if (dynhle) {
>          MpHList_Append(request_obj->hlo, dynhle);
> +      }
>      }
> -
> +    else {
> +      /* hle was NULL; so create hlo from dynhle */
> +      request_obj->hlo = (hlistobject *)MpHList_FromHLEntry(dynhle);
> +    }
> +
>      /*
>       * Here is where we call into Python!
>       * This is the C equivalent of
>
> AFAIK, this works -- but I'm new at this.
>
> best regards,
>
> Joe

[ Post a follow-up to this message ]