GUI_Threading.md

Thread Safety — Sharing gui_data Between Threads

How an IOP module shares gui_data between the GTK main thread and the pixelpipe worker threads, and between two pipe worker threads: which callback runs where, when the GUI critical section is required, how to send an update to the GUI without outliving the module, and which parts of this the framework already does for you.

See also:

• GUI.md — GUI architecture: UI construction, events and callbacks, reparenting
• IOP_Module_API.md — Module API reference, commit_params(), lifecycle
• pixelpipe_architecture.md — Pixelpipe data flow, caching, ROI, ordering

---

The Problem

process() is not a GTK callback and has no guaranteed thread affinity. GTK is not thread-safe. You cannot call GTK functions directly from process().

The same split applies to data: gui_data is not owned by the GTK thread alone. Three directions need care:

• the pipe writing values for the GUI to display,
• the GUI writing values the pipe will read,
• and one pipe writing values another pipe reads, because gui_data is where that channel lives too.

The rules, in short:

• Know which thread you are on. commit_params() and process() are pipe callbacks, not GUI ones.
• Take the GUI critical section for every field more than one thread touches — and only when a GUI exists.
• Hold it as long as the value must stay valid — not just for the load.
• Never call GTK from a pipe worker thread. Hand the update to the main loop (Pattern A), and cancel it before the module or the image goes away.
• Check whether the framework has already done it before hand-rolling buffer, hash and lock plumbing for a cursor readout.

The sections, in order:

Where your code runs

• Which Thread Am I On?
  • Thread-Safe Redraw Helpers

Locking rules

• Using gui_data from commit_params()
• Writing gui_data from a Widget Callback
• Publishing gui_data Through a Proxy
• Passing Values Between Pipes Through gui_data
• The Lock Is Recursive
  • gui_lock Is the Innermost Lock
• Hold the Lock as Long as the Value Must Stay Valid

What the framework already does

• The Framework Service for Per-Pixel Readouts

Sending updates to the GUI

• Guards Before Sending GUI Updates
• Pattern A: Critical Section + g_idle_add
• Pattern B: Message Passing
• The Callback Must Not Outlive the Module or the Image

Reference

• Common Mistakes

Which Thread Am I On?

Always the GTK main thread	Pipeline callbacks — no thread guarantee
gui_init(), gui_cleanup()	commit_params()
gui_update(), gui_changed()	process(), process_cl()
widget callbacks (sliders, buttons, combos)	process_tiling(), process_tiling_cl()
draw / expose callbacks, gui_post_expose()	modify_roi_in(), modify_roi_out(), tiling_callback()
mouse and scroll handlers	init_pipe(), cleanup_pipe()
color_picker_apply()	output_format(), the four colorspace callbacks
	distort_transform(), distort_backtransform(), distort_mask()

The right column is the per-instance src/iop/iop_api.h callbacks the pipe drives that can reach gui_data. It is not the whole API surface — the pipe also calls metadata callbacks such as flags() and operation_tags(), but their signatures give them no module instance, so they cannot touch gui_data. (input_format() is declared but has no caller in the tree, so it is not listed.)

color_picker_apply() is on the left even though the pipe measured the values it reads, because the pipe does not call it. Once the preview pipe has sampled the picker area, it raises DT_SIGNAL_CONTROL_PICKERDATA_READY. That signal is asynchronous, so its handler runs later on the GTK main loop, and that handler is the callback's only caller (src/gui/color_picker_proxy.c, src/control/signal.c). A picker callback may call GTK, and needs the lock only for the gui_data fields a pipe callback also touches, as in a widget callback.

In tree today: the picked values themselves are not handed over under any lock. The preview pipe writes self->picked_color, picked_color_min and picked_color_max, and the picked_output_* arrays beside them, with no lock held, just before it raises the signal (src/develop/pixelpipe_hb.c). Nothing orders the callback's later read against the preview pipe's next run, which rewrites the same arrays. A module cannot close that window: the pipe takes no lock that the callback could take too.

Each column covers the static helpers called from it too. A process() that hands gui_data to a helper does not make the access GTK-thread-safe, and that is where mistakes hide: the offending line is two or three frames down, in a function whose name says nothing about which thread reaches it. When you audit a field, follow the call chain, not just the callback name.

This would be wrong:

static void _rebuild_cache(dt_iop_module_t *self, ...)
{
  dt_iop_mymodule_gui_data_t *g = self->gui_data;
  ...
  g->readout = value;                            // no critical section
  gtk_widget_set_tooltip_text(g->area, text);    // GTK, on whatever thread got here
  gtk_widget_queue_draw(g->area);
}

void process(...)
{
  if(_cache_is_stale(...)) _rebuild_cache(self, ...);   // a pipe worker thread
  ...
}

A mutex of your own does not rescue that. Holding one across _rebuild_cache() serializes your pipe worker threads against each other, which is worth doing for the cache; the GTK main loop never takes that mutex and is not ordered by it, so the two widget calls are exactly as unsynchronized as they were. Only getting them onto the main loop fixes them — Pattern A, or one of the thread-safe redraw helpers below.

Thread-Safe Redraw Helpers

These marshal the redraw onto the GTK main context internally and are safe to call from any thread:

• dt_control_queue_redraw_widget(widget) — redraw a specific widget
• dt_control_queue_redraw_center() — redraw the center view

The Picture for Your Own Instance

The right column has no fixed thread, but for the instance that owns your gui_data the picture is narrower:

• Your darkroom instance — process() runs on one of the three screen pipes' worker threads (full, preview, preview2), never on the GTK main thread. commit_params() usually does too, as part of synchronizing a pipe, but it has a route onto the GTK thread as well: rebuilding a pipe's nodes calls every module's init_pipe(), and basecurve's calls commit_params() straight back (src/iop/basecurve.c) — while the image switch rebuilds the screen pipes from an idle callback (src/views/darkroom.c). So not even this instance gives commit_params() an affinity you can rely on.
• Every instance in another develop context — export, thumbnail generation, snapshots, the duplicate manager, the tethering view's histogram of the last captured image, and the overlay module rendering its second image, among others — belongs to a separate dt_develop_t with its own module instances and no GUI (gui_data == NULL). Most of those develop contexts are built for one render and thrown away, but not all: the pinned second-window preview keeps its own develop context, history, modules and pipes until it is unpinned (src/develop/develop.c). What they have in common is the absent gui_data, not a short life. Several of them run the pipe synchronously from a GTK draw handler, so there the very same callbacks do run on the GTK main thread.

