From 38466221c79c08ae43ec9039dea28b875202b7cf Mon Sep 17 00:00:00 2001
From: Avi Blaszka <ablaszka@amazon.com>
Date: Thu, 13 Apr 2017 17:46:28 +0000
Subject: [PATCH] Make the pre-exit pause duration configurable.

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.
---
 server/main.c | 44 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/server/main.c b/server/main.c
index 351f0f1..fdd6c2a 100644
--- a/server/main.c
+++ b/server/main.c
@@ -260,25 +260,49 @@ static void show_compile_settings(void)
 #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();
-- 
2.7.3.AMZN