Project

General

Profile

Bug #11674

LUA post-import-image event and race conditions.

Added by Žilvinas Žaltiena over 1 year ago. Updated over 1 year ago.

Status:
Fixed
Priority:
Low
Category:
Lua
Target version:
Start date:
07/26/2017
Due date:
% Done:

100%

Affected Version:
2.2.5
System:
other GNU/Linux
bitness:
64-bit
hardware architecture:
amd64/x86

Description

As of now, LUA post-import-image events are being executed asynchronously. I.e.

  1. Image importing is done in dt_image_import(): https://github.com/darktable-org/darktable/blob/release-2.2.5/src/common/image.c#L744
  2. After the image is imported, signal DT_SIGNAL_IMAGE_IMPORT fires in dt_image_import(): https://github.com/darktable-org/darktable/blob/release-2.2.5/src/common/image.c#L968 . That signal is connected to relevant callback here: https://github.com/darktable-org/darktable/blob/release-2.2.5/src/lua/database.c#L274
  3. Callback executes lua event "post-import-image" asynchronously with dt_lua_async_call_alien(): https://github.com/darktable-org/darktable/blob/release-2.2.5/src/lua/database.c#L221

The issues with this approach: race condition between processing in darkroom and lua event.

As post import image event is executed asynchronously, it is possible for image to be opened in darkroom while event is still being executed. This can lead to:
  1. segfaults if image is processed in darkroom and event is applying the style at the same time
  2. failing of xmp parsing in post import image event if it is done the same moment when darkroom is writing to xmp.
  3. User becoming confused because of image changing (i.e. event applying styles) in darkroom without his/her input.
My proposal is to either:
  1. Check for pending "post-import-image" events for selected image before opening it in darkroom and wait for them to finish.
  2. Get rid of asynchronous calling of "post-import-image" using dt_lua_async_call_alien(), and replace it with synchronous calling using dt_lua_event_trigger() like it is already done for "pre-import" event.

Associated revisions

Revision 684b1b69
Added by Žilvinas Žaltiena over 1 year ago

dt_image_import(): call lua post-import-image event synchronously. Fixes #11674

Revision c7a02c76
Added by Jérémy Rosen over 1 year ago

Merge pull request #1513 from zaltysz/lua_sync_call_post_image_import

dt_image_import(): sync call lua post-import-image event. Fixes #11674

History

#2 Updated by Tobias Ellinghaus over 1 year ago

  • % Done changed from 10 to 20
  • Status changed from Confirmed to Triaged

Making the Lua script code be run synchronously wouldn't fix the problem because calling it in a signal handler makes the whole thing async already. It just lowers the probability for the racecondition to hit us. So putting the call directly into image.c seems like the right thing to do.

#3 Updated by Žilvinas Žaltiena over 1 year ago

  • % Done changed from 20 to 100
  • Status changed from Triaged to Fixed

#4 Updated by Roman Lebedev over 1 year ago

  • Target version set to 2.4.0

Also available in: Atom PDF