Fix a use-after-free on the pipeline banner message

src/control/control.c

@@ -521,7 +521,11 @@ void dt_control_draw_busy_msg(cairo_t *cr, int width, int height)
   pango_font_description_set_weight(desc, PANGO_WEIGHT_BOLD);
   layout = pango_cairo_create_layout(cr);
   pango_layout_set_font_description(layout, desc);
-  pango_layout_set_text(layout, darktable.main_message ? darktable.main_message : _("Working..."), -1);
+  // A COPY: a worker thread frees this string on the next dt_set_main_message(), which the
+  // pipeline issues once per module per frame while the darkroom renders.
+  char *const message = dt_get_main_message_copy();
+  pango_layout_set_text(layout, !IS_NULL_PTR(message) ? message : _("Working..."), -1);
+  dt_free(message);
   pango_layout_get_pixel_extents(layout, &ink, NULL);
   if(ink.width > width * 0.98)
   {

src/control/control.h

@@ -92,10 +92,13 @@ void *dt_control_expose(void *voidptr);
 void dt_control_button_pressed(double x, double y, double pressure, int which, int type, uint32_t state);
 
 /** Message painted over the main preview while the pipeline is working. Written by the
- * pipeline (develop/pixelpipe_hb.c), rendered by control/control.c; the storage belongs
- * to the orchestrator. dt_set_main_message() TAKES OWNERSHIP of `message` and frees the
- * previous one; pass NULL to clear. */
-const char *dt_get_main_message(void);
+ * pipeline (develop/pixelpipe_hb.c) from WORKER threads, rendered by control/control.c on the
+ * GUI thread; the storage belongs to the orchestrator and carries its own lock.
+ *
+ * dt_set_main_message() TAKES OWNERSHIP of `message` and frees the previous one; pass NULL to
+ * clear. The reader gets a COPY it must free -- there is no safe way to lend the pointer, since
+ * the next write frees it and the writer is on another thread. */
+char *dt_get_main_message_copy(void);
 void dt_set_main_message(char *message);
 void dt_control_button_released(double x, double y, int which, uint32_t state);
 void dt_control_mouse_moved(double x, double y, double pressure, int which);

src/darktable.c

@@ -566,15 +566,41 @@ void dt_gui_set_themes(GList *themes)
   darktable.themes = themes;
 }
 
-const char *dt_get_main_message(void)
-{
-  return darktable.main_message;
+/* The string is written by pipeline worker threads and read by the GUI thread painting the
+ * banner, so it carries its own lock rather than borrowing control->log_mutex: the borrowed
+ * one was taken by the single writer at its CALL SITE, which left the invariant unenforceable
+ * -- and left the reader, dt_control_draw_busy_msg(), holding no lock at all while a worker
+ * dt_free()d the very pointer it had handed to pango. That fired once per module per frame
+ * whenever the darkroom was rendering.
+ *
+ * A private lock also keeps this independent of dt_control_t's lifetime, which is freed at
+ * darktable.c's teardown while this string is not. */
+/* Statically initialised on purpose, and correct in the _DEBUG build too, where
+ * dt_pthread_mutex_t carries ~1.8 kB of instrumentation after the pthread_mutex_t. A
+ * brace-enclosed initialiser with fewer initialisers than members zero-fills the remainder
+ * (C11 6.7.9p21), and this object has static storage duration anyway, so it lives in .bss --
+ * measured: 0 of the 1864 trailing bytes non-zero before first use. That is the same state
+ * dt_pthread_mutex_init() leaves (it memsets), minus only the `name' field, which
+ * dt_pthread_mutex_lock() snprintf()s over before the one place it reads it.
+ *
+ * Static rather than initialised in dt_init() because this string is written from worker
+ * threads: a lock that is valid from program start has no window in which it is not. */
+static dt_pthread_mutex_t _main_message_lock = { PTHREAD_MUTEX_INITIALIZER };
+
+char *dt_get_main_message_copy(void)
+{
+  dt_pthread_mutex_lock(&_main_message_lock);
+  char *const copy = !IS_NULL_PTR(darktable.main_message) ? g_strdup(darktable.main_message) : NULL;
+  dt_pthread_mutex_unlock(&_main_message_lock);
+  return copy;
 }
 
 void dt_set_main_message(char *message)
 {
+  dt_pthread_mutex_lock(&_main_message_lock);
   dt_free(darktable.main_message);
   darktable.main_message = message;
+  dt_pthread_mutex_unlock(&_main_message_lock);
 }
 
 struct dt_view_manager_t *dt_view_manager_get_global(void)
@@ -749,10 +775,9 @@ static void _metadata_notify(const dt_metadata_notice_t kind, const char *messag
  * under log_mutex and the centre redraw are both worker-safe. */
 static void _pipeline_busy(const char *message_or_null)
 {
-  dt_control_t *const control = dt_control_get_global();
-  dt_pthread_mutex_lock(&control->log_mutex);
+  // dt_set_main_message() takes the string's own lock. Do NOT reintroduce an outer
+  // control->log_mutex here: it guards the message LOG, not this, and it bought nothing.
   dt_set_main_message(message_or_null ? g_strdup(message_or_null) : NULL);
-  dt_pthread_mutex_unlock(&control->log_mutex);
   dt_control_queue_redraw_center();
 }