Bug 60988 - Make the pre-exit pause duration configurable
Summary: Make the pre-exit pause duration configurable
Status: NEW
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: Core (show other bugs)
Version: 2.5-HEAD
Hardware: All All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords: PatchAvailable
Depends on:
Blocks:
 
Reported: 2017-04-13 19:44 UTC by Avi Blaszka
Modified: 2018-08-23 04:06 UTC (History)
0 users



Attachments
Patch (3.61 KB, patch)
2017-04-13 19:44 UTC, Avi Blaszka
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Avi Blaszka 2017-04-13 19:44:15 UTC
Created attachment 34912 [details]
Patch

When piped loggers are used, errors logged during module initialization (in the post-config phase) may not always be present in the error log when the httpd master process exits. This leads to something like "httpd exited with status 1" being written to stdout without any corresponding error messages to explain what went wrong. This occurs because there's a race condition between apache exiting and piped loggers flushing the log entries to disk.
  
Apache waits 10ms before exiting to account for this, but that may not always be enough. To fix this, this change makes the pre-exit pause duration configurable. The current 10ms duration is still used by default but the PRE_EXIT_PAUSE_TIME environment variable can be used to override that value. ap_timeout_parameter_parse is used to parse the environment variable contents allowing a flexible way to express the desired duration. If the value is too large (would lead to under/overflow), < 0, or otherwise invalid, it is ignored and the default value is used.

Git-generated patch (created via git-format-patch attached) from the tip of trunk is attached. Including the diff here as well:


--- httpd_current/server/main.c	2017-04-13 19:35:01.000000000 +0000
+++ httpd_new/server/main.c	2017-04-13 17:11:57.000000000 +0000
@@ -260,25 +260,49 @@
 #endif
 }
 
-#define TASK_SWITCH_SLEEP 10000
+#define DEFAULT_PRE_EXIT_PAUSE_TIME 10000
+
+/*
+ * Pauses before exiting by sleeping for a configurable amount of time,
+ * defaulting to DEFAULT_PRE_EXIT_PAUSE_TIME. The goal is to allow any in-flight
+ * log entries to be flushed to disk before exiting. Failure to do so can result
+ * in some of the entries being lost. This is of particular importance for piped
+ * loggers, which are outside of our control and can take an arbitrarily long
+ * time to complete. The default is intended to at least cause a context switch
+ * to give the other process(es) time. If that isn't enough, the
+ * PRE_EXIT_PAUSE_TIME environment variable can be used to configure a longer
+ * pause time.
+ */
+static void pre_exit_pause(void)
+{
+    apr_interval_time_t sleep_time;
+    const char *sleep_time_env;
+
+    sleep_time = DEFAULT_PRE_EXIT_PAUSE_TIME;
+    sleep_time_env = getenv("PRE_EXIT_PAUSE_TIME");
+
+    if (sleep_time_env) {
+        if (ap_timeout_parameter_parse(sleep_time_env, &sleep_time, "ms") != APR_SUCCESS) {
+            sleep_time = DEFAULT_PRE_EXIT_PAUSE_TIME;
+        }
+    }
+
+    if (sleep_time < 0) {
+        sleep_time = DEFAULT_PRE_EXIT_PAUSE_TIME;
+    }
+
+    apr_sleep(sleep_time);
+}
 
 static void destroy_and_exit_process(process_rec *process,
                                      int process_exit_value)
 {
-    /*
-     * Sleep for TASK_SWITCH_SLEEP micro seconds to cause a task switch on
-     * OS layer and thus give possibly started piped loggers a chance to
-     * process their input. Otherwise it is possible that they get killed
-     * by us before they can do so. In this case maybe valueable log messages
-     * might get lost.
-     */
-
     /* If we are to print an error, we need the name before we destroy pool.
      * short_name is a pointer into argv, so remains valid.
      */
     const char *name = process->short_name ? process->short_name : "httpd";
 
-    apr_sleep(TASK_SWITCH_SLEEP);
+    pre_exit_pause();
     ap_main_state = AP_SQ_MS_EXITING;
     apr_pool_destroy(process->pool); /* and destroy all descendent pools */
     apr_terminate();