Merge pull request #2495 from DataDog/glopes/appsec-log-fsync

Ensure logs are committed to disk upon shutdown
DataDog · Jan 30, 2024 · 2d896c8 · 2d896c8
2 parents 8beff3e + 7dad066
commit 2d896c8
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 11 deletions.
diff --git a/appsec/src/extension/logging.c b/appsec/src/extension/logging.c
@@ -377,9 +377,12 @@ static void _mlog_file(dd_log_level_t level, const char *format, va_list args,
             _dd_log_level_to_str(level), message_data, file, line, function);
 
     efree(message_data);
-    TSRM_MUTEX_LOCK(_mutex);
-    ssize_t written = write(_mlog_fd, data, data_len);
-    TSRM_MUTEX_UNLOCK(_mutex);
+    ssize_t written;
+    do {
+        TSRM_MUTEX_LOCK(_mutex);
+        written = write(_mlog_fd, data, data_len);
+        TSRM_MUTEX_UNLOCK(_mutex);
+    } while (written == -1 && errno == EINTR);
     efree(data);
 
     if (written == -1) {
@@ -451,13 +454,23 @@ void dd_log_shutdown()
 {
     mlog(dd_log_debug, "Shutting down the file logging");
 
-#ifdef ZTS
-    tsrm_mutex_free(_mutex);
-    _mutex = NULL;
-#endif
-    if (_log_strategy == log_use_file && _mlog_fd != -1 &&
-        _mlog_fd > fileno(stderr)) {
-        int ret = close(_mlog_fd);
+    if (_log_strategy == log_use_file &&
+        atomic_load_explicit(&_initialized, memory_order_acquire) &&
+        _mlog_fd != -1 && _mlog_fd > fileno(stderr)) {
+        // no need for the mutex at this point
+        // all the zts workers should have been done
+        // we're guaranteed visibility of the write to _mlog_fd by _initialized
+        int ret;
+        ret = fsync(_mlog_fd);
+        if (ret == -1) {
+            _mlog_php_varargs(dd_log_warning,
+                "Error fsyncing the log file (errno %d: %s)", errno,
+                _strerror(errno));
+        }
+        do {
+            ret = close(_mlog_fd);
+        } while (ret == -1 && errno == EINTR);
+
         if (ret == -1) {
             _mlog_php_varargs(dd_log_warning,
                 "Error closing the log file (errno %d: %s)", errno,
@@ -467,6 +480,11 @@ void dd_log_shutdown()
         closelog();
     }
 
+#ifdef ZTS
+    tsrm_mutex_free(_mutex);
+    _mutex = NULL;
+#endif
+
     _mlog_fd = -1;
     _log_strategy = log_use_nothing;
     atomic_store(&_initialized, false);

diff --git a/appsec/src/extension/request_abort.c b/appsec/src/extension/request_abort.c
@@ -409,8 +409,8 @@ static bool _abort_prelude()
         mlog_g(dd_log_warning, "Failed clearing current headers");
     }
 
-    mlog_g(dd_log_debug, "Discarding output buffers");
     php_output_discard_all();
+    mlog_g(dd_log_debug, "Output buffers have been discarded");
     return true;
 }