Hence the two-sided rule: commit_params() must never touch GTK, and must never wait for the GTK main loop to run anything. Contending for gui_lock is not that — it is a short mutex both sides release quickly, and taking it is the whole point of the next section. It stays short only while whoever holds it blocks on none of the locks the pipe already holds when it takes gui_lock; that is gui_lock Is the Innermost Lock. A synchronous round-trip through the main loop deadlocks even when the locking is right, because the GTK thread may itself be waiting on a pipe.

Exceptions: Pipeline Callbacks That GTK-Thread Code Also Calls

Four entries in the right column are also called directly from GTK-thread code, so they run on either thread and need the same care for a different reason: distort_transform() and distort_backtransform() (mask handling and darkroom zoom both call them), blend_colorspace() (the blend GUI calls it) and default_colorspace() (the color picker calls it while building a picker button, src/gui/color_picker_proxy.c). distort_mask() does not have that property despite the similar name — it is invoked only from pipeline code.

Those two colorspace callbacks are called with NULL for both pipe and piece from the GTK side, so each has to tolerate that. default_colorspace() is also reached indirectly: if you leave blend_colorspace() at its default, default_blend_colorspace() forwards its two arguments straight through.

Using gui_data from commit_params()

commit_params() is the one that surprises people. It reads like module setup code that belongs to the GUI, but it is called while a pipe is being synchronized. The three screen pipes each run on their own worker thread, yet synchronization holds dev->history_mutex across the commit, so commit_params() does not run concurrently with itself for one module instance. It can still overlap with process() on another pipe and with any GTK-thread callback — that is what the lock is for.

Two preconditions, both mandatory:

• The GUI may not exist. gui_data is NULL in export and batch mode, and the GUI critical section — dt_iop_gui_enter_critical_section(), i.e. the module's gui_lock — is only initialized by dt_iop_gui_init(). Entering it without a GUI locks a mutex that was never set up.
• Processing must not depend on the GUI cache. The same commit_params() runs during export, where there is nothing to read. Whatever the cache saves for the darkroom, the non-GUI branch has to compute independently.

The shape:

void commit_params(dt_iop_module_t *self, dt_iop_params_t *params, ...)
{
  dt_iop_mymodule_params_t *p = (dt_iop_mymodule_params_t *)params;
  dt_iop_mymodule_gui_data_t *g = self->gui_data;
  ...
  if(self->dev->gui_attached && g)
  {
    // darkroom: refresh and reuse the GUI-side cache
    dt_iop_gui_enter_critical_section(self);
    if(g->smoothing != p->smoothing) g->interpolation_valid = FALSE;
    g->smoothing = p->smoothing;
    dt_iop_gui_leave_critical_section(self);

    _rebuild_lut(self, p);   // takes the lock itself, so call it outside the section
    ...
  }
  else
  {
    // export or headless: solve from scratch, with no cache to lean on
    _solve_from_scratch(p, ...);
    ...
  }
}

Two things to copy and one to watch. Copy the guard and the critical section around the fields both sides touch. Copy the else branch too — the export path has to reach the same processing result without the cache, and a module that quietly depends on the cache exports differently from how it previewed. What to watch is the helper: pass it p, the argument this commit was handed, and not self->params. Those are the same object in the normal case and different on the pipe's defaults sync, which passes default_params (see IOP_Module_API.md). And keep it outside the critical section if it takes the lock itself — see The Lock Is Recursive.

src/iop/toneequal.c's commit_params() is the in-tree instance of this split.

g != NULL is the whole guard, because a dev whose modules were loaded without a GUI leaves gui_data NULL, and on the normal lifecycle the lock is ready by then: dt_iop_gui_init() initializes gui_lock immediately before it calls gui_init().

Treat that as the lifecycle, not as an invariant you can lean on.

In tree today: dt_iop_gui_init() is the wrapper that creates the lock, and it is the wrapper that guarantees the pairing. A framework path that calls a module's gui_init() directly instead leaves gui_data allocated and gui_lock as it found it — and at least one history route in tree does exactly that today. Nothing in a module can detect it, and nothing you write in commit_params() fixes it. It is noted here only so that "gui_data != NULL means the lock is ready" is read as what normally happens rather than as something the framework promises.

g is the test that holds; self->dev->gui_attached is the in-tree idiom. Write both, as the examples in this document do: it is what the tree writes, at about forty places under src/iop, and it records the intent. But read only g as a test.

gui_attached adds nothing to g in current code. It is fixed while the dt_develop_t is being built and never toggled afterwards. Most non-GUI contexts pass FALSE straight to dt_dev_init() — export, thumbnailing, style application, mask objects. Exactly one passes TRUE and overwrites it: the throw-away develop context dt_dev_image() renders into, which clears the field before it loads a single module (src/develop/develop.c). darktable.develop is given TRUE before darktable decides whether to start a GUI at all (src/common/darktable.c) — so it is TRUE under darktable-cli too, and on its own it is not even a test for "a GUI exists".

Dereferencing self->dev to reach it is safe here, because a module only ever reaches a pipe through dev->iop and so always has a dev by the time commit_params() or process() runs. That is not true everywhere — one instance in the tree has gui_data without a dev; see Publishing gui_data Through a Proxy.

Writing gui_data from a Widget Callback

This is allowed and common. The question is only whether you need the lock:

• Field touched by the GTK thread only — no lock. Mouse position, which node is selected, a cached gradient used solely for drawing the widget.
• Field also read or written by commit_params() or process() — take the lock. Typically these are cached results and their validity flags.

static void _slider_callback(GtkWidget *slider, dt_iop_module_t *self)
{
  dt_iop_mymodule_params_t *p = self->params;
  dt_iop_mymodule_gui_data_t *g = self->gui_data;

  p->xyz = dt_bauhaus_slider_get(slider);

  // g->cached_xyz is also written by commit_params() on a pipe worker thread
  dt_iop_gui_enter_critical_section(self);
  if(g->cached_xyz != p->xyz) g->cache_valid = FALSE;
  g->cached_xyz = p->xyz;
  dt_iop_gui_leave_critical_section(self);

  dt_dev_add_history_item(darktable.develop, self, TRUE);
}

