retouch: failure during OpenCL auto-level statistics strands the GUI handshake

Found by code analysis during the gui_data threading audit, 2026-08-30, while
confirming that g->preview_levels is not a locking defect.

Status: source-confirmed with high confidence; not reproduced at runtime. There is
no known user report. The available test environment has no OpenCL platform, so a
developer should still reproduce the failure with an OpenCL-capable GUI build and fault
injection at the identified call.

This is not a gui_data locking defect. The normal handshake is synchronized and
the lock is released correctly on the failing path. The defect is a missing state
transition after one narrow class of errors.

Summary

retouch hands auto-level results from the full pixelpipe to the GUI through
g->preview_auto_levels. process_cl() changes the state from 1 (requested) to -1
(owned), releases the GUI mutex, and calls rt_process_stats_cl(). If that helper fails
its host allocation, blocking device read, or blocking device write, process_cl()
jumps to cleanup without restoring the request or publishing results.

The ordinary CPU fallback and later pixelpipe runs only claim state 1; the GUI only
consumes state 2; new GUI requests require state 0. The abandoned -1 therefore
makes auto levels inert for that live GUI/module instance. An image switch resets the
retained base instance. Leaving and re-entering darkroom, or otherwise destroying and
recreating the affected GUI/module instance, also recovers it.

Validated state machine

g->preview_auto_levels (src/iop/retouch.c:127) uses four values:

transition	meaning	current source
0 -> 1	GUI requests work	src/iop/retouch.c:1204,1697,1762
1 -> -1	CPU or OpenCL pipe claims it	:3991,4823
-1 -> 2	pipe publishes results	:4003,4837
2 -> -1	GUI claims published results	:1718
-1 -> 0	GUI finishes consuming them	:1738
any value -> 0	new-image/GUI initialization	:2409

The normal handshake transitions use
dt_iop_gui_enter/leave_critical_section(). The pipe owns g->preview_levels while the
token is -1, writes the array, then publishes 2 while holding the mutex. The GUI
observes 2 under the same mutex, changes it to -1, and then reads the array. These
mutex operations supply the required ordering even though the array copy itself is
outside the critical section.

change_image() writes the reset without gui_lock, but the framework externally
serializes image switching against pipe workers: _dev_load_requested_image() acquires
the preview, full, and preview2 pipe mutexes at src/views/darkroom.c:1417-1451, calls
the retained module's change_image() at :1595, and releases those mutexes at
:1641-1644. The full-pipe worker holds its pipe mutex from
src/develop/develop.c:663 through processing and unlocks at :939.

The failing path

The defect is at src/iop/retouch.c:4818-4839:

dt_iop_gui_enter_critical_section(self);
if(g->preview_auto_levels == 1 && !DT_IN_GUI_UPDATE())
{
  g->preview_auto_levels = -1;                       // :4823, request claimed

  dt_iop_gui_leave_critical_section(self);           // :4825, lock released

  levels[0] = levels[1] = levels[2] = 0;
  err = rt_process_stats_cl(self, piece, devid, in_retouch,
                            roi_rt->width, roi_rt->height, levels);
  if(err != CL_SUCCESS) goto cleanup;                // :4830, leaves state -1

  rt_clamp_minmax(levels, levels);
  for(int i = 0; i < 3; i++) g->preview_levels[i] = levels[i];

  dt_iop_gui_enter_critical_section(self);
  g->preview_auto_levels = 2;                        // :4837, skipped on error
}
dt_iop_gui_leave_critical_section(self);

The cleanup: block at :4869-4874 frees OpenCL working data and returns err; it
does not repair g->preview_auto_levels.

Exact trigger scope

Only an error from rt_process_stats_cl() after 1 -> -1 strands this handshake. That
helper has three direct failure sites (src/iop/retouch.c:4044-4077):

• host allocation failure returns DT_OPENCL_SYSMEM_ALLOCATION (:4052-4061);
• the blocking device-to-host buffer read can return an OpenCL error (:4063-4067);
• the blocking host-to-device buffer write returns its OpenCL result (:4072-4077).

The wrappers return their underlying errors at src/common/opencl.c:3116-3179; the
write wrapper also has an explicit CL_MEM_OBJECT_ALLOCATION_FAILURE return at
:3155-3161.

