mod_firehose: Support portable non blocking support, and make non blocking

support optional.


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1220456 13f79535-47bb-0310-9956-ffa450edef68

docs/manual/mod/mod_firehose.xml

@@ -72,10 +72,12 @@
     silently stand down and not record anything, and the server will
     keep running as normal.</p>
 
-    <p>All file writes are non blocking, and buffer overflows will cause
+    <p>By default, all attempts to write will block the server. If the
-    debugging data to be lost. The module makes the call that the
+    webserver has been built against APR v2.0 or later, and an optional
-    reliable running of the server takes precedence over the recording
+    "nonblock" parameter is specified all file writes will be non
-    of firehose data.</p>
+    blocking, and buffer overflows will cause debugging data to be lost.
+    In this case it is possible to prioritise the running of the server
+    over the recording of firehose data.</p>
 
 </section>
 
@@ -144,7 +146,7 @@
 
 <name>FirehoseConnectionInput</name>
 <description>Capture traffic coming into the server on each connection</description>
-<syntax>FirehoseConnectionInput <var>filename</var></syntax>
+<syntax>FirehoseConnectionInput <var>[ block | nonblock ]</var> <var>filename</var></syntax>
 <default>none</default>
 <contextlist><context>server config</context></contextlist>
 <compatibility>FirehoseConnectionInput is only available in Apache 2.5.0 and
@@ -166,7 +168,7 @@ later.</compatibility>
 
 <name>FirehoseConnectionOutput</name>
 <description>Capture traffic going out of the server on each connection</description>
-<syntax>FirehoseConnectionOutput <var>filename</var></syntax>
+<syntax>FirehoseConnectionOutput <var>[ block | nonblock ]</var> <var>filename</var></syntax>
 <default>none</default>
 <contextlist><context>server config</context></contextlist>
 <compatibility>FirehoseConnectionOutput is only available in Apache 2.5.0 and
@@ -188,7 +190,7 @@ later.</compatibility>
 
 <name>FirehoseRequestInput</name>
 <description>Capture traffic coming into the server on each request</description>
-<syntax>FirehoseRequestInput <var>filename</var></syntax>
+<syntax>FirehoseRequestInput <var>[ block | nonblock ]</var> <var>filename</var></syntax>
 <default>none</default>
 <contextlist><context>server config</context></contextlist>
 <compatibility>FirehoseRequestInput is only available in Apache 2.5.0 and
@@ -209,7 +211,7 @@ later.</compatibility>
 
 <name>FirehoseRequestOutput</name>
 <description>Capture traffic going out of the server on each request</description>
-<syntax>FirehoseRequestOutput <var>filename</var></syntax>
+<syntax>FirehoseRequestOutput <var>[ block | nonblock ]</var> <var>filename</var></syntax>
 <default>none</default>
 <contextlist><context>server config</context></contextlist>
 <compatibility>FirehoseRequestOutput is only available in Apache 2.5.0 and
@@ -230,7 +232,7 @@ later.</compatibility>
 
 <name>FirehoseProxyConnectionInput</name>
 <description>Capture traffic coming into the back of mod_proxy</description>
-<syntax>FirehoseProxyConnectionInput <var>filename</var></syntax>
+<syntax>FirehoseProxyConnectionInput <var>[ block | nonblock ]</var> <var>filename</var></syntax>
 <default>none</default>
 <contextlist><context>server config</context></contextlist>
 <compatibility>FirehoseProxyConnectionInput is only available in Apache 2.5.0 and
@@ -250,7 +252,7 @@ later.</compatibility>
 
 <name>FirehoseProxyConnectionOutput</name>
 <description>Capture traffic sent out from the back of mod_proxy</description>
-<syntax>FirehoseProxyConnectionOutput <var>filename</var></syntax>
+<syntax>FirehoseProxyConnectionOutput <var>[ block | nonblock ]</var> <var>filename</var></syntax>
 <default>none</default>
 <contextlist><context>server config</context></contextlist>
 <compatibility>FirehoseProxyConnectionOutput is only available in Apache 2.5.0 and

modules/debugging/mod_firehose.c

@@ -78,6 +78,7 @@ typedef struct firehose_conn_t
     direction_enum direction;
     request_enum request;
     int suppress;
+    apr_int32_t nonblock;
 } firehose_conn_t;
 
 typedef struct firehose_conf_t
