> Am 21.09.2021 um 11:48 schrieb Ruediger Pluem <rpl...@apache.org>:
>
> 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.
+1 for approach 2
