On Wednesday 25 January 2012, Jim Jagielski wrote:
> Looking over the code, impl as a hook seems more "isolated",
> rather than the current impl which is intrusive (which is
> part of what we're trying to avoid, aren't we?)

OK, patch is attached. This needs review/testing for Windows.
diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index 0d4de5c..0c8d4de 100644
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -389,12 +389,14 @@
  *                         private, add ap_create_core_ctx(),
  *                         ap_core_ctx_get_bb(), move core_net rec definition
  *                         to http_core.h
+ * 20120129.0 (2.5.0-dev)  Remove ap_create_core_ctx(), ap_core_ctx_get_bb();
+ *                         add insert_network_bucket hook
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
 
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
-#define MODULE_MAGIC_NUMBER_MAJOR 20120123
+#define MODULE_MAGIC_NUMBER_MAJOR 20120129
 #endif
 #define MODULE_MAGIC_NUMBER_MINOR 0                   /* 0...n */
 
diff --git a/include/http_core.h b/include/http_core.h
index 9332e2e..1cfd182 100644
--- a/include/http_core.h
+++ b/include/http_core.h
@@ -704,24 +704,17 @@ typedef struct core_net_rec {
 } core_net_rec;
 
 /**
- * Allocate and fill the core_ctx_t for the core input filter, but don't
- * create a bucket with the input socket.
- * Normally this is done automatically when the core input filter is called
- * for the first time, but MPMs or protocol modules that need to do special
- * socket setup can call this function to do the initialization earlier.
- * They must add the input socket bucket to the core input filter's bucket
- * brigade, see ap_core_ctx_get_bb().
- * @param c The conn_rec of the connection
- * @return The core_ctx_t to be stored in core_net_rec->in_ctx
- */
-AP_DECLARE(core_ctx_t *) ap_create_core_ctx(conn_rec *c);
-
-/**
- * Accessor for the core input filter's bucket brigade
- * @param c The core_ctx_t to get the brigade from
- * @return The bucket brigade
- */
-AP_DECLARE(apr_bucket_brigade *) ap_core_ctx_get_bb(core_ctx_t *ctx);
+ * Insert the network bucket into the core input filter's input brigade.
+ * This hook is intended for MPMs or protocol modules that need to do special
+ * socket setup.
+ * @param c The connection
+ * @param bb The brigade to insert the bucket into
+ * @param socket The socket to put into a bucket
+ * @return DECLINED if the current function does not handle this connection,
+ *         APR_SUCCESS or an error otherwise.
+ */
+AP_DECLARE_HOOK(apr_status_t, insert_network_bucket,
+                (conn_rec *c, apr_bucket_brigade *bb, apr_socket_t *socket))
 
 /* ----------------------------------------------------------------------
  *
diff --git a/server/core.c b/server/core.c
index eb8147b..e05534b 100644
--- a/server/core.c
+++ b/server/core.c
@@ -82,12 +82,18 @@
 
 APR_HOOK_STRUCT(
     APR_HOOK_LINK(get_mgmt_items)
+    APR_HOOK_LINK(insert_network_bucket)
 )
 
 AP_IMPLEMENT_HOOK_RUN_ALL(int, get_mgmt_items,
                           (apr_pool_t *p, const char *val, apr_hash_t *ht),
                           (p, val, ht), OK, DECLINED)
 
+AP_IMPLEMENT_HOOK_RUN_FIRST(apr_status_t, insert_network_bucket,
+                            (conn_rec *c, apr_bucket_brigade *bb,
+                             apr_socket_t *socket),
+                            (c, bb, socket), DECLINED)
+
 /* Server core module... This module provides support for really basic
  * server operations, including options and commands which control the
  * operation of other modules.  Consider this the bureaucracy module.
@@ -4729,6 +4735,15 @@ AP_DECLARE(apr_uint32_t) ap_random_pick(apr_uint32_t min, apr_uint32_t max)
     return number;
 }
 
+static apr_status_t core_insert_network_bucket(conn_rec *c,
+                                               apr_bucket_brigade *bb,
+                                               apr_socket_t *socket)
+{
+    apr_bucket *e = apr_bucket_socket_create(socket, c->bucket_alloc);
+    APR_BRIGADE_INSERT_TAIL(bb, e);
+    return APR_SUCCESS;
+}
+
 static void core_dump_config(apr_pool_t *p, server_rec *s)
 {
     core_server_config *sconf = ap_get_core_module_config(s->module_config);
@@ -4803,6 +4818,8 @@ static void register_hooks(apr_pool_t *p)
                       APR_HOOK_MIDDLE);
     ap_hook_pre_mpm(ap_create_scoreboard, NULL, NULL, APR_HOOK_MIDDLE);
     ap_hook_child_status(ap_core_child_status, NULL, NULL, APR_HOOK_MIDDLE);
+    ap_hook_insert_network_bucket(core_insert_network_bucket, NULL, NULL,
+                                  APR_HOOK_REALLY_LAST);
 
     /* register the core's insert_filter hook and register core-provided
      * filters
diff --git a/server/core_filters.c b/server/core_filters.c
index d8ff577..202f8d7 100644
--- a/server/core_filters.c
+++ b/server/core_filters.c
@@ -91,24 +91,10 @@ struct core_filter_ctx {
 };
 
 
-AP_DECLARE(core_ctx_t *) ap_create_core_ctx(conn_rec *c)
-{
-    core_ctx_t *ctx = apr_palloc(c->pool, sizeof(*ctx));
-    ctx->b = apr_brigade_create(c->pool, c->bucket_alloc);
-    ctx->tmpbb = apr_brigade_create(c->pool, c->bucket_alloc);
-    return ctx;
-}
-
-AP_DECLARE(apr_bucket_brigade *) ap_core_ctx_get_bb(core_ctx_t *ctx)
-{
-    return ctx->b;
-}
-
 int ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
                          ap_input_mode_t mode, apr_read_type_e block,
                          apr_off_t readbytes)
 {
-    apr_bucket *e;
     apr_status_t rv;
     core_net_rec *net = f->ctx;
     core_ctx_t *ctx = net->in_ctx;
@@ -131,10 +117,13 @@ int ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
 
     if (!ctx)
     {
-        net->in_ctx = ctx = ap_create_core_ctx(f->c);
+        net->in_ctx = ctx = apr_palloc(f->c->pool, sizeof(*ctx));
+        ctx->b = apr_brigade_create(f->c->pool, f->c->bucket_alloc);
+        ctx->tmpbb = apr_brigade_create(f->c->pool, f->c->bucket_alloc);
         /* seed the brigade with the client socket. */
