liquify: g->dragging survives the deletion of the node it points at

Found by code analysis, 2026-09-03.

Severity: low. One swallowed click and one swallowed pan in the darkroom, and it
clears itself on the next primary button release. Reported because it violates an
invariant the module states in its own comments, and because one further interaction —
undo/redo while the state is live — is not bounded the same way (see An escalation
that is not bounded, below).

Summary

_btn_make_radio_cb() (src/iop/liquify.c:3534) deletes the node g->temp points at
without ending the drag that points at the same node:

3555:  if(g->status & DT_LIQUIFY_STATUS_PREVIEW)
3556:  {
3557:    node_delete(p, g->temp);
3558:    g->temp = NULL;
3559:    g->status &= ~DT_LIQUIFY_STATUS_PREVIEW;
3560:  }

There is no end_drag() anywhere in the function. g->dragging.elem (:273) still
holds the address of the slot node_delete() has just invalidated, so is_dragging()
(:1339-1342, return g->dragging.elem != NULL) keeps returning TRUE with no mouse
button held.

The drag it inherits was never a real drag. _start_new_shape() (:3187-3209) opens
the stamp preview — "start the preview mode to show the shape that will be created",
:3596 — and starts a drag on it with no button down:

3198:  g->temp = alloc_move_to(self, pt);
...
3201:  g->status |= DT_LIQUIFY_STATUS_PREVIEW;
...
3207:  start_drag(g, DT_LIQUIFY_LAYER_CENTERPOINT, g->temp);
3208:  g->last_hit = NOWHERE;

The field's own comment at :273 reads "Element being dragged with mouse button",
which is the state _start_new_shape() does not create and _btn_make_radio_cb() then
fails to clear.

Why this is not a judgement call

The module states the rule twice in its own source, in button_pressed(), and obeys it
at both sites:

3130:    // always end dragging before manipulating the path list to avoid
3131:    // dangling pointers
3132:    end_drag(g);

3149:    // always end dragging before manipulating the path list to avoid
3150:    // dangling pointers
3151:    end_drag(g);

Every other place that removes the node under construction calls end_drag() first:
button_released():3234 before g->temp = NULL, and button_released():3291 before the
right-click cancel's node_delete(p, g->temp) at :3296. :3557 is the one deletion
that does not.

How a user reaches it

The pseudo-drag holds no pointer grab — the module takes none, grab does not appear in
liquify.c — so the tool buttons keep taking clicks while it is live. The guard at the
head of the callback does not stop any of this:

3543:  if(is_dragging(g) && g->temp && node_prev(p, g->temp))

A fresh move_to has header.prev == -1 (node_alloc:342-355 sets it), so node_prev()
returns NULL and the callback proceeds.

Three of the four ways out of _btn_make_radio_cb() leave the stale drag behind:

exit	stale drag?
:3565-3573, btn == NULL from gui_focus()	yes
:3598-3603, point/line/curve tool	no — the _start_new_shape() call at :3602 re-points dragging at the freshly allocated node
:3598-3603, node tool	yes — no _start_new_shape()
:3607-3610, the active tool clicked off	yes

So: click the point tool, then click the node tool, with no canvas click in between.
Losing module focus does it too — gui_focus():2737-2745 calls
_btn_make_radio_cb(NULL, FALSE, self) at :2743.

gui_reset():3711-3718 is not a path to it: it clears g->dragging, g->temp and
g->status at :3714-3716 before making the same call, and the cleared status keeps the
:3555 block from running at all.

What the resulting state actually is

Not a dangling pointer, despite what the comments at :3130-3131 warn about.
dragging.elem is a raw dt_liquify_path_data_t * into the inline array
self->params->nodes[MAX_NODES] (:254-257; MAX_NODES is 100 at :47), so it is
always in bounds and always points into live storage.

Nor does it come to point at a different node. node_delete():426-447 invalidates the
slot at :445-446 and calls node_gc():395-424, which does compact
(p->nodes[e] = p->nodes[e+1], :410) — but the array is kept hole-free by
construction: node_alloc() hands out the first invalidated slot, and node_gc()
invalidates everything past the last live node (:421-423). A PREVIEW g->temp is
therefore always the topmost live slot, and deleting the top slot moves nothing.

