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