darktable-org / darktable

darktable is an open source photography workflow application and raw developer
https://www.darktable.org
GNU General Public License v3.0
9.55k stars 1.13k forks source link

InputNG: darktable crashs with X Touch Mini device on start if arbitrary shortcuts are defined #10786

Closed MStraeten closed 2 years ago

MStraeten commented 2 years ago

latest changes in InputNG introduced following issue:

darktable crashes on start if shortcuts are defined (even no midi shortcuts) and the XTocuh mini is connected:

(darktable:32033): Gtk-CRITICAL **: 14:22:06.924: gtk_window_add_accel_group: assertion 'GTK_IS_WINDOW (window)' failed
[midi_open_devices] opened midi device 'X-TOUCH MINI' via 'CoreMIDI' as midi0
libdarktable.dylib was compiled with optimization - stepping may behave oddly; variables may not be available.
Process 32033 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x40)
    frame #0: 0x000000010096f47a libdarktable.dylib`_process_shortcut [inlined] _shortcut_match(f=0x00007ff7bfefc4f0, fb_log=0x00007ff7bfefc528) at accelerators.c:2734:14 [opt]
   2731
   2732 static gboolean _shortcut_match(dt_shortcut_t *f, gchar **fb_log)
   2733 {
-> 2734   f->views = darktable.view_manager->current_view->view(darktable.view_manager->current_view);
   2735   gpointer v = GINT_TO_POINTER(f->views);
   2736
   2737   GSequenceIter *existing = g_sequence_search(darktable.control->shortcuts, f, _shortcut_compare_func, v);
Target 0: (darktable) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x40)
  * frame #0: 0x000000010096f47a libdarktable.dylib`_process_shortcut [inlined] _shortcut_match(f=0x00007ff7bfefc4f0, fb_log=0x00007ff7bfefc528) at accelerators.c:2734:14 [opt]
    frame #1: 0x000000010096f472 libdarktable.dylib`_process_shortcut(move_size=NaN) at accelerators.c:2961:6 [opt]
    frame #2: 0x000000010096f3d6 libdarktable.dylib`dt_shortcut_move(id=<unavailable>, time=0, move=1, size=NaN) at accelerators.c:3100:20 [opt]
    frame #3: 0x000000011812ae4f libmidi.so`update_with_move(midi=0x0000600003923b50, timestamp=0, controller=1, move=<unavailable>) at midi.c:251:24 [opt]
    frame #4: 0x000000011812b8ed libmidi.so`_timeout_midi_update(user_data=<unavailable>) at midi.c:516:46 [opt]
    frame #5: 0x0000000100518509 libglib-2.0.0.dylib`g_timeout_dispatch + 20
    frame #6: 0x000000010051b58d libglib-2.0.0.dylib`g_main_context_dispatch + 257
    frame #7: 0x00007ff8097f6cb7 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 23
    frame #8: 0x00007ff8097f6b54 CoreFoundation`__CFRunLoopDoObservers + 543
    frame #9: 0x00007ff8097f6312 CoreFoundation`__CFRunLoopRun + 1652
    frame #10: 0x00007ff8097f55dd CoreFoundation`CFRunLoopRunSpecific + 563
    frame #11: 0x00000001191c7038 libSDL2-2.0.0.dylib`PLATFORM_hid_enumerate + 81
    frame #12: 0x00000001191c8262 libSDL2-2.0.0.dylib`SDL_hid_enumerate_REAL + 44
    frame #13: 0x00000001192719d5 libSDL2-2.0.0.dylib`HIDAPI_UpdateDeviceList + 72
    frame #14: 0x0000000119271f2e libSDL2-2.0.0.dylib`HIDAPI_JoystickDetect + 42
    frame #15: 0x000000011927195f libSDL2-2.0.0.dylib`HIDAPI_JoystickInit + 101
    frame #16: 0x00000001191cbd31 libSDL2-2.0.0.dylib`SDL_JoystickInit + 93
    frame #17: 0x00000001191a6dd8 libSDL2-2.0.0.dylib`SDL_InitSubSystem_REAL + 265
    frame #18: 0x000000011900f50b libgamepad.so`gamepad_open_devices(self=0x00000001140553f0) at gamepad.c:251:6 [opt]
    frame #19: 0x00000001009b2e13 libdarktable.dylib`dt_lib_init_module(m=0x00000001140553f0) at lib.c:757:5 [opt]
    frame #20: 0x0000000100876eac libdarktable.dylib`dt_module_load_modules(subdir=<unavailable>, module_size=472, load_module_so=(libdarktable.dylib`dt_lib_load_module at lib.c:580), init_module=<unavailable>, sort_modules=(libdarktable.dylib`dt_lib_sort_plugins at lib.c:559)) at module.c:59:21 [opt]
    frame #21: 0x00000001009b279c libdarktable.dylib`dt_lib_init(lib=<unavailable>) at lib.c:1045:28 [opt]
    frame #22: 0x00000001007fc876 libdarktable.dylib`dt_init(argc=<unavailable>, argv=<unavailable>, init_gui=<unavailable>, load_data=1, L=0x0000000000000000) at darktable.c:1121:5 [opt]
    frame #23: 0x000000010000ff3a darktable`main(argc=<unavailable>, argv=<unavailable>) at main.c:92:6 [opt]
    frame #24: 0x00000001000214fe dyld`start + 462