The realised state is an invalidated slot: header.type == DT_LIQUIFY_PATH_INVALIDATED
(0), header.prev == header.next == -1. mouse_moved()'s drag branch reads it
(:2870-2881), and node_next() / node_prev() on it return NULL, so the CENTERPOINT
switch at :2883-2900 falls through default: break at :2898-2899 without writing
anything.

Consequence

is_dragging() being stuck TRUE changes three things until the next primary release:

• hit testing is skipped. mouse_moved:2826 (if(!is_dragging(g) && _layers_showing(g)))
  and button_pressed:3120 both gate on it, so hovering highlights nothing and the next
  click leaves g->last_hit at the NOWHERE that _start_new_shape:3208 left. With the
  node tool that click selects nothing.
• panning is swallowed. mouse_moved reaches handled = TRUE at :2981 through the
  drag branch, and darkroom.c:4498-4502 only pans when the module did not handle the
  motion, so a primary-button drag on the canvas does nothing but the edge auto-scroll at
  darkroom.c:4508-4511.
• the overlay drops its interpolated paths. :1815 makes interpolated NULL while
  is_dragging(g), so the interpolated warp preview disappears from the canvas.

It self-clears at button_released:3281-3284, which calls end_drag() on any primary
release while is_dragging(). That release is itself consumed (handled = 2), which is
the swallowed click.

An escalation that is not bounded

The "invalidated slot" argument above holds only as long as the params content does not
change under the stale pointer. Undo, redo and the history slider go through
dt_dev_pop_history_items() (src/develop/develop.c:1780), which replays into the same
buffer — memcpy(hist->module->params, hist->params, ...) at develop.c:1738, after
resetting every module to defaults at :1711 — and then calls dt_iop_gui_update() on
each module at :1795. liquify's gui_update() (liquify.c:3634-3637) only calls
update_warp_count(); nothing clears g->dragging, and the module implements no
change_image().

So an undo to a state with more nodes can refill the invalidated slot with a live node
while dragging.elem still points at it, after which the next mouse motion over the
canvas moves that node to the cursor (:2890-2896) with no button pressed. That would be
silent geometry corruption rather than a swallowed click.

This chain was not exercised. It is read off the code above and nothing more; the
severity assigned to this report is for the observed swallowed click and pan. Anyone
triaging it should decide whether the undo interaction is real before rating it.

Suggested fix

Follow the module's own rule:

  if(g->status & DT_LIQUIFY_STATUS_PREVIEW)
  {
    // always end dragging before manipulating the path list
    end_drag(g);
    node_delete(p, g->temp);
    g->temp = NULL;
    g->status &= ~DT_LIQUIFY_STATUS_PREVIEW;
  }

The _start_new_shape() call at :3602 re-arms the drag on the tool-switch path that
needs it, so clearing it here costs nothing there. The alternative — calling end_drag()
at the top of the function, before the :3543 guard — would change what that guard does
and is not what the other sites do.

Related reports

• #22134 — _start_new_shape():3199 dereferences alloc_move_to() without a NULL
  check and segfaults on a full path. Same two functions, independent defects: that one
  is a missing NULL check, this one a missing end_drag(). Fixing #22134 by returning
  early leaves g->temp NULL and the PREVIEW bit clear, which is the state the
  :3555 block would have produced anyway, so it neither fixes nor worsens this one.
• Apart from #22134 the corpus holds no liquify report. Not checked against
  upstream — no gh in this environment, so a pre-existing GitHub issue for this
  defect cannot be ruled out.

Environment

Verified line by line against 3c73bf2aaa (branch dt-lockcheck; src/ matches the
2026-09-03 upstream rebase). A few citations differ by one to five lines from row 24 of
discipline-gap-validated.md, which was written against 4d9e40f30e: node_gc's
compaction is :410 not :407, gui_focus() is :2737-2745 not :2740-2744, and the
darkroom pan branch is darkroom.c:4498-4502 not :4499-4503. The numbers here are the
current ones. Static analysis only — nothing here was reproduced at runtime.

Start in src/iop/liquify.c at _btn_make_radio_cb() around lines 3534-3610, then compare its preview-node deletion with end_drag() calls in button_pressed() and button_released(). Verify the point-to-node and focus-loss paths no longer leave is_dragging() true after the preview node is deleted, and inspect the undo/redo path through src/develop/develop.c if validating the reported escalation.