From e2b14d6d94808ee9b305c83b3b9940a47e917109 Mon Sep 17 00:00:00 2001
From: K Pamnany <kpamnany@users.noreply.github.com>
Date: Fri, 24 May 2024 12:18:06 -0400
Subject: [PATCH] Refactor JSON printing code

Concurrent `write()` calls to the same file descriptor should be
thread-safe, but can result in interleaving. Refactor the JSON
printing code used from `jl_safe_printf()` to assemble the message
into a buffer and use a single `write()` call.
---
 src/jl_uv.c | 71 ++++++++++++++++++++++++++++++++---------------------
 1 file changed, 43 insertions(+), 28 deletions(-)

diff --git a/src/jl_uv.c b/src/jl_uv.c
index d7c0fac5a9bfa9..cd47d979648256 100644
--- a/src/jl_uv.c
+++ b/src/jl_uv.c
@@ -678,56 +678,71 @@ JL_DLLEXPORT int jl_printf(uv_stream_t *s, const char *format, ...)
     return c;
 }
 
-STATIC_INLINE void print_error_msg_as_json(char *buf) JL_NOTSAFEPOINT
+STATIC_INLINE int copystp(char *dest, const char *src)
 {
-    // Our telemetry on SPCS expects a JSON object per line
-    // The following lines prepare the timestamp string and the JSON object
+    char *d = stpcpy(dest, src);
+    return (int)(d - dest);
+}
+
+// RAI-specific
+STATIC_INLINE void write_to_safe_crash_log(char *buf) JL_NOTSAFEPOINT
+{
+    // Our telemetry on SPCS expects a JSON object per line.
+    // We ignore write failures because there is nothing we can do.
+    int buflen = strlen(buf);
+    assert(buflen <= 4096);
+    char wbuf[4096];
+    bzero(wbuf, 4096);
+    int wlen = 0;
+
+    // JSON preamble
+    wlen += copystp(&wbuf[wlen], "\n{\"level\":\"Error\", \"timestamp\":\"");
+
+    // Timestamp
     struct timeval tv;
     struct tm* tm_info;
-    char timestamp_buffer[50];
-    // Get current time
     gettimeofday(&tv, NULL);
     tm_info = gmtime(&tv.tv_sec);
-    // Format time
-    int offset = strftime(timestamp_buffer, 25, "%Y-%m-%dT%H:%M:%S", tm_info);
-    // Append milliseconds
-    snprintf(timestamp_buffer + offset, 25, ".%03d", tv.tv_usec / 1000);
-    const char *json_preamble_p1 = "\n{\"level\":\"Error\", \"timestamp\":\"";
-    const char *json_preamble_p2 = "\", \"message\": \"";
-    const char *json_postamble = "\"}\n";
-    // Ignore write failures because there is nothing we can do
-    write(jl_sig_fd, json_preamble_p1, strlen(json_preamble_p1));
-    write(jl_sig_fd, timestamp_buffer, strlen(timestamp_buffer));
-    write(jl_sig_fd, json_preamble_p2, strlen(json_preamble_p2));
-    // JSON escape the input string
-    for(size_t i = 0; i < strlen(buf); i += 1) {
+    wlen += strftime(&wbuf[wlen], 42, "%Y-%m-%dT%H:%M:%S", tm_info);
+    sprintf(&wbuf[wlen], ".%03ld", (long)tv.tv_usec / 1000);
+    wlen += 4;
+
+    // JSON preamble to message
+    wlen += copystp(&wbuf[wlen], "\", \"message\": \"");
+
+    // Message
+    for (size_t i = 0; i < buflen; i++) {
         switch (buf[i]) {
             case '"':
-                write(jl_sig_fd, "\\\"", 2);
+                wlen += copystp(&wbuf[wlen], "\\\"");
                 break;
             case '\b':
-                write(jl_sig_fd, "\\b", 2);
+                wlen += copystp(&wbuf[wlen], "\\b");
                 break;
             case '\n':
-                write(jl_sig_fd, "\\n", 2);
+                wlen += copystp(&wbuf[wlen], "\\n");
                 break;
             case '\r':
-                write(jl_sig_fd, "\\r", 2);
+                wlen += copystp(&wbuf[wlen], "\\r");
                 break;
             case '\t':
-                write(jl_sig_fd, "\\t", 2);
+                wlen += copystp(&wbuf[wlen], "\\t");
                 break;
             case '\\':
-                write(jl_sig_fd, "\\\\", 2);
+                wlen += copystp(&wbuf[wlen], "\\\\");
                 break;
             default:
-                write(jl_sig_fd, buf + i, 1);
+                wbuf[wlen++] = buf[i];
+                break;
         }
     }
-    write(jl_sig_fd, json_postamble, strlen(json_postamble));
+    wlen += copystp(&wbuf[wlen], "\"}\n");
+    write(jl_sig_fd, wbuf, wlen);
     fdatasync(jl_sig_fd);
 }
 
+extern int jl_inside_heartbeat_thread(void);
+
 JL_DLLEXPORT void jl_safe_printf(const char *fmt, ...)
 {
     static char buf[1000];
@@ -747,8 +762,8 @@ JL_DLLEXPORT void jl_safe_printf(const char *fmt, ...)
     // order is important here: we want to ensure that the threading infra
     // has been initialized before we start trying to print to the
     // safe crash log file
-    if (jl_sig_fd != 0 && (jl_inside_signal_handler() || jl_inside_heartbeat_thread()) {
-        print_error_msg_as_json(buf);
+    if (jl_sig_fd != 0 && (jl_inside_signal_handler() || jl_inside_heartbeat_thread())) {
+        write_to_safe_crash_log(buf);
     }
     if (write(STDERR_FILENO, buf, strlen(buf)) < 0) {
         // nothing we can do; ignore the failure