If you are unsure whether a field crosses threads, search for every read and write of it and check which callback each one sits in, using the table above.

Asking for a reprocess is not synchronization. A widget callback that writes a field and then calls dt_dev_reprocess_center(), dt_dev_reprocess_all() or dt_dev_add_history_item() still needs the lock. Those calls set pipe->changed, invalidate buffers and queue a redraw; none of them waits for a running pipe or issues a barrier, so a pipe already inside process() is not ordered against your write at all — it may see the old value, the new one, or change its mind mid-frame if the compiler re-loads the field.

Publishing gui_data Through a Proxy

Some modules publish a value to the rest of darktable through dev->proxy — exposure publishes its effective exposure that way, and agx reads it from a GTK-thread helper (src/iop/agx.c). The caller sits in another module and has no way to take your gui_lock, so the accessor has to make its own access safe.

Derive, don't cache. Before asking how to lock the field, ask whether it has to be a field at all. A value that is a function of params and of the image can be recomputed inside the accessor, on the caller's thread, from data that thread already owns. Then there is no shared field, no lock, and no window in which the pipe and the GUI disagree. Having a pipe worker thread compute the same value into gui_data so that the GUI can read it back buys nothing and costs a data race.

exposure shows both halves of the answer, because its two modes are genuinely different:

