crash when switching images in darkroom and connected midi device used for accelerators

[MStraeten]

if my XTouch Mini is connected and used for accelerators, switching images in darkroom via space, backspace or selecting an image in the filmstrip. Unfortunately hard to reproduce - if it occurs, then it's reproducible but removing the midi device and attaching this later can be fine for log time. I tried to find a history stack to get a reproducible minimum scenario - but havn't found one...

crash log: darktable-2021-11-14-213335.txt

backtrace

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x28)
    frame #0: 0x0000000100817b7b libdarktable.dylib`_action_process_slider [inlined] dt_bauhaus_slider_get(widget=0x0000000000000000) at bauhaus.c:2394:14 [opt]
   2391 {
   2392   // first cast to bh widget, to check that type:
   2393   const dt_bauhaus_widget_t *w = (dt_bauhaus_widget_t *)DT_BAUHAUS_WIDGET(widget);
-> 2394   if(w->type != DT_BAUHAUS_SLIDER) return -1.0f;
   2395   const dt_bauhaus_slider_data_t *d = &w->data.slider;
   2396   if(d->max == d->min)
   2397   {
Target 0: (darktable) stopped.
(lldb) up
frame #1: 0x0000000100817b3b libdarktable.dylib`_action_process_slider(target=<unavailable>, element=0, effect=1, move_size=NaN) at bauhaus.c:2950:23 [opt]
   2947   GtkWidget *widget = GTK_WIDGET(target);
   2948   dt_bauhaus_widget_t *bhw = DT_BAUHAUS_WIDGET(widget);
   2949   dt_bauhaus_slider_data_t *d = &bhw->data.slider;
-> 2950   const float value = dt_bauhaus_slider_get(widget);
   2951   const float min_visible = powf(10.0f, -dt_bauhaus_slider_get_digits(widget));
   2952
   2953   if(!isnan(move_size))
(lldb)
frame #2: 0x00000001009f1aa4 libdarktable.dylib`_process_shortcut(move_size=<unavailable>) at accelerators.c:2976:21 [opt]
   2973         fsc.effect = DT_ACTION_EFFECT_DEFAULT_UP;
   2974     }
   2975
-> 2976     return_value =  _process_action(fsc.action, fsc.instance, fsc.element, fsc.effect, move_size);
   2977   }
   2978   else if(!isnan(move_size))
   2979   {
(lldb)
frame #3: 0x00000001009f1866 libdarktable.dylib`dt_shortcut_move(id=<unavailable>, time=0, move=<unavailable>, size=NaN) at accelerators.c:3098:20 [opt]
   3095
   3096   float return_value = 0;
   3097   if(isnan(size))
-> 3098     return_value = _process_shortcut(size);
   3099   else
   3100   {
   3101     _cancel_delayed_release();
(lldb)
libmidi.so was compiled with optimization - stepping may behave oddly; variables may not be available.
frame #4: 0x000000011774addf libmidi.so`update_with_move(midi=0x0000600003919ad0, timestamp=0, controller=2, move=<unavailable>) at midi.c:251:24 [opt]
   248
   249  void update_with_move(midi_device *midi, PmTimestamp timestamp, gint controller, float move)
   250  {
-> 251    float new_position = dt_shortcut_move(midi->id, timestamp, controller, move);
   252
   253    if(midi->is_x_touch_mini && controller != 9 && controller != 10)
   254    {
(lldb)
frame #5: 0x000000011774b8ed libmidi.so`_timeout_midi_update(user_data=<unavailable>) at midi.c:516:46 [opt]
   513    {
   514      midi_device *midi = devices->data;
   515
-> 516      for(int i = 0; i < midi->num_knobs; i++) update_with_move(midi, 0, i + midi->first_knob, NAN);
   517
   518      for(int i = 0; i < midi->num_keys; i++)
   519        midi_write(midi, midi->is_x_touch_mini ? 0 : midi->channel, 0x9,
(lldb)
frame #6: 0x00000001005112f0 libglib-2.0.0.dylib`g_timeout_dispatch + 20
libglib-2.0.0.dylib`g_timeout_dispatch:
->  0x1005112f0 <+20>: testl  %eax, %eax
    0x1005112f2 <+22>: je     0x100511329               ; <+77>
    0x1005112f4 <+24>: movl   %eax, %ebx
    0x1005112f6 <+26>: movq   %r14, %rdi
(lldb)

To Reproduce

unfortunately no scenario yet that reproduces the issue each time will do my best to find one ...

Platform

• darktable version :darktable-3.7.0+1488~g2bc5b6a66
• OS : OSx

[dterrahe]

What I think could be happening here is that you have a midi knob mapped to a slider in a module with multiple instances in one image and only one instance in the next. When the image is switched (by pressing any of the shortcuts) the extra module instance is deleted (including its widgets) and the target pointer of the slider's action temporarily set to NULL. In a next step, the target will be updated to the currently preferred shortcut instance (which might be first or last) but in the mean time, the midi device polls for an update and tries to access the null pointer.

Probably the below check is the culprit (once again; there've been problems with it before): https://github.com/darktable-org/darktable/blob/4a7774f95d54bebaa05a33b1d7d48b03881031ed/src/gui/accelerators.c#L2910-L2911

Would you please try if merging those two lines to || (!action_target && !_widget_invisible(action_target)))) solves the problem? If it does, hopefully it doesn't introduce other bugs...

[MStraeten]

it solves the problem, but it also doesn't allow to change settings further ;) --> dt shows "not active" So even worse.

[MStraeten]

both images contains several instances of colorbalancergb which sliders are mapped to the midi device. fprintf(stderr, "\n%p\n",action_target); indicates 0x0 --> Nullpointer for the first knob mapped to a colorbalancergb slider.

[MStraeten]

first occurrence of this issue: https://github.com/darktable-org/darktable/commit/9573128990e429c2890f2f075295a31c77309885 reverting that change prevents the crash

[dterrahe]

So even worse.

Yeah, sorry, my "fix" was just dumb. Had I been able to test it myself, I would not have wasted your time.

Basically it looks like the fix for #10181, enabling a shortcut for a (ratings) widget that has been deleted, was a bad idea, because now all other widgets can receive shortcuts after being deleted as well.

Can I ask you to test another possible solution for the ratings shortcut malfunction please? Change https://github.com/darktable-org/darktable/blob/ec2c49648c569f39775d972a98c8b9df2da1fac0/src/gui/accelerators.c#L3760 to if(label && action_def && !ac->target) ac->target = widget; and also revert the "fix" from #10181, i.e. remove https://github.com/darktable-org/darktable/blob/ec2c49648c569f39775d972a98c8b9df2da1fac0/src/gui/accelerators.c#L2910

My intention with this is to solve the problem with the ratings shortcut that the action is initially linked to the stars in the bottom row of lighttable, but then that link is overwritten when thumbnail overlays with stars are also linked to the action. When the thumbnail and its stars get deleted, the action is now linked to a NULL widget. So I'm now hoping that if an action link is not overwritten with a new widget (i.e. it keeps pointing to the original stars/rating widget, which never gets destroyed) it will not get set to NULL.

The risk is that this might also affect the initialisation of multi-instance module widgets, since those used to overwrite the actions of the 1st instance. But I believe that those action->widget pointers get updated each time focus changes, as well as after an image update, so hopefully that will have no impact.

[MStraeten]

that fixes the crash, enables modification for single instance modules (active and inactive) but not for multi instance modules even these are active after switching to the next image

[dterrahe]

Are you saying multi-instance only doesn't work "after switching to the next image" or do they never work, even after changing focus between the multi-instances or creating a new instance?

[MStraeten]

it works fine with multinstance moduels when opening the image coming from lighttable. the switching to the next image result in "not active" for multiinstance modules. Even if i activate one of the instances

[dterrahe]

And then when you switch focus to a multi-instance module, does it start working again? This probably needs some additional calls to dt_iop_connect_accels_multi to fix, but it would be helpful to know if manually triggering those (by changing the focused module) actually helps, or if this is a completely different problem.

Did the same problem with multi-instance not being enabled after switching images exist before 9573128 ?

[MStraeten]

my workflow:

• open first image, rotate knob --> colorbalancerg slider moves
• space to switsch to next image
• rotate knob for single instance module (exposure): exposure changes
• rotate knob for multiinstance module (colorbalancergb): "not active"
• click on module header of one colorbalancergb instance - module is expanded
• rotate knob for hue shift - "not active"
• move one slider per mouse
• rotate knob for hue shift - "not active"

if i just revert 9573128 it's fine

addition: if i activate the lowest instance in the stack - rotating the knob is fine. (preference setting is using the last instance) once the lowest instance was activated after switching to a further image there also the lowest instance is changed. After activating a further instance the lowest instance continues to be changed on rotating knobs.