OpenCL failures before the claim at retouch.c:4823 leave the request at 1, so a CPU
fallback or later run can still serve it. Failures after publication at :4837 leave
valid statistics in state 2, so the GUI can consume them. Those errors must not be
folded into this report or repaired by resetting the token.

No kernel is launched inside rt_process_stats_cl(). A device/driver failure from
earlier queued work might surface around a blocking transfer, but the current source and
OpenCL API contract do not establish a prior kernel failure as the returned error here.
The validated trigger is therefore a statistics allocation/readback/writeback failure,
not a generic OpenCL or kernel failure.

Fallback and notification evidence

On the ordinary direct OpenCL path, pixelpipe_hb.c calls process_cl() at
src/develop/pixelpipe_hb.c:2584-2590, records its returned error at :2616-2624, and
normally invokes _pixelpipe_process_on_CPU() at :2917-2976. The CPU retouch path
requires g->preview_auto_levels == 1 at src/iop/retouch.c:3986-4005, so it skips the
stranded -1 instead of recomputing the requested levels.

A pending pipe stop is checked before the fallback at
src/develop/pixelpipe_hb.c:2643-2666. In that case the job normally restarts through
src/develop/develop.c:853-893; the restarted CPU or OpenCL path also requires state
1, so leaving -1 still loses the request.

Fast OpenCL tiling does not change the result. process_tiling_cl_fast() calls
process_cl() from a sequential tile loop at src/develop/tiling.c:1811-1839 and
returns on the first error. No later tile repairs or reclaims the state.

After a valid full-pipe completion, dt_dev_process_image_job() raises the UI-pipe
finished signal at src/develop/develop.c:925-946. Retouch's callback at
src/iop/retouch.c:1707-1740 consumes only state 2; it ignores both 1 and -1.
An older queued finished callback therefore cannot clear a restored pending request, but
it also cannot recover the current stranded state.

Consequences and recovery

• There is no deadlock or lock leak. process_cl() releases the critical section before
  the fallible helper and never reaches the trailing unlock on the cleanup jump.
• The affected render normally continues through CPU fallback, so no persisted-pixel
  corruption or data loss is established.
• The manual auto-level button and the automatic wavelet-scale triggers stop producing
  new levels because their request transitions require state 0.
• Disabling/re-enabling retouch, rebuilding pipes, changing module order, resetting
  parameters, and ordinary undo/redo do not assign this GUI token and do not repair it.
• change_image() resets the retained instance at src/iop/retouch.c:2409.
  gui_init() also calls change_image() at :2422-2428; consequently darkroom
  re-entry or destruction/recreation of the affected GUI/module instance also recovers.

Severity is low: the trigger is an uncommon OpenCL statistics-transfer or allocation
failure and normal image processing can still fall back to CPU. The failure is sticky
and has no user-facing message, however, so the auto-level control appears to stop
working until the GUI state is reset.

GTK 3 and GTK 4

No GTK-version-dependent behavior affects the defect or the proposed fix. GTK 3 emits
the button's clicked signal in gtk-3.24.52/gtk/gtkbutton.c:1541; GTK 4
has the corresponding emission in gtk-4.23.3/gtk/gtkbutton.c:802,850.
Both only dispatch darktable's GUI request callback. The abandoned token and its repair
are entirely inside darktable's pixelpipe/GUI handoff.

The pending documentation in dev-doc/GUI_Threading.md
matches the traced behavior: pipe callbacks have no GTK-thread affinity, shared
gui_data is synchronized with the module GUI mutex, pipe results return through the
main loop, and retained base instances use change_image() to clear per-image GUI
state. dev-doc/IOP_Module_API.md:498-500 also documents the
normal OpenCL-to-CPU fallback.

Suggested fix

Restore the pending request immediately around the one failing call, under the same GUI
critical section used by the state machine:

err = rt_process_stats_cl(self, piece, devid, in_retouch,
                          roi_rt->width, roi_rt->height, levels);
if(err != CL_SUCCESS)
{
  dt_iop_gui_enter_critical_section(self);
  if(g->preview_auto_levels == -1)
    g->preview_auto_levels = 1;
  dt_iop_gui_leave_critical_section(self);
  goto cleanup;
}

