LUA post-import-image event and race conditions.
As of now, LUA post-import-image events are being executed asynchronously. I.e.
- Image importing is done in dt_image_import(): https://github.com/darktable-org/darktable/blob/release-2.2.5/src/common/image.c#L744
- 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
- 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:
- segfaults if image is processed in darkroom and event is applying the style at the same time
- failing of xmp parsing in post import image event if it is done the same moment when darkroom is writing to xmp.
- User becoming confused because of image changing (i.e. event applying styles) in darkroom without his/her input.
- Check for pending "post-import-image" events for selected image before opening it in darkroom and wait for them to finish.
- 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.
#1 Updated by Žilvinas Žaltiena almost 2 years ago
The fix in the second way from proposal list: https://github.com/zaltysz/darktable/commit/684b1b695411ce699a27cebc6ce5f3777f1966aa
#2 Updated by Tobias Ellinghaus almost 2 years 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 almost 2 years ago
- % Done changed from 20 to 100
- Status changed from Triaged to Fixed
Applied in changeset darktable|684b1b695411ce699a27cebc6ce5f3777f1966aa.