mod_proxy,mpm_event: Replace ap_mpm_register_poll_callback*() by ap_mpm_poll_suspended() to avoid races.

Author: ylavic

include/ap_mmn.h

@@ -735,14 +735,18 @@
  *                         ap_check_output_pending()
  * 20211221.27 (2.5.1-dev) Add min_connection_timeout hook and
  *                         ap_get_connection_timeout()
+ * 20211221.28 (2.5.1-dev) Add ap_mpm_poll_suspended() and
+ *                         AP_MPMQ_CAN_POLL_SUSPENDED
+ * 20240701.0 (2.5.1-dev)  Axe ap_mpm_register_poll_callback and
+ *                         ap_mpm_register_poll_callback_timeout
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
 
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
-#define MODULE_MAGIC_NUMBER_MAJOR 20211221
+#define MODULE_MAGIC_NUMBER_MAJOR 20240701
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 27             /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 0      /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a

include/ap_mpm.h

@@ -184,6 +184,8 @@ AP_DECLARE(apr_status_t) ap_os_create_privileged_process(
 #define AP_MPMQ_CAN_POLL             18
 /** MPM supports CONN_STATE_ASYNC_WAITIO */
 #define AP_MPMQ_CAN_WAITIO           19
+/** MPM implements the poll_suspended hook */
+#define AP_MPMQ_CAN_POLL_SUSPENDED   20
 /** @} */
 
 /**
@@ -206,54 +208,13 @@ typedef void (ap_mpm_callback_fn_t)(void *baton);
 /* only added support in the Event MPM....  check for APR_ENOTIMPL */
 AP_DECLARE(apr_status_t) ap_mpm_resume_suspended(conn_rec *c);
 /* only added support in the Event MPM....  check for APR_ENOTIMPL */
+AP_DECLARE(apr_status_t) ap_mpm_poll_suspended(conn_rec *c, apr_pool_t *p,
+                                               const apr_array_header_t *pfds,
+                                               apr_interval_time_t timeout);
+/* only added support in the Event MPM....  check for APR_ENOTIMPL */
 AP_DECLARE(apr_status_t) ap_mpm_register_timed_callback(
         apr_time_t t, ap_mpm_callback_fn_t *cbfn, void *baton);
 
-/**
- * Register a callback on the readability or writability on a group of
- * sockets/pipes.
- * @param p Pool used by the MPM for its internal allocations
- * @param pfds Array of apr_pollfd_t
- * @param cbfn The callback function
- * @param baton userdata for the callback function
- * @return APR_SUCCESS if all sockets/pipes could be added to a pollset,
- * APR_ENOTIMPL if no asynch support, or an apr_pollset_add error.
- * @remark When activity is found on any 1 socket/pipe in the list, all are removed
- * from the pollset and only 1 callback is issued.
- * @remark The passed in pool can be cleared by cbfn and tofn when called back,
- *         it retains no MPM persistent data and won't be used until the next call
- *         to ap_mpm_register_poll_callback[_timeout].
- */
-
-AP_DECLARE(apr_status_t) ap_mpm_register_poll_callback(
-        apr_pool_t *p, const apr_array_header_t *pfds,
-        ap_mpm_callback_fn_t *cbfn, void *baton);
-
-/**
- * Register a callback on the readability or writability on a group of sockets/pipes,
- * with a timeout.
- * @param p Pool used by the MPM for its internal allocations
- * @param pfds Array of apr_pollfd_t
- * @param cbfn The callback function
- * @param tofn The callback function if the timeout expires
- * @param baton userdata for the callback function
- * @param timeout timeout for I/O in microseconds, unlimited if <= 0
- * @return APR_SUCCESS if all sockets/pipes could be added to a pollset,
- * APR_ENOTIMPL if no asynch support, or an apr_pollset_add error.
- * @remark When activity is found on any 1 socket/pipe in the list, all are removed
- * from the pollset and only 1 callback is issued. 
- * @remark For each call, only one of tofn or cbfn will be called, never both.
- * @remark The passed in pool can be cleared by cbfn and tofn when called back,
- *         it retains no MPM persistent data and won't be used until the next call
- *         to ap_mpm_register_poll_callback[_timeout].
- */
-
-AP_DECLARE(apr_status_t) ap_mpm_register_poll_callback_timeout(
-        apr_pool_t *p, const apr_array_header_t *pfds,
-        ap_mpm_callback_fn_t *cbfn, ap_mpm_callback_fn_t *tofn,
-        void *baton, apr_time_t timeout);
-
-
 typedef enum mpm_child_status {
     MPM_CHILD_STARTED,
     MPM_CHILD_EXITED,

include/httpd.h

@@ -1334,7 +1334,7 @@ typedef enum  {
     CONN_STATE_PROCESSING,          /* Processed by process_connection hooks */
     CONN_STATE_HANDLER,             /* Processed by the modules handlers */
     CONN_STATE_WRITE_COMPLETION,    /* Flushed by the MPM before entering CONN_STATE_KEEPALIVE */
-    CONN_STATE_SUSPENDED,           /* Suspended in the MPM until ap_run_resume_suspended() */
+    CONN_STATE_SUSPENDED,           /* Suspended from the MPM until ap_run_resume_suspended() */
     CONN_STATE_LINGER,              /* MPM flushes then closes the connection with lingering */
     CONN_STATE_LINGER_NORMAL,       /* MPM has started lingering close with normal timeout */
     CONN_STATE_LINGER_SHORT,        /* MPM has started lingering close with short timeout */

include/mpm_common.h

@@ -422,22 +422,12 @@ AP_DECLARE_HOOK(int, mpm_query, (int query_code, int *result, apr_status_t *rv))
 AP_DECLARE_HOOK(apr_status_t, mpm_register_timed_callback,
                 (apr_time_t t, ap_mpm_callback_fn_t *cbfn, void *baton))
 
-/**
- * register the specified callback
- * @ingroup hooks
- */
-AP_DECLARE_HOOK(apr_status_t, mpm_register_poll_callback,
-                (apr_pool_t *p, const apr_array_header_t *pds,
-                 ap_mpm_callback_fn_t *cbfn, void *baton))
-
-/* register the specified callback, with timeout 
+/** Put suspended connection's pollfds into the MPM's pollset
  * @ingroup hooks
- *
  */
-AP_DECLARE_HOOK(apr_status_t, mpm_register_poll_callback_timeout,
-                (apr_pool_t *p, const apr_array_header_t *pds,
-                ap_mpm_callback_fn_t *cbfn, ap_mpm_callback_fn_t *tofn,
-                void *baton, apr_time_t timeout))
+AP_DECLARE_HOOK(apr_status_t, mpm_poll_suspended,
+                (conn_rec *c, apr_pool_t *p, const apr_array_header_t *pfds,
+                 apr_interval_time_t timeout))
 
 /** Resume the suspended connection 
  * @ingroup hooks

modules/http/http_core.c

@@ -182,20 +182,22 @@ static int ap_process_http_async_connection(conn_rec *c)
                  * of nondeterministic failures later.
                  */
                 r = NULL;
-            }
 
-            if (cs->state != CONN_STATE_WRITE_COMPLETION &&
-                cs->state != CONN_STATE_SUSPENDED &&
-                cs->state != CONN_STATE_LINGER) {
-                /* Something went wrong; close the connection */
-                cs->state = CONN_STATE_LINGER;
+                switch (cs->state) {
+                case CONN_STATE_WRITE_COMPLETION:
+                case CONN_STATE_SUSPENDED:
+                case CONN_STATE_LINGER:
+                    return OK;
+                default:
+                    /* Unexpected, close */
+                    break;
+                }
             }
         }
-        else {   /* ap_read_request failed - client may have closed */
-            cs->state = CONN_STATE_LINGER;
-        }
     }
 
+    /* Something went wrong; close the connection */
+    cs->state = CONN_STATE_LINGER;
     return OK;
 }