• Manual mode — the effective exposure is the exposure parameter plus the two optional compensations, and both compensations come from the image's EXIF data. All of that is available to the accessor, so the accessor derives the value and does not read gui_data at all. Nothing is shared, so nothing needs locking, provided the accessor runs on the GTK thread, which owns params. The thread contract on dt_dev_proxy_exposure_t requires that of every caller (src/develop/develop.h).
• Deflicker mode — the correction is computed from a histogram of the raw file, which is nowhere in params, so there is nothing to derive from and the value has to be published. The scalar that carries it follows the locking half of Pattern A: _process_common_setup() writes g->deflicker_computed_exposure inside a critical section on the pipe worker thread, and both of its GTK-thread readers, the _show_computed() idle callback and the proxy accessor, read it inside one (src/iop/exposure.c). The idle callback is not a model for the other half: it locks and reads gui_data without testing it at all, gui_cleanup() removes only one queued source, and the module has no change_image(), all short of The Callback Must Not Outlive the Module or the Image (#22007).

The histogram behind that scalar is a second piece of shared state, and it is not covered. Publishing a value safely does not make the state it was computed from safe; each shared field needs its own answer.

In tree today: gui_update() and gui_changed() free g->deflicker_histogram and rebuild it on the GTK thread, while _process_common_setup() reads that pointer and its statistics on a pipe worker thread — neither side takes gui_lock, and in gui_update() the free sits between two critical sections that guard other fields. That is the trap from Hold the Lock as Long as the Value Must Stay Valid, live in the tree: a heap buffer in gui_data that one thread can free while the other is walking it.

So which shape a proxy accessor needs depends on which of the two cases it is. A derived value makes the accessor a plain function of params. A value that genuinely has to come from the pipe makes it this:

static float _mymodule_proxy_get_value(dt_iop_module_t *self)
{
  dt_iop_mymodule_gui_data_t *g = self->gui_data;
  if(!g) return 0.0f;

  dt_iop_gui_enter_critical_section(self);
  const float v = g->computed_value;
  dt_iop_gui_leave_critical_section(self);
  return v;
}

The example tests g and stops there, and stopping there is the point. A proxy accessor takes whatever instance its caller hands it, and self->dev is not guaranteed: on the throw-away instance darktable builds at startup so that widgets can register their accelerators, gui_data is allocated and dev is NULL (see GUI.md).

Reordering the guard does not help, and it is worth being explicit about why, because the opposite is easy to assume. On that instance gui_data is allocated, so g is non-NULL: if(!g || !self->dev->gui_attached) gets straight past !g and dereferences the NULL dev, exactly as if(self->dev->gui_attached && g) would. Short-circuit evaluation only protects the operand it skips, and here it skips neither. What protects you is not reaching for self->dev at all — as the example above does — or testing it on its own if you need something from it:

  if(!g || !self->dev) return 0.0f;

Inside process() and commit_params() none of this arises, for the reason given in Using gui_data from commit_params(): a module reaches a pipe only through dev->iop, so it always has a dev there and either order is safe. That is why the commit_params() example earlier in this document can write self->dev->gui_attached && g without qualification.

That startup instance is also why the dev->proxy function pointers outlive every live instance. It is torn down again straight away, and exposure's gui_cleanup() clears proxy.exposure.module, but nothing clears the function pointers it registered (src/iop/exposure.c) — so a non-NULL accessor pointer does not mean there is an instance behind it. In tree the callers carry that check: dt_dev_exposure_get_effective_exposure() resolves a live, enabled instance itself and passes that one, and the other two getters go through a helper that requires proxy.exposure.module to be set (src/develop/develop.c).

The producing side — usually commit_params() or process() — must take the same lock. Publishing a raw pointer into gui_data through a proxy is worse still: the reader then has no lock to take at all, and no way to know the GUI is being torn down.

Passing Values Between Pipes Through gui_data

gui_data is also the channel one pipe uses to hand a value to another. A module that needs a property of the whole image usually cannot get it on the full pipe, which normally sees only the region of interest at the current zoom. The preview pipe does see the whole image, at reduced size, so the module computes the value there, publishes it into gui_data, and lets the full pipe pick it up.

The framework primitive for the handover is dt_dev_sync_pixelpipe_hash() (src/develop/develop.c). The publisher stores the value together with the cumulative hash of its own upstream pipe state. The consumer hands the primitive that hash cell and its gui_lock, and the primitive waits until the stored hash matches what the consumer's own upstream state hashes to.

Read that as evidence, not as proof. The primitive compares a hash it snapshotted against one it computes afterwards, and returns as soon as the two agree; it does not snapshot the payload, and it does not stop the publisher running again and replacing payload and hash before the consumer gets round to reading them. The hash it compares is also a hash of selected upstream piece values, not a full render identity — the cache key is a different and larger thing. So a match makes a leftover from the previous history state unlikely; it does not make it impossible.

A handful of modules use it, each passing &self->gui_lock as the lock argument so that the primitive can read the publisher's hash safely while it waits.

Not every hash in gui_data means this is happening. A module may keep one purely to memoize its own work, comparing it only against a hash it wrote itself on the same pipe, and never waiting for anybody. A hash beside a payload is a freshness marker; it is a handover only when one pipe waits on a hash another pipe wrote.

The consuming side:

// on the consuming pipe
if(g && dt_pipe_is_full(piece->pipe))
{
  dt_iop_gui_enter_critical_section(self);
  const dt_hash_t hash = g->hash;      // has anything been published yet?
  dt_iop_gui_leave_critical_section(self);

  if(hash != DT_INVALID_HASH
     && !dt_dev_sync_pixelpipe_hash(self->dev, piece->pipe, self->iop_order,
                                    DT_DEV_TRANSFORM_DIR_BACK_INCL,
                                    &self->gui_lock, &g->hash))
    dt_control_log(_("inconsistent output"));

  dt_iop_gui_enter_critical_section(self);
  d->value = g->published_value;       // the payload, whatever it is
  dt_iop_gui_leave_critical_section(self);
}

and the publishing side, usually in the same function:

// on the producing pipe
if(g && dt_pipe_is_preview(piece->pipe))
{
  const dt_hash_t hash = dt_dev_hash_plus(self->dev, piece->pipe, self->iop_order,
                                          DT_DEV_TRANSFORM_DIR_BACK_INCL);
  dt_iop_gui_enter_critical_section(self);
  g->published_value = d->value;   // both go in under one lock, so no reader can
  g->hash = hash;                  // catch the pair half-updated
  dt_iop_gui_leave_critical_section(self);
}

src/iop/levels.c carries both halves in one helper, commit_params_late(), called from process() and process_cl().

Four things about that shape do not follow from the rest of this document.

g != NULL is not testing for a GUI here. Nothing is being displayed; the test is there because the channel itself lives in gui_data and so exists only when a GUI does. The rule from Using gui_data from commit_params() applies unchanged — processing must not depend on the cache — and these modules obey it. The in-tree consumers each recompute from scratch when they are on the producing pipe, or when the published value is still at its uninitialized sentinel. Note what that fallback does not cover: it triggers on a value that was never published, not on one that arrived late. After a timed-out wait the consumer uses whatever is in gui_data. Between that and the limits above, a module here has to be able to live with a stale value as well as with none.

process() may block here, and that is the supported idiom. The wait is bounded by the pixelpipe_synchronization_timeout preference — or by darktable.opencl->opencl_synchronization_timeout on a GPU pipe — and it gives up early once the pipe is flagged for shutdown. Setting that preference to zero or less switches the wait off, and the primitive then reports success without checking anything. After a real timeout it still reports success if the history stack has changed underneath, since a reprocess is already on its way; it fails only when neither holds, and that failure is the inconsistent output in the snippet above. This is not the wait ruled out in Which Thread Am I On?. What that section rules out is a synchronous round-trip through the GTK main loop, because the GTK thread may itself be waiting on a pipe. Waiting on another pipe is a different thing, and the framework supplies the primitive for it.

Do not hold gui_lock across the call. The primitive takes the lock you hand it on every polling iteration, so calling it from inside a critical section self-deadlocks on the recursive mutex. Snapshot what you need, release, then call — as the consuming snippet above does. Underneath, the probe also takes dev->history_mutex, because hashing the upstream state walks the pipe. The primitive releases your gui_lock before it does that, so the two are never nested; keep it that way on your side, for the reason in gui_lock Is the Innermost Lock.

A scalar hands over cleanly; an allocation does not. When the payload is a plain number, the span in which it must stay valid ends at the load and the short critical section above is exactly right. When it is a heap object, copying the pointer out under the lock buys nothing: only the pointer load is protected, and the publisher's next run may have freed the object before you are done with it.

The hash handshake does not close that window. It establishes that the publisher has finished a run matching your upstream state; it does not stop the next slider move from starting another one that frees and replaces the object while you are still reading it. A widget callback that resets the published state can free it too. This is Hold the Lock as Long as the Value Must Stay Valid wearing an inter-pipe costume: hold the lock through the last use, copy the data rather than the pointer, or transfer ownership so the publisher cannot free it. Pick one of the three before you publish an allocation.

The Lock Is Not Recursive

dt_iop_gui_enter_critical_section() takes a plain, non-recursive mutex. Taking it twice on the same thread deadlocks — and the second acquisition is usually invisible, hidden inside a helper you call.

// WRONG — self-deadlock if _rebuild_cache() takes the lock itself
dt_iop_gui_enter_critical_section(self);
g->cached_xyz = p->xyz;
_rebuild_cache(self);
dt_iop_gui_leave_critical_section(self);

// RIGHT — keep the section around the writes only
dt_iop_gui_enter_critical_section(self);
g->cached_xyz = p->xyz;
dt_iop_gui_leave_critical_section(self);

_rebuild_cache(self);   // takes the lock itself

So before calling anything from inside a critical section, check whether the callee locks. Framework helpers count — dt_dev_sync_pixelpipe_hash() in particular, which takes the lock you hand it on every polling iteration. Keep critical sections short and free of function calls where you can — that avoids the problem instead of reasoning about it.

gui_lock Is the Innermost Lock

Taking gui_lock twice is one way a helper inside a critical section hangs you. The other has nothing to do with recursion: holding gui_lock while you block on a lock that the pipe already holds when it takes gui_lock.

Every pipe path that takes gui_lock takes it last. dt_dev_pixelpipe_change() takes dev->history_mutex and keeps it across the whole node sync. The sync takes the pipe's busy_mutex and, inside both, calls commit_params(): only for the top history item's module when that item is all that changed (dt_dev_pixelpipe_synch_top()), for every module on a full sync (dt_dev_pixelpipe_synch_all()). process() runs with busy_mutex held for the whole pipe run (src/develop/pixelpipe_hb.c). A module that uses gui_data from either callback therefore takes gui_lock inside those:

dev->history_mutex  ->  pipe->busy_mutex  ->  gui_lock     commit_params(), node sync
                        pipe->busy_mutex  ->  gui_lock     process(), pipe run

The two lines are two paths, not one order between history_mutex and busy_mutex. process() may take history_mutex under busy_mutex too: in the publishing snippet in Passing Values Between Pipes Through gui_data, dt_dev_hash_plus() takes and releases it before the snippet enters the section, and src/iop/levels.c does the same. What every path agrees on is where gui_lock goes.

So nothing may block on them the other way round. Holding gui_lock across a call that blocks on history_mutex or on a pipe's busy_mutex is an AB-BA inversion: your thread holds gui_lock and waits for the mutex, a pipe worker holds the mutex and waits for gui_lock, and neither ever gets what it is waiting for. When your thread is the GTK one, the darkroom is frozen for good. No round trip through the main loop is involved, and nothing in the module's own file looks wrong. It is not only a GTK-side rule, either: gui_lock belongs to the module instance, which all three screen pipes share, so process() on one pipe holding it across such a call deadlocks against a node sync on another, and the next GTK callback that enters the section hangs behind them.

That dev->history_mutex is recursive does not help: recursion lets one thread take the same mutex again, and does nothing about two threads taking two mutexes in opposite orders.

As with recursion, the second lock is usually hidden inside a call. The common ones that take history_mutex (all in src/develop/develop.c):

• dt_dev_distort_transform_plus() and dt_dev_distort_backtransform_plus(), the usual way to map a cursor position onto a buffer pixel;
• dt_dev_hash_plus(), and through it dt_dev_sync_pixelpipe_hash(), whose own note is in Passing Values Between Pipes Through gui_data;
• dt_dev_add_history_item() and dt_dev_add_masks_history_item().

// WRONG — gui_lock held across a call that takes history_mutex
dt_iop_gui_enter_critical_section(self);
g->cursor_exposure = log2f(_luminance_at_cursor(self));   // backtransforms inside
dt_iop_gui_leave_critical_section(self);

// RIGHT — map the cursor first, then lock for the buffer read and the write
float pt[2] = { x, y };
dt_dev_distort_backtransform_plus(self->dev, self->dev->preview_pipe, self->iop_order,
                                  DT_DEV_TRANSFORM_DIR_FORW_EXCL, pt, 1);

dt_iop_gui_enter_critical_section(self);
g->cursor_exposure = log2f(_luminance_at(g, pt[0], pt[1]));   // reads g's buffer only
dt_iop_gui_leave_critical_section(self);

The RIGHT form moves only the transform out of the section. Moving the whole helper out would fix the inversion and open the other trap: the buffer read then happens with no lock held, which Hold the Lock as Long as the Value Must Stay Valid rules out.

toneequal's scrolled() had the WRONG form until 890c7f5fdc (darktable-org#22133, found by reading the code rather than from a reported freeze). That commit moved the whole helper out of the section, so it is not an example of the RIGHT form: the buffer read in scrolled() still happens with no lock held, and darktable-org#22068, still open, tracks it.

The framework keeps to the same order where it cannot avoid nesting: dt_preview_data_is_fresh() already holds gui_lock when it needs the preview pipe's busy_mutex, so it only tries to take it and treats a busy pipe as "not fresh" (src/develop/preview_data.c). That reverse nesting is safe only because a trylock cannot block: turning it into a plain lock brings the inversion back.

Hold the Lock as Long as the Value Must Stay Valid

Shortening a critical section past the point where the value is still needed is its own bug, and a nastier one, because the module now looks locked.

// WRONG — the pointer load is protected, the pointee is not
dt_iop_gui_enter_critical_section(self);
my_cache_t *c = g->cache;
dt_iop_gui_leave_critical_section(self);

use_cache(c);   // another thread may have freed g->cache by now

The lock has to cover the whole span in which the value must stay valid, not just the load. For a plain scalar that span ends at the load, and a snapshot is exactly right. For anything the other thread can free or resize — a heap pointer, a buffer plus the width and height that describe it — it ends when you stop using the value, and you have to pick one of:

• Hold the lock through the last use. The simplest thing that works — but only if every path that frees or replaces the object takes the same lock. gui_cleanup() does not, and does not have to — see The Callback Must Not Outlive the Module or the Image. Every other path that frees or replaces the object — a widget callback, a reset, a reload — still has to take it. It also needs that use to be short: holding the mutex across an allocation, a device transfer or a full-buffer copy blocks the other thread for as long as it takes, and when the blocked thread is the GTK one the user sees it.
• Copy the data, not the pointer, inside the section, then work on your copy.
• Reference-count the object, or hand ownership over explicitly, so the other thread cannot free something you still hold.

The same trap catches tuples. If a buffer and its dimensions are written together under the lock, read them together under the lock too. Reading the dimensions again after releasing can pair a new size with an old allocation.

That rule covers a buffer being replaced. It does not cover one being refilled in place while the flag beside it still says the old contents are good. Clear the flag inside a critical section before the fill starts, and commit the new contents' hash and the flag inside another one once the data is there; between the two the reader sees "not valid" and stays away:

  dt_iop_gui_enter_critical_section(self);
  g->buffer_valid = FALSE;              // reader now knows to stay away
  dt_iop_gui_leave_critical_section(self);

  _fill_buffer(g->buffer, ...);         // long, and deliberately outside the lock

  dt_iop_gui_enter_critical_section(self);
  g->hash = new_hash;                   // hash and flag committed together
  g->buffer_valid = TRUE;
  dt_iop_gui_leave_critical_section(self);

Without the first section there is a window in which a valid-flagged buffer holds half of one image and half of another — a different failure from the size mismatch above, and one that no amount of locking around the read will catch.

src/iop/toneequal.c is worth reading for the producer half of this, with one difference. It clears the flag and fills outside the lock as above, but it commits the hash and raises the flag in two separate critical sections rather than one. That is still safe, and only because of the order: a reader that lands in the gap sees a fresh hash with the flag still down and waits, never the reverse. Committing both in one section, as shown here, is the version that does not need that argument made for it.

The flag only pays off if the reader honors it for as long as it uses the buffer. Testing it under the lock, releasing, and then reading puts you back in the first trap in this section: the writer can clear the flag and start refilling in between. Test and use inside one critical section, or copy out what you need while you hold it.

The Framework Service for Per-Pixel Readouts

Everything above is the hand-rolled version. For the most common instance of it — a per-pixel value produced on the preview pipe and read on the GTK thread under the mouse cursor — the framework already has it: src/develop/preview_data.h. Its file comment says what it is for, and names the modules that used to duplicate it.

The service owns the buffer, the hash and the locking:

• dt_preview_data_alloc() in gui_init() and dt_preview_data_free() in gui_cleanup() are the bookends. These are the two entry points that do not take the lock — _free() leans on the same teardown guarantee gui_cleanup() does, scoped as in The Callback Must Not Outlive the Module or the Image.
• dt_preview_data_store() is the write side: it sizes the buffer, fills it through a callback of yours and commits the hash.
• dt_preview_data_resize() and dt_preview_data_set_hash() are the two-step form of that, for a fill too expensive to hold the lock across.
• dt_preview_data_get() reads one component of one pixel, from the GTK thread, while the pipe may be writing.
• dt_preview_data_is_fresh() compares the stored hash against the module's piece in the current preview pipe and answers yes or no. It also answers no, without comparing, whenever it cannot take the preview pipe's busy_mutex, which the pipe holds for a whole run and while its nodes are synchronized or rebuilt. A no therefore does not tell stale data from a busy pipe, and the data may well be current: of the pipe's pieces, only yours and those before it enter the hash (its other inputs are listed in Hash-based Caching), so editing a later module usually re-runs the preview pipe without changing the hash. If you request a reprocess on a no, as colorequal does, request it once and not again until a yes, which is what its reprocess_pending flag is for. dt_preview_data_get_hash() does not compare anything — it hands back the stored hash so you can do the comparison yourself. dt_preview_data_invalidate() marks the data stale without dropping the buffer.

The other seven take your module's gui_lock internally for the fields they own, so for the service's own buffer and hash you do not have to take it yourself. What is awkward to build by hand is the guarantee the header attaches to dt_preview_data_store(): resize, fill and hash commit happen inside a single critical section, so the GUI can never observe a resized but not-yet-filled buffer.

The two-step form gives up that single-section guarantee, and hands you the piece you need to replace it: if dt_preview_data_resize() has to resize, it calls a callback of yours while still holding the lock, so you can drop your own validity flag atomically with the resize — the refill-in-place discipline from Hold the Lock as Long as the Value Must Stay Valid, with the framework opening the critical section for you. In src/iop/toneequal.c that callback is four lines long and clears one flag.

The internal locking that spares you taking gui_lock also means you must not be holding it when you call the seven. The lock is recursive, so calling one of them from inside your own critical section deadlocks the thread on a lock it already holds. Your fill and resize callbacks are already inside one: the service calls them from its own section, so they may neither call the seven nor enter the section themselves. dt_preview_data_get() is the likeliest to end up in the wrong place. It returns one value and reads like an array access, so it is easy to drop into a section that has just tested your validity flag.

That section is exactly where the read belongs, though: the end of Hold the Lock as Long as the Value Must Stay Valid has the reader test the flag and use the buffer inside one critical section, so a reader that honors a flag cannot go through the accessor. Inside that section, read pd.buf, pd.width and pd.height directly, as toneequal's update_histogram() does. That is safe because dt_preview_data_store() and dt_preview_data_resize() replace those fields only while holding the same lock (src/develop/preview_data.c).

With dt_preview_data_store() there is no flag to consult, since the fill happens inside the lock, but the accessor on its own is still enough only for one component at a buffer pixel you already have. Mapping the cursor to that pixel needs pd.width and pd.height, and reading several components of one pixel needs them all from the same fill. Both need a single hold of the lock, so do them by hand inside one section as well. colorequal's mouse_moved() maps the cursor that way, for a single component. For several, index each one as pd.buf[(y * pd.width + x) * pd.components + c] inside the same section, which is the index the accessor computes. dt_preview_data_get() re-checks its coordinates against the current size under its own lock, so dimensions read earlier cannot make it read out of range, but after a resize they make it read the wrong pixel.

Nor does the accessor tell you whether the value is current. It does not look at the hash, and when dt_preview_data_store() fails to allocate a new size it keeps the old buffer and only invalidates the hash. dt_preview_data_is_fresh() answers that question, called outside any section of yours like the rest of the seven, but only for the stored data at the moment it runs, in a section of its own, and only when it answers yes: a no may just mean the preview pipe was busy. A dt_preview_data_store() can replace buffer and hash between your read and the check, so a TRUE does not vouch for a value read before or after it. colorequal uses it as a gate, deciding whether to show its cursor and whether to request a reprocess, and reads the buffer again on the next mouse move. If a value must be tied to its hash, read pd.hash in the same section as the value: dt_preview_data_store() commits the two together.

In tree today: not all of dt_preview_data_is_fresh() runs in its section. It tests the buffer pointer in an early return, before it takes the lock at all, against a field that dt_preview_data_store() and dt_preview_data_resize() free and replace while holding it. The pointer is only compared with NULL and never dereferenced, so racing the first allocation can make the function answer FALSE, but not a wrong TRUE.

toneequal and colorequal use the service. What stays yours is what the header says is module-specific: computing the value, drawing it, and mapping the cursor position to a buffer pixel — that last one depends on which geometry modules sit after yours in the pipe, so the service cannot do it for you.

This replaces the buffer, hash and lock bookkeeping for that one case. It does not replace gui_lock for your module's other shared fields, and it is not a general gui_data mutex — everything else in this document still applies to them.

Guards Before Sending GUI Updates

The guard block below belongs before you schedule a GUI update from process(). It has three lines, but only two of them are tests, and only the first is unconditional.

dt_iop_mymodule_gui_data_t *g = self->gui_data;

if(g != NULL                          // GUI exists (not export) — the test that holds
   && self->dev->gui_attached         // the in-tree idiom, not a second test
   && dt_pipe_is_full(piece->pipe))   // the pipe this value belongs to
{
  // Schedule GUI update...
}

The pipe test is the one to think about rather than copy. dt_pipe_is_full() is right when the value only means something for the full-resolution render, and it keeps the two preview pipes from queueing an update apiece. When the value is produced elsewhere, test for that pipe instead: exposure computes its deflicker readout on the preview pipe and publishes it from there (dt_pipe_is_preview(), src/iop/exposure.c). What is never right is no pipe test at all — then every pipe running your module queues its own update.

The middle line is the in-tree idiom, not a working guard — see Using gui_data from commit_params().

Reading g once is enough for the length of the run: on the four darkroom teardown paths the framework holds the screen-pipe mutexes across module GUI teardown, so a non-NULL g cannot be freed while your process() or commit_params() is executing. What the guard does not cover is work you hand to the main loop, and the scope of that teardown guarantee — both are in The Callback Must Not Outlive the Module or the Image.

Pattern A: Critical Section + g_idle_add

Store computed values in gui_data under mutex, then schedule a GTK-thread callback. This is the pattern to reach for first; Pattern B below is for the payload that cannot live in gui_data.

g is self->gui_data, tested as in Guards Before Sending GUI Updates — that guard block is required context for both patterns.

// In process():
if(g != NULL && self->dev->gui_attached      // see "Guards Before Sending GUI Updates"
   && dt_pipe_is_full(piece->pipe))          // — and pick the pipe test to match
{                                            //   where your value is produced
  dt_iop_gui_enter_critical_section(self);
  g->computed_exposure = exposure;
  dt_iop_gui_leave_critical_section(self);
  g_idle_add(_show_computed, self);
}

// Callback (GTK main thread):
static gboolean _show_computed(gpointer user_data)
{
  dt_iop_module_t *self = user_data;
  dt_iop_mymodule_gui_data_t *g = self->gui_data;
  // fallback check only — see "The Callback Must Not Outlive the Module or the Image"
  if(!g) return G_SOURCE_REMOVE;

  dt_iop_gui_enter_critical_section(self);
  float val = g->computed_exposure;
  dt_iop_gui_leave_critical_section(self);

  gchar *str = g_strdup_printf(_("%.2f EV"), val);
  gtk_label_set_text(g->label, str);
  g_free(str);

  return G_SOURCE_REMOVE;  // Run once, then remove
}

// In gui_cleanup(): drop callbacks still queued for this module
while(g_idle_remove_by_data(self)) ;
// And in change_image(): an image switch keeps the base instance and its gui_data,
// so a source queued for the old image would otherwise run against the new one

Pattern B: Message Passing

Allocate a message struct that the callback owns and frees. The payload never lives in gui_data, so no critical section is needed for it — but cancelling the callback is harder, and the bookkeeping that fixes it is more than the critical section this pattern set out to avoid. Prefer Pattern A unless the payload genuinely cannot live in gui_data; the reasoning is at the end of The Callback Must Not Outlive the Module or the Image.

The snippet below shows the dispatch and the callback only. It is not complete: the source-id bookkeeping that makes it cancellable is described in that section, and has to be added.

typedef struct
{
  dt_iop_module_t *self;
  float values[3];
} mymodule_gui_msg_t;

// Callback (GTK main thread):
static gboolean _update_gui(gpointer data)
{
  mymodule_gui_msg_t *msg = data;
  dt_iop_mymodule_gui_data_t *g = msg->self->gui_data;
  // fallback check only — see "The Callback Must Not Outlive the Module or the Image"
  if(!g)
  {
    g_free(msg);
    return G_SOURCE_REMOVE;
  }

  memcpy(g->display_values, msg->values, sizeof(g->display_values));
  gtk_widget_queue_draw(g->area);   // reach the widget through g, not through msg->self

  g_free(msg);  // Callback owns the message
  return G_SOURCE_REMOVE;
}

// At end of process():
if(g != NULL && self->dev->gui_attached      // see "Guards Before Sending GUI Updates"
   && dt_pipe_is_full(piece->pipe))
{
  mymodule_gui_msg_t *msg = g_malloc(sizeof(*msg));
  msg->self = self;
  memcpy(msg->values, local_values, sizeof(msg->values));
  // keyed on msg, so it cannot be cancelled by data — see the next section
  g_idle_add(_update_gui, msg);
}

Two details in that callback are not decoration. The NULL check earns more here than it does in Pattern A: a source keyed on the message cannot be cancelled with g_idle_remove_by_data(), so until you add the source-id bookkeeping described in the next section it is the only thing standing between a deleted instance — the one teardown shape that keeps the module struct and frees only its GUI — and a NULL dereference. On the shapes that free the struct as well it is already too late, which is the whole of why it is a fallback check and not a guard.

And every field the callback touches is reached through g, so that one check covers the whole body — including the widget. Taking the widget from msg->self->widget instead would leave a dereference the check does not guard: dt_iop_gui_cleanup_module() destroys that widget and sets the field to NULL, right beside where it frees gui_data, with the in-source note that it does so because asynchronous work can still be carrying the module (src/develop/imageop.c). Keep your widget pointers in gui_data and the guard stays honest.

The Callback Must Not Outlive the Module or the Image

The framework guarantees one thing here: on all four darkroom teardown paths, no module GUI is torn down while a pipe is running. Those four sites — leaving the darkroom, switching image, deleting an instance, and undoing or redoing a module add or delete — hold all three screen-pipe mutexes across dt_iop_gui_cleanup_module(). Deleting an instance and undo/redo take them through the dt_dev_pixelpipe_stop_and_lock_all() / dt_dev_pixelpipe_unlock_all() pair (src/develop/develop.c), which also flags the pipes for shutdown; darkroom exit and the image switch lock the same three mutexes directly (src/views/darkroom.c), the image switch with trylock and a re-queue for as long as a pipe is busy. One consequence for module authors: on those four paths gui_cleanup() runs with all three screen-pipe mutexes held, so it must not call anything that waits on a pipe. Those four are not the only callers, though — the startup accelerator probe tears its throw-away instance down with no pipe lock held at all (src/develop/imageop.c; on that startup instance dev is NULL as well, see GUI.md). So take the mutexes as a constraint on what gui_cleanup() may do, never as a guarantee it can lean on.

That closes the window against pipe worker threads. It does not close the one you open yourself by handing work to the main loop.

g_idle_add() hands the main loop a raw dt_iop_module_t *. What teardown does to that pointer depends on the path, and there are three shapes:

• Leaving the darkroom — for every module that has a GUI: gui_cleanup() runs, gui_lock is destroyed, gui_data is freed, and then the module itself is freed. (Hidden modules never get a GUI, so all three teardown shapes skip them.)
• Deleting an instance, or undo/redo of an add or delete — the same GUI teardown runs, but the module struct survives: it is parked in dev->alliop for pipes that may still reference it, and freed when the darkroom is left or the image is switched.
• Switching image — this one splits (src/views/darkroom.c). Each module's base instance — the one with the lowest multi_priority — is kept: no gui_cleanup(), gui_lock and gui_data stay alive, and the instance is re-used for the new image after dt_iop_reload_defaults() and, if it implements one, change_image(). Its extra instances are torn down like a darkroom exit — gui_cleanup(), then the struct is freed — and the ones the new image's history needs are rebuilt with a fresh gui_init().

In the first two shapes your source is still sitting in the main loop and fires against freed GUI state. In the third, for a base instance, nothing is freed and nothing cancels the source: it fires on a live module and writes a value computed from the previous image into the new image's widgets.

The if(!self->gui_data) return G_SOURCE_REMOVE; at the top of both pattern callbacks does not cover all three; what it does on each is worked through below. Cancel the source instead.

Cancel in gui_cleanup(). If the source data is self, that is one line:

void gui_cleanup(dt_iop_module_t *self)
{
  ...
  // g_idle_remove_by_data() drops one source per call, so drain
  while(g_idle_remove_by_data(self)) ;
}

gui_cleanup() is not the only cancellation point, because it does not run on every transition that invalidates your payload. A base instance survives an image switch, so a source queued while the old image was loaded fires against the new one. Drain in change_image() too — that is the callback the framework gives a retained instance for exactly this kind of reset, and it runs while the three screen-pipe mutexes are still held, so no pipe worker thread can queue a source behind the drain. The modules that implement change_image() use it to clear GUI state; they are listed in IOP_Module_API.md:

void change_image(dt_iop_module_t *self)
{
  while(g_idle_remove_by_data(self)) ;
  ...   // reset the rest of gui_data for the new image
}

If the queued value can simply be recomputed, cancelling is the whole fix: the new image's first pipe run queues a fresh update.

The loop is easy to drop, because a single call looks like it cancels. This would be wrong:

void gui_cleanup(dt_iop_module_t *self)
{
  ...
  g_idle_remove_by_data(self);   // removes ONE source, and returns whether it did
}

process() can queue a source on every qualifying run, so several can be outstanding when the GUI is torn down, and one call is not guaranteed to remove them all. src/bauhaus/bauhaus.c drains in a while loop and is the model for that line.

Why the loop is not a detail. Every one of these transitions holds the screen-pipe mutexes before it touches a module GUI, so nothing can queue a fresh source once gui_cleanup() — or change_image() — has started. Draining there is therefore sufficient, as long as it actually drains.

If the source data is a heap message (Pattern B), g_idle_remove_by_data() cannot find it — the source is keyed on the message, not on the module. You then have to track the source ids yourself: keep the id in gui_data, and queue with g_idle_add_full() passing g_free as the GDestroyNotify. Drop the g_free(msg) from the callback if you do — the notification runs after a normal dispatch as well as on cancellation, so keeping both frees the message twice. A second update also has to supersede the first rather than overwrite its id unremoved. That bookkeeping is why Pattern B's opening preference for Pattern A applies.

What the gui_data Check Actually Does

dt_iop_gui_cleanup_module() sets module->gui_data to NULL after freeing it, and teardown and idle dispatch both run on the GTK main thread, so they cannot interleave. That is enough to make if(!self->gui_data) return G_SOURCE_REMOVE; meaningful on one of the three shapes and not on the other two:

• For a deleted instance it is a real guard. The module struct is still there, parked in dev->alliop, and gui_data has been cleared, so the check sees NULL and returns; without it the callback dereferences NULL and the process stops there.
• On darkroom exit, and for an extra instance on an image switch, the struct itself is freed right after its GUI, so reading self->gui_data is the use-after-free. The check comes too late whatever it returns — and what it returns is undefined: the freed bytes may still hold the NULL that cleanup wrote, or anything the allocator has since put there.
• For a base instance on an image switch the check is blind rather than late. Nothing is freed, gui_data is still allocated, so the test passes and the callback runs to completion against the wrong image.

Where the check does work it has to come first, ahead of dt_iop_gui_enter_critical_section(), which would otherwise lock a destroyed mutex. Treat it as a fallback check, never as a substitute for cancelling: only the drain covers all three shapes.

Common Mistakes

Each row is one WRONG line and the section that explains it.

Mistake	WRONG	Explained in
GTK call from process(), directly or through a helper	gtk_label_set_text(g->label, "value");	Which Thread Am I On?
Assuming commit_params() runs on the GTK thread	void commit_params(...) { gtk_widget_queue_draw(g->area); }	Which Thread Am I On?
No critical section when the pipe writes gui_data — queueing the GUI read for later does not synchronize the write	g->computed_value = result; in process(), then g_idle_add(_update_gui, self);	Using gui_data from commit_params()
Entering the critical section without checking that the GUI exists	dt_iop_gui_enter_critical_section(self); in commit_params(), unguarded	Using gui_data from commit_params()
No critical section in a widget callback either, when the pipe reads the field	g->cache_valid = FALSE; in a slider callback	Writing gui_data from a Widget Callback
Treating a reprocess request as a barrier	dt_dev_reprocess_center(self->dev, self->iop_order); after writing a shared field	Writing gui_data from a Widget Callback
Calling a locking helper from inside a critical section	_update_cache(self); between enter and leave	The Lock Is Recursive
Holding gui_lock across a call that takes history_mutex or a pipe's busy_mutex	g->exposure = _value_at_cursor(self); between enter and leave, where the helper backtransforms the cursor	gui_lock Is the Innermost Lock
Locking the pointer load and not the pointee	my_cache_t *c = g->cache; under the lock, use_cache(c); after it	Hold the Lock as Long as the Value Must Stay Valid
Calling a dt_preview_data_* accessor from inside a critical section, or from the fill or resize callback	dt_preview_data_get(&g->pd, x, y, 0, &v); between enter and leave, after testing a validity flag	The Framework Service for Per-Pixel Readouts
No pipe test at all, so every pipe queues its own update	if(g != NULL) g_idle_add(...);	Guards Before Sending GUI Updates
Forgetting to free the Pattern B message — or freeing it twice	return G_SOURCE_REMOVE; with no g_free(data), or one alongside a g_free GDestroyNotify	The Callback Must Not Outlive the Module or the Image
Queued callback with no cancellation	g_idle_add(_update_gui, self); with no drain in gui_cleanup() or change_image()	The Callback Must Not Outlive the Module or the Image