Restoring 1 preserves the unserved request for the immediate CPU fallback or a
restarted run. It does not guarantee that CPU processing happens in the same run because
a pending pipe stop can return first. Resetting to 0 would keep the control usable but
silently discard the request.

The == -1 check is inexpensive defensive hardening; no current image-switch race
requires it because the pipe mutexes exclude that overlap. A separate
claimed_auto_levels flag is unnecessary if the repair remains inline in the sole
claiming branch. The repair should not be moved to the common cleanup: label: failures
before the claim need no state change, and failures after publication must preserve
state 2.

Runtime verification still needed

This report has static control-flow proof, not a hardware reproduction. A developer
should double-check it with a current OpenCL-capable GUI build:

1. Request retouch auto levels on the full pipe and inject a failure specifically from
   rt_process_stats_cl() after the 1 -> -1 claim. Failing its read or write wrapper is
   a narrower seam than failing an arbitrary OpenCL call.
2. Before the fix, confirm that the CPU fallback/restarted run completes but the token
   remains -1, subsequent requests do nothing, and image switching or GUI recreation
   resets it.
3. With the fix, confirm that the token returns to 1, the CPU fallback or restarted
   run publishes 2, and the GUI callback consumes it back to 0.
4. Exercise direct OpenCL processing, fast OpenCL tiling, and a pending pipe-shutdown
   restart. Stress an image switch as a lifecycle check even though the source locks
   exclude concurrent change_image().
5. Verify that failures after publication leave state 2 and are still consumed.

The existing build in the analysis environment was stale and darktable-cltest
reported zero OpenCL platforms; Xvfb, Oclgrind, and POCL were also unavailable. No
runtime result is claimed.

Related

• #22067 covers four genuinely
  unlocked retouch display fields. Those fields have no ownership token; this report is
  a separate, correctly synchronized handshake abandoned on one error path.

Debated and rejected

• “The report is invalid because darktable falls back to CPU.” Rejected. The fallback
  exists, but CPU retouch only claims state 1; the OpenCL error leaves -1
  (retouch.c:3989,4823; pixelpipe_hb.c:2917-2976).
• “Any retouch OpenCL failure strands auto levels.” Rejected as too broad. Only the
  error return from rt_process_stats_cl() occurs between the claim at :4823 and the
  publish at :4837. Earlier errors leave 1; later errors leave 2.
• “A kernel error is a validated trigger.” Rejected. The helper launches no kernel
  and directly reports allocation and blocking buffer-transfer errors. A prior device
  failure surfacing at synchronization remains possible but was not established.
• “The normal handshake is itself a data race.” Rejected. The mutex-protected token
  transfers exclusive ownership of g->preview_levels and supplies ordering. This is a
  missing failure transition, not the unlocked-field defect covered by #22067.
• “Restoring 1 can overwrite change_image()'s new-image reset.” Rejected for the
  current lifecycle. Image switching holds all three screen-pipe mutexes across
  change_image(), while process_cl() runs under the full-pipe mutex
  (darkroom.c:1417-1451,1595,1641-1644; develop.c:663-939).
• “After a failed fast tile, another concurrent tile can claim the restored request.”
  Rejected. The tile loop is sequential and immediately goes to finish on the first
  process_cl() error (tiling.c:1811-1863).
• “The fix needs a broad cleanup flag and restoration from common cleanup.” Rejected.
  There is one relevant failing call inside the claiming branch. Common cleanup also
  handles errors before the claim and after successful publication, where restoring
  1 would be wrong.
• “Resetting to 0 is the preferred safe fix.” Rejected as the preferred behavior.
  It avoids a stuck token but discards the user's unserved request; 1 lets normal
  fallback or restart complete it. A 0 reset remains a conservative product choice if
  deliberately dropping the request is desired.
• “Only an image switch can recover.” Rejected as absolute wording. Image switching
  is the in-session reset for the retained base instance, but darkroom re-entry or
  destruction/recreation of the affected GUI/module instance also initializes the token
  to 0.

Start with the state transitions and failing call in src/iop/retouch.c:4818-4839, then inspect rt_process_stats_cl() at :4044-4077 and the fallback paths in src/develop/pixelpipe_hb.c. Use an OpenCL-capable GUI build with failure injection at the statistics transfer, and verify the request is recoverable while failures before claim and after publication retain their existing states.