`exposure`: `_show_computed()` idle source can outlive the module

exposure: _show_computed() idle source can outlive the module

Found by code analysis, 2026-08-24.
Re-checked against 3c73bf2aaa on 2026-09-03; citations refreshed and two of the three
gaps restated — see Corrections at the end.

Severity: latent. No known crash; the callback only sets a label today. It is one net
short of the documented pattern in three separate ways.

Summary

_process_common_setup() queues a GTK idle source from the pipe thread to display the
computed deflicker correction:

506:    // second, show computed correction in UI.
507:    if(g && dt_pipe_is_preview(piece->pipe))
508:    {
509:      dt_iop_gui_enter_critical_section(self);
510:      g->deflicker_computed_exposure = exposure;
511:      dt_iop_gui_leave_critical_section(self);
512:
513:      g_idle_add(_show_computed, self);
514:    }

_process_common_setup() is called from process() (:560) and process_cl() (:532),
so the source is queued once per preview-pipe run of the module, not once per commit. The
whole block is inside if(d->deflicker) (:487), and d->deflicker is only set for
EXPOSURE_MODE_DEFLICKER on a raw single-channel TYPE_UINT16 image (commit_params,
:644-650) — that is the reachability envelope for everything below.

Three gaps

1. Cancellation removes one source, not all. gui_cleanup() (:1346) calls
   g_idle_remove_by_data(self) once (:1356), but the pipe can queue several before
   any runs. dev-doc/GUI_Threading.md:887-888 gives the correct form and
   :912-925 explains why the single call looks sufficient and is not:

   // g_idle_remove_by_data() drops one source per call, so drain
   while(g_idle_remove_by_data(self)) ;
   

2. The callback locks before it checks. _show_computed() (:1030-1047) has no
   if(!g) return G_SOURCE_REMOVE;, and the order of what it does instead matters:

   1030: static gboolean _show_computed(gpointer user_data)
   1031: {
   1032:   dt_iop_module_t *self = user_data;
   1033:   dt_iop_exposure_gui_data_t *g = self->gui_data;
   1034:
   1035:   dt_iop_gui_enter_critical_section(self);
   1036:   if(g->deflicker_computed_exposure != EXPOSURE_CORRECTION_UNDEFINED)
   

   :1033 only loads the pointer, so a NULL gui_data is still harmless there. :1035
   locks self->gui_lock (src/develop/imageop.h:344-348), and
   dt_iop_gui_cleanup_module() destroys that mutex at src/develop/imageop.c:2435,
   before it clears gui_data at :2436-2437 and after it has already run the module's
   own gui_cleanup() at :2428. So on the deleted-instance and undo/redo shapes — where
   the struct survives, parked in dev->alliop (dev-doc/GUI_Threading.md:861-863) — a
   surviving source locks a destroyed mutex at :1035 and never reaches the deref at
   :1036 that the missing check was meant to cover.

   The doc says this directly: "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"
   (:960-961). The fix below is unchanged — the check goes at the top of the function --
   but it is a fallback for one of the three teardown shapes, not the fix. On darkroom exit
   and for an extra instance on an image switch the struct itself is freed
   (src/views/darkroom.c:4328-4339 and :1530-1544 respectively), so reading
   self->gui_data at :1033 is already the
   use-after-free and no check placed anywhere helps (dev-doc/GUI_Threading.md:951-955).
   Only the drain covers all three.

3. No change_image(), so a queued source can survive an image switch. exposure
   implements none, and its base instance is retained across the switch
   (src/views/darkroom.c:1522-1529), so a source queued while the old image was loaded
   fires against the new one. dev-doc/GUI_Threading.md:892-907 names change_image() as
   the second cancellation point for exactly this.

   The source firing late is real; the wrong-image label write it would cause is not,
   because the field it reads has been reset by then. gui_update() (:673) blanks
   deflicker_used_EC at :725 and writes
   deflicker_computed_exposure = EXPOSURE_CORRECTION_UNDEFINED at :727, and it is
   reached on every switch: _dev_load_requested_image() calls
   dt_dev_pop_history_items() at src/views/darkroom.c:1601, which runs
   dt_iop_gui_update() over every module in dev->iop
   (src/develop/develop.c:1791-1797). All of that is on the GTK thread, so it completes
   before any queued idle source can run, and the guard at :1036 then suppresses the
   write.

   That makes the gap latent for a second reason, and a fragile one: what protects the new
   image is a value-level reset in an unrelated callback, not the source-level cancel the
   pattern asks for. Any payload that does not go through
   EXPOSURE_CORRECTION_UNDEFINED — which is what "it will not stay harmless if the
   callback ever grows" amounts to here — loses the protection without anything in
   _show_computed() changing.

Suggested fix

• while(g_idle_remove_by_data(self)) ; in gui_cleanup(), replacing the single call at
  :1356.
• if(!g) return G_SOURCE_REMOVE; at the top of _show_computed(), above the
  dt_iop_gui_enter_critical_section() at :1035, as a fallback for the deleted-instance
  shape.
• A change_image() that drains the queue and resets deflicker_computed_exposure to
  EXPOSURE_CORRECTION_UNDEFINED. The reset duplicates what gui_update() already does
  at :727; the drain is the part that is missing.

Corrections to the version filed as #22007

• Line numbers refreshed against 3c73bf2aaa. The filed report cites gui_cleanup at
  :1365 (now :1356), _show_computed at :1046-1063 (now :1030-1047) and the
  queueing block at :504-512 (now :506-514).
• Gap 2 was stated as a missing NULL check reached before a deref. What the code reaches
  first is the lock, so on the one shape where the check would work it never gets there.
• Gap 3 was stated as harmless "because it only writes a label". The reason it is harmless
  is narrower and worth recording: gui_update() resets the value the callback tests.

Environment

Present in 56898c27b8; re-verified against 3c73bf2aaa, where src/ matches upstream
master as of 2026-09-03. Static analysis only — nothing verified at runtime.

Start with _process_common_setup(), gui_cleanup(), _show_computed(), and the module lifecycle guidance in dev-doc/GUI_Threading.md; then trace change_image() and the teardown paths cited in src/views/darkroom.c. Done means queued sources are drained during cleanup and image changes, the callback checks gui_data before locking, and the computed exposure is reset as described.