I noticed a crash in process_lingering_close

The crash happens here:

#0  0x00007f5d4af33b28 in apr_socket_timeout_set (sock=sock@entry=0x0, 
t=t@entry=2000000) at network_io/unix/sockopt.c:86
        stat = <optimized out>
#1  0x00000000004706a4 in process_lingering_close (cs=0x7f5c88000ed0) at 
event.c:1466
        csd = 0x0
        dummybuf = '\000' <repeats 30216 times>...
        nbytes = 0
        rv = <optimized out>
        q = <optimized out>
#2  0x000000000047154b in worker_thread (thd=0x1a31480, dummy=<optimized out>) 
at event.c:2142
        csd = 0x7f5c88000cd0
        cs = 0x0
        te = 0x0
        ptrans = 0x7f5c88000c58
        ti = <optimized out>
        process_slot = 3
        thread_slot = 1
        rv = 0
        is_idle = 0
#3  0x00007f5d4a48015a in start_thread () from /lib64/libpthread.so.0
No symbol table info available.
#4  0x00007f5d49fadf73 in clone () from /lib64/libc.so.6


The reason is that in this case a third party module that has hooked into the 
pre_connection hook returns an error which causes
the hook to stop running. This prevents core_pre_connection from running which 
is APR_HOOK_REALLY_LAST.
Hence the socket is not set into c->conn_config and hence ap_get_conn_socket 
returns NULL.
The quick fix that prevents that from happening is

Index: server/mpm/event/event.c
===================================================================
--- server/mpm/event/event.c    (revision 1893352)
+++ server/mpm/event/event.c    (working copy)
@@ -1040,6 +1040,16 @@
             ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, APLOGNO(00469)
                           "process_socket: connection aborted");
             c->aborted = 1;
+            /*
+             * In case we errored, the pre_connection hook of the core
+             * module maybe did not run (it is APR_HOOK_REALLY_LAST) and
+             * hence the socket might not have been set in c->conn_config
+             * and the lingering close process cannot fetch it from there
+             * via ap_get_conn_socket. Hence set it in this case.
+             */
+            if (!ap_get_conn_socket(c)) {
+                ap_set_core_module_config(c->conn_config, sock);
+            }
         }

         /**


But I think it is incomplete. I think we should do all that core_pre_connection 
does.
We cannot call it from event.c as it is a static in core.c. If we agree that we 
want to execute what it does I see two ways forward:

1. Make core_pre_connection part of the public API and have the pre_connection 
hook of core just call it.
2. Create a wrapper around ap_run_pre_connection as a public API and if 
ap_run_pre_connection returns != OK && != DONE do
things like setting c->aborted to 1 and call core_pre_connection or do a subset 
of it.

I would be in favour of 2. as 1. seems to provide an API function for only a 
very specific case and of limited use. 2. seems to
deliver a more "save" version of ap_run_pre_connection that could be used in 
several locations as a drop in replacement for
ap_run_pre_connection.

Regards

RĂ¼diger

Reply via email to