Skip to content

Fixes raster masks not being properly released#21243

Open
masterpiga wants to merge 1 commit into
darktable-org:masterfrom
masterpiga:raster
Open

Fixes raster masks not being properly released#21243
masterpiga wants to merge 1 commit into
darktable-org:masterfrom
masterpiga:raster

Conversation

@masterpiga
Copy link
Copy Markdown
Collaborator

@masterpiga masterpiga commented Jun 5, 2026

What this fixes

is_raster_mask_used() only ever checked the table membership — never whether the consumer is deleted, de-synced, in raster mode, or even enabled — so a disabled consumer could keep the producer publishing and invalidating all downstream cachelines every run.

The fix

dt_iop_prune_stale_raster_users now drops a registered consumer when any of these holds — all evaluated without ever dereferencing a freed pointer:

  • not in the module list → instance deleted
  • sink.source != this → de-synced
  • !sink->enabled → disabled
  • !(mask_mode & DEVELOP_MASK_RASTER) → not in raster mode

The approach is self-correcting: re-enable a previously disabled module (or switch it back to a raster mask) and commit_blend_params re-registers it before the next prune, so a producer resumes publishing exactly when a real consumer needs it.

Should it wait for 5.8 (or 5.6.1)?

@TurboGit @jenshannoschwalm you be the judge. Unless I overlooked something very obvious, it is a pretty inoffensive change that brings significant improvements.

Co-authored with Claude.

@masterpiga masterpiga added bugfix pull request fixing a bug scope: performance doing everything the same but faster labels Jun 5, 2026
@masterpiga masterpiga requested a review from TurboGit June 5, 2026 19:10
@jenshannoschwalm
Copy link
Copy Markdown
Collaborator

Thanks

  1. Nicely fixes expensive writing rastermasks and pixelpipe cache handling if early history steps declared using the rastermask and late history did not. Until now you had to compress history to achieve this. I had an in-depth look at it, tested and indeed seems to be very safe.
  2. why not static _iop_prune_stale_raster_users() as it`s used exclusively in pixelpipe_hb.c and there seems to be no other use case?

@masterpiga
Copy link
Copy Markdown
Collaborator Author

2. why not static _iop_prune_stale_raster_users() as it`s used exclusively in pixelpipe_hb.c and there seems to be no other use case?

Makes sense, I undid the changes to the public api.

@TurboGit
Copy link
Copy Markdown
Member

TurboGit commented Jun 6, 2026

I tend for waiting for 5.8 to stay safe for the release.

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 scope: performance doing everything the same but faster

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants