memdebug: fix realloc logging

Do the whole realloc and the subsequent logging under mutex lock. This
fixed log entries that state allocation a memory location before realloc
logs it as being freed.

Closes #19900

diff --git a/lib/memdebug.c b/lib/memdebug.c
index 991c9eb921c387b2e4c6771e163c5f6d0dce5989..05cd45a6f9bd724807e3ea19d374f956b916f62e 100644 (file)
--- a/lib/memdebug.c
+++ b/lib/memdebug.c
@@ -71,6 +71,29 @@ static bool dbg_mutex_init = 0;
 static curl_mutex_t dbg_mutex;
 #endif
 
+static bool curl_dbg_lock(void)
+{
+#if defined(USE_THREADS_POSIX) || defined(USE_THREADS_WIN32)
+  if(dbg_mutex_init) {
+    Curl_mutex_acquire(&dbg_mutex);
+    return TRUE;
+  }
+#endif
+  return FALSE;
+}
+
+static void curl_dbg_unlock(bool was_locked)
+{
+#if defined(USE_THREADS_POSIX) || defined(USE_THREADS_WIN32)
+  if(was_locked)
+    Curl_mutex_release(&dbg_mutex);
+#else
+  (void)was_locked;
+#endif
+}
+
+static void curl_dbg_log_locked(const char *format, ...) CURL_PRINTF(1, 2);
+
 /* LeakSantizier (LSAN) calls _exit() instead of exit() when a leak is detected
    on exit so the logfile must be closed explicitly or data could be lost.
    Though _exit() does not call atexit handlers such as this, LSAN's call to
@@ -295,6 +318,7 @@ void *curl_dbg_realloc(void *ptr, size_t wantedsize,
                        int line, const char *source)
 {
   struct memdebug *mem = NULL;
+  bool was_locked;
 
   size_t size = sizeof(struct memdebug) + wantedsize;
 
@@ -303,6 +327,10 @@ void *curl_dbg_realloc(void *ptr, size_t wantedsize,
   if(countcheck("realloc", line, source))
     return NULL;
 
+  /* need to realloc under lock, as we get out-of-order log
+   * entries otherwise, since another thread might alloc the
+   * memory released by realloc() before otherwise would log it. */
+  was_locked = curl_dbg_lock();
 #ifdef __INTEL_COMPILER
 #  pragma warning(push)
 #  pragma warning(disable:1684)
@@ -318,10 +346,11 @@ void *curl_dbg_realloc(void *ptr, size_t wantedsize,
 
   mem = (Curl_crealloc)(mem, size);
   if(source)
-    curl_dbg_log("MEM %s:%d realloc(%p, %zu) = %p\n",
-                source, line, (void *)ptr, wantedsize,
-                mem ? (void *)mem->mem : (void *)0);
+    curl_dbg_log_locked("MEM %s:%d realloc(%p, %zu) = %p\n",
+                        source, line, (void *)ptr, wantedsize,
+                        mem ? (void *)mem->mem : (void *)0);
 
+  curl_dbg_unlock(was_locked);
   if(mem) {
     mem->size = wantedsize;
     return mem->mem;
@@ -522,29 +551,18 @@ int curl_dbg_fclose(FILE *file, int line, const char *source)
   return res;
 }
 
-/* this does the writing to the memory tracking log file */
-void curl_dbg_log(const char *format, ...)
+static void curl_dbg_vlog(const char * const fmt,
+                          va_list ap) CURL_PRINTF(1, 0);
+
+static void curl_dbg_vlog(const char * const fmt, va_list ap)
 {
   char buf[1024];
-  size_t nchars;
-  va_list ap;
-
-  if(!curl_dbg_logfile)
-    return;
-
-  va_start(ap, format);
-  nchars = curl_mvsnprintf(buf, sizeof(buf), format, ap);
-  va_end(ap);
+  size_t nchars = curl_mvsnprintf(buf, sizeof(buf), fmt, ap);
 
   if(nchars > (int)sizeof(buf) - 1)
     nchars = (int)sizeof(buf) - 1;
 
   if(nchars > 0) {
-#if defined(USE_THREADS_POSIX) || defined(USE_THREADS_WIN32)
-    bool lock_mutex = dbg_mutex_init;
-    if(lock_mutex)
-      Curl_mutex_acquire(&dbg_mutex);
-#endif
     if(sizeof(membuf) - nchars < memwidx) {
       /* flush */
       fwrite(membuf, 1, memwidx, curl_dbg_logfile);
@@ -557,11 +575,35 @@ void curl_dbg_log(const char *format, ...)
     }
     memcpy(&membuf[memwidx], buf, nchars);
     memwidx += nchars;
-#if defined(USE_THREADS_POSIX) || defined(USE_THREADS_WIN32)
-    if(lock_mutex)
-      Curl_mutex_release(&dbg_mutex);
-#endif
   }
 }
 
+static void curl_dbg_log_locked(const char *format, ...)
+{
+  va_list ap;
+
+  if(!curl_dbg_logfile)
+    return;
+
+  va_start(ap, format);
+  curl_dbg_vlog(format, ap);
+  va_end(ap);
+}
+
+/* this does the writing to the memory tracking log file */
+void curl_dbg_log(const char *format, ...)
+{
+  bool was_locked;
+  va_list ap;
+
+  if(!curl_dbg_logfile)
+    return;
+
+  was_locked = curl_dbg_lock();
+  va_start(ap, format);
+  curl_dbg_vlog(format, ap);
+  va_end(ap);
+  curl_dbg_unlock(was_locked);
+}
+
 #endif /* CURLDEBUG */