@@ -483,14 +484,9 @@ static int firehose_open_logs(apr_pool_t *p, apr_pool_t *plog,
 
         conn = (firehose_conn_t *) conf->firehoses->elts;
         for (i = 0; i < conf->firehoses->nelts; i++) {
-            /* TODO: make this non blocking behaviour optional, as APR doesn't yet
+            if (APR_SUCCESS != (rv = apr_file_open(&conn->file, conn->filename,
-             * support non blocking opening of files.
+                    APR_FOPEN_WRITE | APR_FOPEN_CREATE | APR_FOPEN_APPEND
-             * TODO: make this properly portable.
+                            | conn->nonblock, APR_OS_DEFAULT, plog))) {
-             */
-            apr_os_file_t file = open(conn->filename, O_WRONLY
-                    | O_CREAT | O_APPEND | O_NONBLOCK, 0777);
-            if (file < 0) {
-                rv = APR_FROM_OS_ERROR(apr_get_os_error());
                 ap_log_error(APLOG_MARK,
                         APLOG_WARNING,
                         rv, s, "mod_firehose: could not open '%s' for write, disabling firehose %s%s %s filter",
@@ -498,21 +494,6 @@ static int firehose_open_logs(apr_pool_t *p, apr_pool_t *plog,
                         conn->request == FIREHOSE_REQUEST ? " request" : "connection",
                         conn->direction == FIREHOSE_IN ? "input" : "output");
             }
-            else if (APR_SUCCESS != (rv = apr_os_file_put(
-                    &conn->file, &file, APR_FOPEN_WRITE
-                            | APR_FOPEN_CREATE | APR_FOPEN_APPEND, plog))) {
-                close(file);
-                ap_log_error(APLOG_MARK,
-                        APLOG_WARNING,
-                        rv, s, "mod_firehose: could not open '%s' for write, disabling firehose %s%s %s filter",
-                        conn->filename, conn->proxy == FIREHOSE_PROXY ? "proxy " : "",
-                        conn->request == FIREHOSE_REQUEST ? " request" : "connection",
-                        conn->direction == FIREHOSE_IN ? "input" : "output");
-            }
-            else {
-                apr_pool_cleanup_register(plog, conn->file,
-                        logs_cleanup, logs_cleanup);
-            }
             conn++;
         }
 
@@ -561,9 +542,11 @@ static void *firehose_merge_sconfig(apr_pool_t *p, void *basev,
     return cconf;
 }
 
-static void firehose_enable_connection(cmd_parms *cmd, const char *name,
+static const char *firehose_enable_connection(cmd_parms *cmd, const char *arg1,
-        proxy_enum proxy, direction_enum direction, request_enum request)
+        const char *arg2, proxy_enum proxy, direction_enum direction,
+        request_enum request)
 {
+    const char *name = arg2 ? arg2 : arg1;
 
     firehose_conn_t *firehose;
     firehose_conf_t
@@ -578,10 +561,31 @@ static void firehose_enable_connection(cmd_parms *cmd, const char *name,
     firehose->direction = direction;
     firehose->request = request;
 
+    if (arg2) {
+        if (!strcmp(arg1, "nonblock")) {
+#ifdef APR_FOPEN_NONBLOCK
+            firehose->nonblock = APR_FOPEN_NONBLOCK;
+#else
+            return "The parameter 'nonblock' is not supported by APR on this platform";
+#endif
+        }
+        else if (!strcmp(arg1, "block")) {
+            firehose->nonblock = 0;
+        }
+        else {
+            return apr_psprintf(cmd->pool,
+                    "The parameter '%s' should be 'block' or 'nonblock'", arg1);
+        }
+    }
+    else {
+        firehose->nonblock = 0;
+    }
+
+    return NULL;
 }
 
 static const char *firehose_enable_connection_input(cmd_parms *cmd,
-        void *dummy, const char *name)
+        void *dummy, const char *arg1, const char *arg2)
 {
 
     const char *err = ap_check_cmd_context(cmd, NOT_IN_DIR_LOC_FILE
@@ -590,14 +594,13 @@ static const char *firehose_enable_connection_input(cmd_parms *cmd,
         return err;
     }
 
-    firehose_enable_connection(cmd, name, FIREHOSE_NORMAL, FIREHOSE_IN,
+    return firehose_enable_connection(cmd, arg1, arg2, FIREHOSE_NORMAL,
-            FIREHOSE_CONNECTION);
+            FIREHOSE_IN, FIREHOSE_CONNECTION);
 
-    return NULL;
 }
 
 static const char *firehose_enable_connection_output(cmd_parms *cmd,
-        void *dummy, const char *name)
+        void *dummy, const char *arg1, const char *arg2)
 {
 
     const char *err = ap_check_cmd_context(cmd, NOT_IN_DIR_LOC_FILE
@@ -606,14 +609,13 @@ static const char *firehose_enable_connection_output(cmd_parms *cmd,
         return err;
     }
 
-    firehose_enable_connection(cmd, name, FIREHOSE_NORMAL, FIREHOSE_OUT,
+    return firehose_enable_connection(cmd, arg1, arg2, FIREHOSE_NORMAL,
-            FIREHOSE_CONNECTION);
+            FIREHOSE_OUT, FIREHOSE_CONNECTION);
 
-    return NULL;
 }
 
 static const char *firehose_enable_request_input(cmd_parms *cmd, void *dummy,
-        const char *name)
+        const char *arg1, const char *arg2)
 {
 
     const char *err = ap_check_cmd_context(cmd, NOT_IN_DIR_LOC_FILE
@@ -622,14 +624,13 @@ static const char *firehose_enable_request_input(cmd_parms *cmd, void *dummy,
         return err;
     }
 
-    firehose_enable_connection(cmd, name, FIREHOSE_NORMAL, FIREHOSE_IN,
+    return firehose_enable_connection(cmd, arg1, arg2, FIREHOSE_NORMAL,
-            FIREHOSE_REQUEST);
+            FIREHOSE_IN, FIREHOSE_REQUEST);
 
-    return NULL;
 }
 
 static const char *firehose_enable_request_output(cmd_parms *cmd, void *dummy,
-        const char *name)
+        const char *arg1, const char *arg2)
 {
 
     const char *err = ap_check_cmd_context(cmd, NOT_IN_DIR_LOC_FILE
@@ -638,14 +639,13 @@ static const char *firehose_enable_request_output(cmd_parms *cmd, void *dummy,
         return err;
     }
 
-    firehose_enable_connection(cmd, name, FIREHOSE_NORMAL, FIREHOSE_OUT,
+    return firehose_enable_connection(cmd, arg1, arg2, FIREHOSE_NORMAL,
-            FIREHOSE_REQUEST);
+            FIREHOSE_OUT, FIREHOSE_REQUEST);
 
-    return NULL;
 }
 
 static const char *firehose_enable_proxy_connection_input(cmd_parms *cmd,
-        void *dummy, const char *name)
+        void *dummy, const char *arg1, const char *arg2)
 {
 
     const char *err = ap_check_cmd_context(cmd, NOT_IN_DIR_LOC_FILE
@@ -654,14 +654,13 @@ static const char *firehose_enable_proxy_connection_input(cmd_parms *cmd,
         return err;
     }
 
-    firehose_enable_connection(cmd, name, FIREHOSE_PROXY, FIREHOSE_IN,
+    return firehose_enable_connection(cmd, arg1, arg2, FIREHOSE_PROXY,
-            FIREHOSE_CONNECTION);
+            FIREHOSE_IN, FIREHOSE_CONNECTION);
 
-    return NULL;
 }
 
 static const char *firehose_enable_proxy_connection_output(cmd_parms *cmd,
-        void *dummy, const char *name)
+        void *dummy, const char *arg1, const char *arg2)
 {
 
     const char *err = ap_check_cmd_context(cmd, NOT_IN_DIR_LOC_FILE
@@ -670,25 +669,24 @@ static const char *firehose_enable_proxy_connection_output(cmd_parms *cmd,
         return err;
     }
 
-    firehose_enable_connection(cmd, name, FIREHOSE_PROXY, FIREHOSE_OUT,
+    return firehose_enable_connection(cmd, arg1, arg2, FIREHOSE_PROXY,
-            FIREHOSE_CONNECTION);
+            FIREHOSE_OUT, FIREHOSE_CONNECTION);
 
-    return NULL;
 }
 
 static const command_rec firehose_cmds[] =
 {
-        AP_INIT_TAKE1("FirehoseConnectionInput", firehose_enable_connection_input, NULL,
+        AP_INIT_TAKE12("FirehoseConnectionInput", firehose_enable_connection_input, NULL,
                 RSRC_CONF, "Enable firehose on connection input data written to the given file/pipe"),
-        AP_INIT_TAKE1("FirehoseConnectionOutput", firehose_enable_connection_output, NULL,
+        AP_INIT_TAKE12("FirehoseConnectionOutput", firehose_enable_connection_output, NULL,
                 RSRC_CONF, "Enable firehose on connection output data written to the given file/pipe"),
-        AP_INIT_TAKE1("FirehoseRequestInput", firehose_enable_request_input, NULL,
+        AP_INIT_TAKE12("FirehoseRequestInput", firehose_enable_request_input, NULL,
                 RSRC_CONF, "Enable firehose on request input data written to the given file/pipe"),
-        AP_INIT_TAKE1("FirehoseRequestOutput", firehose_enable_request_output, NULL,
+        AP_INIT_TAKE12("FirehoseRequestOutput", firehose_enable_request_output, NULL,
                 RSRC_CONF, "Enable firehose on request output data written to the given file/pipe"),
-        AP_INIT_TAKE1("FirehoseProxyConnectionInput", firehose_enable_proxy_connection_input, NULL,
+        AP_INIT_TAKE12("FirehoseProxyConnectionInput", firehose_enable_proxy_connection_input, NULL,
                 RSRC_CONF, "Enable firehose on proxied connection input data written to the given file/pipe"),
-        AP_INIT_TAKE1("FirehoseProxyConnectionOutput", firehose_enable_proxy_connection_output, NULL,
+        AP_INIT_TAKE12("FirehoseProxyConnectionOutput", firehose_enable_proxy_connection_output, NULL,
                 RSRC_CONF, "Enable firehose on proxied connection output data written to the given file/pipe"),
         { NULL }
 };