darktable-org / darktable-org/darktable

overlay.c calls GTK functions from a pixelpipe worker thread

Open
#21,915 2 comments 0 reactions 1 assignee View on GitHub

@kofa73 is already working on this.

Since Aug 19, 2026.

Dominant language
C
Stars
13.1k
Forks
1.4k
Avg merge
22h 14m
Merged PRs (30d)
198

Description

Found by Claude + Codex via code analysis, no known crash reported.

Updated 2026-08-23 during the gui_data threading dev-doc review: same defect,
restructured, with the OpenCL path, the trigger sequence and the environment added. The
description below is no longer the version that note refers to. The material that is new
relative to #21915 — including an answer to @ralfbrown's question about whether
gtk_widget_queue_draw() is thread-safe — is drafted as a comment in
comment-21915-update.md, not yet posted.

Description

overlay builds its overlay image in _setup_overlay(), which runs on a pixelpipe
worker thread: it is called from process() and process_cl(). That function reaches
into the module's GUI data and does three things a worker thread must not do.

  1. It calls GTK directly — a widget redraw and two tooltip changes on g->area.
    GTK must only be called from the main (GTK) thread, in both GTK3 and GTK4.
  2. It writes module parameters and adds a history itemp->imgid and
    dt_dev_add_history_item(), when a missing overlay image is found again by path.
  3. It leaks the tooltip stringgtk_widget_set_tooltip_text() copies the text,
    and the g_strdup_printf() result is never freed.

The GTK calls are not confined to an error path: the success branch sets a tooltip on
every rebuild. The module's own overlay_threadsafe mutex does not help — it
serialises the two pipe threads against each other, and no GTK-side code takes it, so
nothing orders these calls against the main loop.

Affects

Both the CPU and the OpenCL path, because both call the same helpers:
process()_get_overlay_argb() / _get_overlay_rgba_f()_setup_overlay(), and
process_cl() → the same two helpers. Not specific to either backend.

Steps to reproduce

The GTK calls happen whenever the cached overlay buffer is empty and has to be rebuilt.
The cache is cleared when the overlay image changes (drag and drop), when the module is
moved in the pipe, when the darkroom image changes, and when the compositing mode is
switched; it also starts empty on the first run.

  1. Open an image in the darkroom and enable overlay.
  2. Drag another image onto the module's drop area to use it as the overlay.
  3. Switch to a different image and back, or move the module in the pipe.
  4. Each rebuild after that runs _setup_overlay() on a pipe worker, which sets the
    tooltip on g->area from that thread.

There is no deterministic user-visible symptom: this is a data race against the GTK
main loop, so it can pass unnoticed, leave a stale tooltip or redraw, or crash, depending
on timing. Two consequences are deterministic and do not depend on timing:

  • Leak — with the overlay image missing from the library, every rebuild leaks one
    tooltip string.
  • Unrequested history item — remove the overlay image from the library while leaving
    the file on disk, then re-import it. The next rebuild finds it again by path, rewrites
    p->imgid and calls dt_dev_add_history_item() from the worker thread, so a history
    entry appears that the user did not create.

Evidence

All code is in src/iop/overlay.c.

_setup_overlay() takes the GUI data and calls GTK on it:

static void _setup_overlay(dt_iop_module_t *self,
                           const dt_dev_pixelpipe_iop_t *piece, ...)
{
  dt_iop_overlay_params_t *p = self->params;
  const dt_iop_overlay_gui_data_t *g = self->gui_data;
  ...
      p->imgid = new_imgid;
      imgid = new_imgid;
      dt_dev_add_history_item(dev, self, TRUE);
      if(g)
        gtk_widget_queue_draw(GTK_WIDGET(g->area));       // GTK call
    }
    else if(g)
    {
      const gchar *tooltip = g_strdup_printf
        (_("overlay image missing from database\n\n"
           "'%s'" ), p->filename);
      gtk_widget_set_tooltip_text(GTK_WIDGET(g->area), tooltip);  // GTK call, and
                                                                  // tooltip is leaked
    }
  }

  if(image_exists)
  {
    if(g)
      gtk_widget_set_tooltip_text(GTK_WIDGET(g->area), "");       // GTK call

The call chain that puts this on a worker thread:

  • process() and process_cl() call _get_overlay_argb() and _get_overlay_rgba_f().
  • Both of those call _setup_overlay() whenever the cached overlay buffer is missing:
static float *_get_overlay_rgba_f(dt_iop_module_t *self, ...)
{
  ...
  if(!*pbuf)
  {
    _setup_overlay(self, piece, FALSE /* legacy */, pbuf, pwidth, pheight);

_get_overlay_argb() has the same shape, with TRUE for the legacy argument.

The tooltip in the success case is set on every rebuild, so the GTK calls are not
limited to the error path where the overlay image is missing.

dev-doc/GUI.md, section Common Mistakes, gives a GTK call inside process() as its
first example of what not to do.

Environment

  • Present in 943d74a50e5baeecee26005cf20309e32f487949 (2026-08-22, nightly-540-g943d74a50e).
  • Found by code analysis in a container; no GUI run, no OS/GPU/driver data to report, and
    the OpenCL relevance above is from the call graph, not from a run with OpenCL enabled.

Suggested fix

Do the GTK work on the GTK thread instead of on the worker:

  • Replace gtk_widget_queue_draw() with dt_control_queue_redraw_widget()
    (src/control/control.h), which is safe to call from any thread.
  • Set the tooltip from a g_idle_add() callback that runs on the GTK thread, passing the
    text it needs, and free that text in the callback. This also removes the leak.
  • Move the p->imgid write and the dt_dev_add_history_item() call off the pipe path.
    Changing module parameters and history from inside process() is a separate problem
    from the GTK calls, and it needs a decision about where the re-import of a missing
    overlay image should be detected instead.

The redraw and tooltip parts are small and can be done on their own. The history-item
part is the one that needs thought.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.