Skip to content

Conversation

@jenshannoschwalm
Copy link
Collaborator

Raising DT_SIGNAL_TAG_CHANGED in dt_history_delete_on_image_ext() possibly leads to a race condition in the _control_work_res() dispatcher, both fight for the imgid lock possibly leading to a standstill on fast systems.
If we want the signal we have the explicit dt_history_delete_on_image() doing that.

Raising DT_SIGNAL_DEVELOP_MIPMAP_UPDATED is done after unlocking imgid.

While being here:

  1. dt_history_hash_write_from_history() is also done while the imgid is locked for history changes.
  2. Functions presented via history.h check for a valid imgid for safety
  3. dt_history_item_as_string() is only used here so static and removed from history.h
  4. dt_history_end_attop() is static so it has been renamed according to standard.
  5. Readabilty for _control_generic_images_job_create()
  6. Tested for all functions called by any g_main_context_invoke() variant and modified the return value for maintenance.

Release note: Fixed a possibly standstill while discarding history on fast systems

Fixes #20151

@jenshannoschwalm jenshannoschwalm added this to the 5.4.1 milestone Jan 15, 2026
@jenshannoschwalm jenshannoschwalm added bugfix pull request fixing a bug priority: high core features are broken and not usable at all, software crashes labels Jan 15, 2026
Raising DT_SIGNAL_TAG_CHANGED in dt_history_delete_on_image_ext() possibly leads to
a race condition in the _control_work_res() dispatcher, both fight for the imgid lock
possibly leading to a standstill on fast systems.
If we want the signal we have the explicit dt_history_delete_on_image() doing that.

Raising DT_SIGNAL_DEVELOP_MIPMAP_UPDATED is done after unlocking imgid.

While being here:
1. dt_history_hash_write_from_history(imgid, DT_HISTORY_HASH_CURRENT) is also done while the
   imgid is locked for history changes.
2. Functions presented via history.h check for a valid imgid for safety
3. dt_history_item_as_string() is only used here so static and removed from history.h
4. dt_history_end_attop() is static so it has been renamed according to standard.
…context_invoke()

Just maintenance for clarity.
@jenshannoschwalm jenshannoschwalm force-pushed the history_delete_race_conditions branch from 35d3f0f to cd695b9 Compare January 15, 2026 12:08
@jenshannoschwalm
Copy link
Collaborator Author

Thanks for fetching the wrong check. I just again cross checked that we signal the tag change whereever we wanted it to do so. Nothing found.

Copy link
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.

Looks good, thanks!

@TurboGit TurboGit merged commit 36d0f30 into darktable-org:master Jan 15, 2026
5 checks passed
@jenshannoschwalm jenshannoschwalm deleted the history_delete_race_conditions branch January 15, 2026 18:02
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Background job "Discard history" does not work properly

2 participants