Skip to content

Fixes race condition during mask resizing#21102

Merged
TurboGit merged 1 commit into
darktable-org:masterfrom
masterpiga:fix_mask_crash
May 23, 2026
Merged

Fixes race condition during mask resizing#21102
TurboGit merged 1 commit into
darktable-org:masterfrom
masterpiga:fix_mask_crash

Conversation

@masterpiga
Copy link
Copy Markdown
Collaborator

@masterpiga masterpiga commented May 23, 2026

Fixes race condition between the pixelpipe worker thread and the UI thread accessing dev->forms concurrently.

Crash report (txt)

During a mask resize operation, the call chain in the pixelpipe worker is:

dt_masks_group_render_roi
  → _group_get_mask_roi          [group.c:651]
    → dt_masks_get_from_id(module->dev, fpt->formid)   ← reads dev->forms LIVE
      → _path_get_mask_roi
        → _path_get_pts_border   ← crashes iterating form->points

At blend.c:595, the outer form is correctly fetched from piece->pipe->forms — a deep copy of dev->forms made at the start of the pipeline run (pixelpipe_hb.c:3092):

pipe->forms = dt_masks_dup_forms_deep(dev->forms, NULL);

But when _group_get_mask_roi needs to look up child forms (the shapes inside a group), it bypasses the pipe's copy and goes directly to dev->forms:

// group.c:651 — BUG
dt_masks_form_t *sel = dt_masks_get_from_id(module->dev, fpt->formid);
//                                           ^^^^^^^^^^^ dev->forms — live, mutable

The same bug exists in _group_get_mask at group.c:333.

While the pixelpipe worker iterates over a child path form's form->points GList in _path_get_pts_border, the UI thread (responding to a mouse resize) can modify that same GList in dev->forms — freeing or relocating nodes. When the worker thread then follows a ->next pointer on a freed GList node, it reads address 0x0000000000000008 (GList.next is at offset 8 on 64-bit) and crashes.


Fix

Changed both unsafe lookups from dev->forms to piece->pipe->forms using dt_masks_get_from_id_ext:

group.c:333 (in _group_get_mask):

// Before:
dt_masks_form_t *sel = dt_masks_get_from_id(module->dev, fpt->formid);

// After:
dt_masks_form_t *sel = dt_masks_get_from_id_ext(piece->pipe->forms, fpt->formid);

group.c:651 (in _group_get_mask_roi):

// Before:
dt_masks_form_t *sel = dt_masks_get_from_id(module->dev, fpt->formid);

// After:
dt_masks_form_t *sel = dt_masks_get_from_id_ext(piece->pipe->forms, fpt->formid);

Both functions already have piece as a parameter with piece->pipe->forms available. dt_masks_get_from_id_ext already exists and takes exactly a GList * (the forms list) — it's already used correctly in blend.c:595 for the outer form lookup.

Co-authored with Claude.

@masterpiga masterpiga added this to the 5.6 milestone May 23, 2026
@masterpiga masterpiga added bugfix pull request fixing a bug priority: high core features are broken and not usable at all, software crashes scope: image processing correcting pixels labels May 23, 2026
Copy link
Copy Markdown
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one, thanks!

@TurboGit TurboGit merged commit 484dc3e into darktable-org:master May 23, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix pull request fixing a bug priority: high core features are broken and not usable at all, software crashes scope: image processing correcting pixels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants