Skip to content

Commit

Permalink
Added locking for history and style operations
Browse files Browse the repository at this point in the history
As develop history_mutex and global db_image mutexes are both of recursive type we
can and should use them for locking the history or image struct while modifying history.
  • Loading branch information
jenshannoschwalm committed Jan 28, 2025
1 parent 46cd9cc commit 9c71889
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 7 deletions.
21 changes: 14 additions & 7 deletions src/common/styles.c
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,7 @@ void dt_styles_apply_style_item(dt_develop_t *dev,
const gboolean append)
{
// get any instance of the same operation so we can copy it
dt_pthread_mutex_lock(&dev->history_mutex);
dt_iop_module_t *mod_src =
dt_iop_get_module_by_op_priority(dev->iop, style_item->operation, -1);

Expand Down Expand Up @@ -797,6 +798,7 @@ void dt_styles_apply_style_item(dt_develop_t *dev,
free(module);
}
}
dt_pthread_mutex_unlock(&dev->history_mutex);
}

void _styles_apply_to_image_ext(const char *name,
Expand Down Expand Up @@ -997,27 +999,32 @@ void dt_styles_apply_to_image(const char *name,
const gboolean overwrite,
const dt_imgid_t imgid)
{
dt_lock_image(imgid);
_styles_apply_to_image_ext(name, duplicate, overwrite, imgid, TRUE);
dt_unlock_image(imgid);
}

void dt_styles_apply_to_dev(const char *name, const dt_imgid_t imgid)
{
if(!darktable.develop || !dt_is_valid_imgid(darktable.develop->image_storage.id))
dt_develop_t *dev = darktable.develop;
if(!dev || !dt_is_valid_imgid(dev->image_storage.id))
return;

dt_pthread_mutex_lock(&dev->history_mutex);
/* write current history changes so nothing gets lost */
dt_dev_write_history(darktable.develop);
dt_dev_write_history(dev);

dt_dev_undo_start_record(darktable.develop);
dt_dev_undo_start_record(dev);

/* apply style on image and reload*/
_styles_apply_to_image_ext(name, FALSE, FALSE, imgid, FALSE);
dt_dev_reload_image(darktable.develop, imgid);

DT_CONTROL_SIGNAL_RAISE(DT_SIGNAL_TAG_CHANGED);
dt_dev_reload_image(dev, imgid);

/* record current history state : after change (needed for undo) */
dt_dev_undo_end_record(darktable.develop);
dt_dev_undo_end_record(dev);
dt_pthread_mutex_unlock(&dev->history_mutex);

DT_CONTROL_SIGNAL_RAISE(DT_SIGNAL_TAG_CHANGED);

// rebuild the accelerators (style might have changed order)
dt_iop_connect_accels_all();
Expand Down
10 changes: 10 additions & 0 deletions src/develop/develop.c
Original file line number Diff line number Diff line change
Expand Up @@ -621,9 +621,11 @@ void dt_dev_load_image(dt_develop_t *dev,
// we need a global lock as the dev->iop set must not be changed
// until read history is terminated
dt_pthread_mutex_lock(&darktable.dev_threadsafe);
dt_pthread_mutex_lock(&dev->history_mutex);
dev->iop = dt_iop_load_modules(dev);

dt_dev_read_history_ext(dev, dev->image_storage.id, FALSE);
dt_pthread_mutex_unlock(&dev->history_mutex);
dt_pthread_mutex_unlock(&darktable.dev_threadsafe);

dev->first_load = FALSE;
Expand Down Expand Up @@ -797,6 +799,7 @@ static void _dev_add_history_item_ext(dt_develop_t *dev,
const gboolean auto_name_module)
{
// try to auto-name the module based on the presets if possible
dt_pthread_mutex_lock(&dev->history_mutex);
if(auto_name_module)
{
_dev_auto_module_label(dev, module);
Expand Down Expand Up @@ -938,6 +941,7 @@ static void _dev_add_history_item_ext(dt_develop_t *dev,
// possibly save database and sidecar file
if(dev->autosaving)
_dev_auto_save(dev);
dt_pthread_mutex_unlock(&dev->history_mutex);
}

const dt_dev_history_item_t *dt_dev_get_history_item(dt_develop_t *dev, const char *op)
Expand Down Expand Up @@ -1072,6 +1076,7 @@ void dt_dev_add_masks_history_item_ext(dt_develop_t *dev,
{
dt_iop_module_t *module = _module;
gboolean enable = _enable;
dt_pthread_mutex_lock(&dev->history_mutex);

// no module means that is called from the mask manager, so find the iop
if(module == NULL)
Expand All @@ -1094,6 +1099,7 @@ void dt_dev_add_masks_history_item_ext(dt_develop_t *dev,
else
dt_print(DT_DEBUG_ALWAYS,
"[dt_dev_add_masks_history_item_ext] can't find mask manager module");
dt_pthread_mutex_unlock(&dev->history_mutex);
}

void dt_dev_add_masks_history_item(
Expand Down Expand Up @@ -1156,6 +1162,7 @@ void dt_dev_reload_history_items(dt_develop_t *dev)
dev->focus_hash = 0;

dt_lock_image(dev->image_storage.id);
dt_pthread_mutex_lock(&dev->history_mutex);

dt_ioppr_set_default_iop_order(dev, dev->image_storage.id);
dt_dev_pop_history_items(dev, 0);
Expand Down Expand Up @@ -1211,11 +1218,13 @@ void dt_dev_reload_history_items(dt_develop_t *dev)
// set the module list order
dt_dev_reorder_gui_module_list(dev);

dt_pthread_mutex_unlock(&dev->history_mutex);
dt_unlock_image(dev->image_storage.id);
}

void dt_dev_pop_history_items_ext(dt_develop_t *dev, const int32_t cnt)
{
dt_pthread_mutex_lock(&dev->history_mutex);
dt_ioppr_check_iop_order(dev, 0, "dt_dev_pop_history_items_ext begin");

const int end_prev = dev->history_end;
Expand Down Expand Up @@ -1287,6 +1296,7 @@ void dt_dev_pop_history_items_ext(dt_develop_t *dev, const int32_t cnt)
}
if(masks_changed)
dt_masks_replace_current_forms(dev, forms);
dt_pthread_mutex_unlock(&dev->history_mutex);
}

void dt_dev_pop_history_items(dt_develop_t *dev, const int32_t cnt)
Expand Down

0 comments on commit 9c71889

Please sign in to comment.