-        e = apr_bucket_socket_create(net->client_socket, f->c->bucket_alloc);
-        APR_BRIGADE_INSERT_TAIL(ctx->b, e);
+        rv = ap_run_insert_network_bucket(f->c, ctx->b, net->client_socket);
+        if (rv != APR_SUCCESS)
+            return rv;
     }
     else if (APR_BRIGADE_EMPTY(ctx->b)) {
         return APR_EOF;
diff --git a/server/mpm/winnt/child.c b/server/mpm/winnt/child.c
index 30a525a..31af51a 100644
--- a/server/mpm/winnt/child.c
+++ b/server/mpm/winnt/child.c
@@ -736,6 +736,24 @@ static winnt_conn_ctx_t *winnt_get_connection(winnt_conn_ctx_t *context)
     return context;
 }
 
+static apr_status_t winnt_insert_network_bucket(conn_rec *c,
+                                                apr_bucket_brigade *bb,
+                                                apr_socket_t *socket)
+{
+    apr_bucket *e;
+    winnt_conn_ctx_t *context = ap_get_module_config(c->conn_config,
+                                                     &mpm_winnt_module);
+    if (context == NULL)
+        return DECLINED;
+
+    /* seed the brigade with AcceptEx read heap bucket */
+    e = context->overlapped.Pointer;
+    APR_BRIGADE_INSERT_HEAD(bb, e);
+    /* also seed the brigade with the client socket. */
+    e = apr_bucket_socket_create(socket, c->bucket_alloc);
+    APR_BRIGADE_INSERT_TAIL(bb, e);
+    return APR_SUCCESS;
+}
 
 /*
  * worker_main()
@@ -810,31 +828,9 @@ static DWORD __stdcall worker_main(void *thread_num_val)
         {
             apr_bucket_free(e);
         }
-        else if (e)
+        else
         {
-            core_net_rec *net;
-            ap_filter_t *filt;
-
-            filt = c->input_filters;
-            while ((strcmp(filt->frec->name, "core_in") != 0) && filt->next)
-                filt = filt->next;
-            net = filt->ctx;
-
-            if (net->in_ctx == NULL)
-            {
-                core_ctx_t *ctx = ap_create_core_ctx(c);
-                apr_bucket_brigade *bb = ap_core_ctx_get_bb(ctx);
-
-                /* seed the brigade with AcceptEx read heap bucket */
-                e = context->overlapped.Pointer;
-                APR_BRIGADE_INSERT_HEAD(bb, e);
-
-                /* also seed the brigade with the client socket. */
-                e = apr_bucket_socket_create(net->client_socket,
-                                             c->bucket_alloc);
-                APR_BRIGADE_INSERT_TAIL(bb, e);
-                net->in_ctx = ctx;
-            }
+            ap_set_module_config(c->conn_config, &mpm_winnt_module, context);
         }
 
         if (!c->aborted)
diff --git a/server/mpm/winnt/mpm_winnt.c b/server/mpm/winnt/mpm_winnt.c
index cdf7cac..4f6f8fe 100644
--- a/server/mpm/winnt/mpm_winnt.c
+++ b/server/mpm/winnt/mpm_winnt.c
@@ -1758,6 +1758,8 @@ static void winnt_hooks(apr_pool_t *p)
     ap_hook_mpm(winnt_run, NULL, NULL, APR_HOOK_MIDDLE);
     ap_hook_mpm_query(winnt_query, NULL, NULL, APR_HOOK_MIDDLE);
     ap_hook_mpm_get_name(winnt_get_name, NULL, NULL, APR_HOOK_MIDDLE);
+    ap_hook_insert_network_bucket(winnt_insert_network_bucket, NULL, NULL,
+                                  APR_HOOK_MIDDLE);
 }
 
 AP_DECLARE_MODULE(mpm_winnt) = {
diff --git a/server/mpm/winnt/mpm_winnt.h b/server/mpm/winnt/mpm_winnt.h
index 43adda9..f5833bc 100644
--- a/server/mpm/winnt/mpm_winnt.h
+++ b/server/mpm/winnt/mpm_winnt.h
@@ -67,6 +67,7 @@ void mpm_nt_eventlog_stderr_flush(void);
 
 /* From mpm_winnt.c: */
 
+extern module mpm_winnt_module;
 extern int ap_threads_per_child;
 
 extern DWORD my_pid;
@@ -92,6 +93,9 @@ void hold_console_open_on_error(void);
 
 /* From child.c: */
 void child_main(apr_pool_t *pconf);
+apr_status_t winnt_insert_network_bucket(conn_rec *c,
+                                         apr_bucket_brigade *bb,
+                                         apr_socket_t *socket);
 
 #endif /* APACHE_MPM_WINNT_H */
 /** @} */

Reply via email to