To Reproduce Please provide detailed steps to reproduce the behaviour, for example:

  1. delete all shortcutsrc* files in config directory, connect XTouch mini
  2. run darktable --> everything is fine
  3. close darktable, run again --> crash occurs
  4. disconnect Midi device
  5. run darktable --> everything is fine

Which commit introduced the error bisecting in 3.8.x branch gave

95d625dda15b39cf8c4f0a22c1724055247dd8b0 is the first bad commit
commit 95d625dda15b39cf8c4f0a22c1724055247dd8b0
Author: phweyland <philippe.weyland@laposte.net>
Date:   Fri Dec 31 13:50:22 2021 -0300

    tags presets: append preset's tags instead of replacing.

    fixes #10741

 src/libs/tagging.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

bisecting in master gave:

576828122e8bf88bc24069d6a70cf42bdc423703 is the first bad commit
commit 576828122e8bf88bc24069d6a70cf42bdc423703
Author: Diederik ter Rahe <dterrahe@yahoo.com>
Date:   Mon Dec 20 01:11:37 2021 +0100

    make notebook pages selected image[s] lib shortcuttable

 src/libs/image.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Platform

TurboGit commented 2 years ago

A bit strange than bisecting on master & 3.8.x branch gives different result. The 3.8 wrong commit is on tagging.c, seems quite suspicious that this could have introduced an issue with midi devices... Or maybe we have a corrupted memory somewhere and then everything can happen!

parafin commented 2 years ago

This is a duplicate of #10680

MStraeten commented 2 years ago

i doubt, since i cant reproduce this with my build from Januar 1. darktable-3.8.0+35~gc7dfebcac

parafin commented 2 years ago

backtrace is the same nontheless

parafin commented 2 years ago

it wasn't triggered before on your system probably due to timings (timeout not yet pending when enumeration was happening, reversing the order of some init steps changed the timings).

dterrahe commented 2 years ago

It looks like on mac SDL2 calls the gtk idle loop during the initialisation of the gamepad module. Since I haven't seen this behavior on any of the platforms I have access to, finding a solution might be a bit of trial and error where I'll depend on you guys to test. Also, I don't have access to any of my midi/gamepad devices this week so won't even be able to test if I break things on linux or windows.

MStraeten commented 2 years ago

but why this occurs after the merge made today in this case?

dterrahe commented 2 years ago

Did you update any libraries / dependencies on your machine? The code in the backtrace didn't significantly change for a while/months.

Would you please test if simply adding if(!darktable.view_manager->current_view) return FALSE; as the first line of _shortcut_match prevents the issue?

MStraeten commented 2 years ago

the addition fixes the issue for me - as deleting libSDL2 does - but thats not a solution for those using gamepads ;)

i keep macports quite updated so libraries are frequently updated. But during bisecting i was able to get builds without that issue using the same macports config ...

parafin commented 2 years ago

As I noted in another ticket, that's probably just timings changing a bit - timeout is obviously triggered only if some time has passed.

dterrahe commented 2 years ago

10762 performs some more (presets, i.e. database -> slow) initialisation steps during lib setup, which would make it more likely that the first delayed idle callback for the x-touch is timed out before the gamepad initialisation and therefore is handled when libsdl2 hands control back to the gtk idle loop.

Either the lib initalisation steps are much faster on the platforms I use/test or the mac is the only platform where libsdl2 behaves like this. Either way, the additional line to test if we actually have an active view, or haven't yet finished initialising, is correct (should have been there -> fixes an oversight/bug) and possibly fixes the only instance where this problem occurs.