View Issue Details

IDProjectCategoryView StatusLast Update
0006602ardourbugspublic2016-09-17 04:41
Reporterx42 Assigned To 
PrioritynormalSeveritymajorReproducibilityalways
Status newResolutionopen 
Product Version4.X git (version in description) 
Summary0006602: crash in undo if two operations are performed at the same time
DescriptionStart a drag, then perform a 2nd operation via keyboard shortcut.

"S" (split) is currently special cased, but others. e.g Delete (region, marker, midi PC), gain increase/decrease or any other operation that can be triggered by a keyboard shortcut and save an undo will crash Ardour.
Additional Informationcurrent: Ardour 4.2-398-g77ee3d1
Tagsundo

Relationships

parent of 0006338 closedtnaugle while dragging / cutting region 
related to 0006803 closedx42 Assertion failed/core dump in Strip Silence dialog 
related to 0006836 closed Ardour crashes when pressing and releasing the delete key while dragging a note. 

Activities

timbyr

2015-09-27 05:12

developer   ~0017369

I can confirm that a crash occurs while dragging a midi event and pressing the delete key that Ardour will crash. I can't get a crash when dragging/moving a region or range marker and pressing the delete key though. It seems to delete the region/marker and undo works. Copy dragging a region seems insensitive to delete during drag. This is with a nightly version 4.2.420 on linux.

I'm not sure what you mean at the end of your description "and save an undo will crash Ardour" if that is relevant could you explain a bit more..

x42

2015-09-27 11:57

administrator   ~0017370

To be specific, any keyboard-triggered action that calls Session::commit_reversible_command() and thereby saves an undo command is potentially dangerous.

There are various reports, not all of which I was able to reproduce,

09:11 < joules> oh damn I pressed delete on the location bar and poof
09:18 < joules> yes! I was draggin, like "Can I select all these if I drag..oops nope =)"

as a side-note: Selecting 2 markers, start drag, press delete, no crash here. but on end-drag. the 2nd marker reappears. Might be a separate bug, but it's really the same cause: two operations going on at the same time.

tartina

2015-09-27 17:11

reporter   ~0017380

Last edited: 2015-09-27 19:30

I can confirm that dragging a midi event and pressing delete crashes ardour, but I think this is not caused by commit_reversible_command(), but because NoteDrag::total_dx calls Evoral::Note<Evoral::Beats>::time() with invalid MIDI note (deleted).
I made a patch that seems to work:
https://github.com/tartina/ardour/commit/a66b6f5cc5bea3e56e2705a9ece2a2355b771dc2
It's similar to tymbyr's patch for bug 0006338
I also think that this workaround
https://github.com/tartina/ardour/commit/7df3c1e38403377395b26ef3b0379d450f30ca3d
would make Editor::commit_reversible_command () more robust.

x42

2015-09-27 17:16

administrator   ~0017381

I think that special casing all actions with if ( XXX is active) { don't to YYY } is the wrong approach.

It's fine for a quick short-term hotifx (e.g 0006338) , but not a solution. There are too many cases.

tartina

2015-09-27 18:01

reporter   ~0017383

I agree. Maybe keyboard shortcuts should be inactive when other operations are in progress.

x42

2015-09-28 08:41

administrator   ~0017386

yep, good call. "If there is already a pending undo command, ignore keyboard triggered actions."

The check to allow triggering actions by keyboard is Session::current_operations().empty() and maybe a new method to also check if Session::_current_trans is NULL.

The harder part is overriding actions. The best option is probably: ARDOUR_UI_UTILS::key_press_focus_accelerator_handler() but this is work in progress (will change radically with the "tabbed" branch in Ardour 4.4)

tartina

2015-09-28 12:49

reporter   ~0017390

I think this time Session::commit_reversible_command() is not responsible for this crash.
The problem is in NoteDrag::total_dx (gtk2_ardour/editor_drag.cc:5313) at line 5316.
frameoffset_t const n = _region->source_beats_to_absolute_frames (_primary->note()->time ());
calls note->time() with an invalid note (it has already been deleted by keyboard delete key).
I'll attach the backtrace.

2015-09-28 12:49

 

midi_note_delete_while_dragging_bt.txt (3,840 bytes)   
#0  0x00000000006311ab in boost::detail::atomic_increment(int*) (pw=0x74672e786f425673)
    at /usr/include/boost/smart_ptr/detail/sp_counted_base_gcc_x86.hpp:66
#1  0x00000000006311f8 in boost::detail::sp_counted_base::add_ref_copy() (this=0x74672e786f42566b)
    at /usr/include/boost/smart_ptr/detail/sp_counted_base_gcc_x86.hpp:134
#2  0x0000000000631331 in boost::detail::shared_count::shared_count(boost::detail::shared_count const&) (this=0x7fffffffc278, r=...)
    at /usr/include/boost/smart_ptr/detail/shared_count.hpp:454
#3  0x000000000077c32b in boost::shared_ptr<Evoral::Note<Evoral::Beats> >::shared_ptr(boost::shared_ptr<Evoral::Note<Evoral::Beats> > const&) (this=0x7fffffffc270) at /usr/include/boost/smart_ptr/shared_ptr.hpp:323
#4  0x000000000077c3cb in NoteBase::note() const (this=0x462cea0) at ../gtk2_ardour/note_base.h:102
#5  0x000000000085d6e3 in NoteDrag::total_dx(unsigned int) const (this=0x514b1d0, state=256) at ../gtk2_ardour/editor_drag.cc:5316
#6  0x000000000085df55 in NoteDrag::finished(_GdkEvent*, bool) (this=0x514b1d0, ev=0x52174e0, moved=true) at ../gtk2_ardour/editor_drag.cc:5441
#7  0x000000000084289b in Drag::end_grab(_GdkEvent*) (this=0x514b1d0, event=0x52174e0) at ../gtk2_ardour/editor_drag.cc:307
#8  0x0000000000842134 in DragManager::end_grab(_GdkEvent*) (this=0x34595d0, e=0x52174e0) at ../gtk2_ardour/editor_drag.cc:154
#9  0x000000000083b90a in Editor::track_canvas_button_release_event(_GdkEventButton*) (this=0x30ef460, event=0x52174e0)
    at ../gtk2_ardour/editor_canvas_events.cc:197
#10 0x00000000007e3844 in sigc::bound_mem_functor1<bool, Editor, _GdkEventButton*>::operator()(_GdkEventButton* const&) const (this=0x34fc6c8, _A_a1=@0x7fffffffc418: 0x52174e0) at /usr/include/sigc++-2.0/sigc++/functors/mem_fun.h:1856
#11 0x00000000007dca79 in sigc::adaptor_functor<sigc::bound_mem_functor1<bool, Editor, _GdkEventButton*> >::operator()<_GdkEventButton* const&>(_GdkEventButton* const&) const (this=0x34fc6c0, _A_arg1=@0x7fffffffc418: 0x52174e0) at /usr/include/sigc++-2.0/sigc++/adaptors/adaptor_trait.h:89
#12 0x00000000007d5580 in sigc::internal::slot_call1<sigc::bound_mem_functor1<bool, Editor, _GdkEventButton*>, bool, _GdkEventButton*>::call_it(sigc::internal::slot_rep*, _GdkEventButton* const&) (rep=0x34fc690, a_1=@0x7fffffffc418: 0x52174e0) at /usr/include/sigc++-2.0/sigc++/functors/slot.h:137
#13 0x00007ffff19c53ab in (anonymous namespace)::Widget_signal_button_release_event_callback(_GtkWidget*, _GdkEventButton*, void*) ()
    at /lib64/libgtkmm-2.4.so.1
#14 0x00007ffff38f983d in _gtk_marshal_BOOLEAN__BOXED () at /lib64/libgtk-x11-2.0.so.0
#15 0x00007ffff458fcd5 in g_closure_invoke () at /lib64/libgobject-2.0.so.0
#16 0x00007ffff45a18d4 in signal_emit_unlocked_R () at /lib64/libgobject-2.0.so.0
#17 0x00007ffff45a9ae2 in g_signal_emit_valist () at /lib64/libgobject-2.0.so.0
#18 0x00007ffff45aa29f in g_signal_emit () at /lib64/libgobject-2.0.so.0
#19 0x00007ffff3a2983c in gtk_widget_event_internal () at /lib64/libgtk-x11-2.0.so.0
#20 0x00007ffff38f7b24 in gtk_propagate_event () at /lib64/libgtk-x11-2.0.so.0
#21 0x00007ffff38f7eeb in gtk_main_do_event () at /lib64/libgtk-x11-2.0.so.0
#22 0x00007ffff35507ac in gdk_event_dispatch () at /lib64/libgdk-x11-2.0.so.0
#23 0x00007ffff428fa8a in g_main_context_dispatch () at /lib64/libglib-2.0.so.0
#24 0x00007ffff428fe20 in g_main_context_iterate.isra () at /lib64/libglib-2.0.so.0
---Type <return> to continue, or q <return> to quit---
#25 0x00007ffff4290142 in g_main_loop_run () at /lib64/libglib-2.0.so.0
#26 0x00007ffff38f6f37 in gtk_main () at /lib64/libgtk-x11-2.0.so.0
#27 0x00007ffff7588fee in Gtkmm2ext::UI::run(Receiver&) (this=0x183fb30, old_receiver=...) at ../libs/gtkmm2ext/gtk_ui.cc:280
#28 0x0000000000a7bbe0 in main(int, char**) (argc=1, argv=0x7fffffffcdc8) at ../gtk2_ardour/main.cc:389

x42

2015-09-28 13:10

administrator   ~0017392

Ah, so this calls for a drags()->active() check, rather than (or in addition to) reversible_command.
The former usually implies the latter (drag start prepares Undo) but not for midi-note level editing.

What a mess :) There goes multi-touch, multi-operation support...

timbyr

2016-09-15 13:33

developer   ~0018648

I have committed tartina's fix for this issue to master as e2b72419 which should be included in a nightly build >= 5.3.113

Please confirm that this issue is now fixed.

A change to enable both operations occurring at the same time(delete while dragging), can wait for another day.

tartina

2016-09-16 07:17

reporter   ~0018661

Last edited: 2016-09-16 07:19

I can confirm that now you can't delete a midi note while dragging it, so ardour doesn't crash anymore, but there may be other double operations which could still crash it.

I still have a doubt: in Editor::commit_reversible_command is it safe to call _session->commit_reversible_command() if before vector is empty?

timbyr

2016-09-17 04:41

developer   ~0018672

Sorry, my last comment(c18648) was meant to be for issue 0006836.

The underlying issue relating to undo/redo and multiple operations is not fixed.

Issue History

Date Modified Username Field Change
2015-09-21 09:27 x42 New Issue
2015-09-21 09:27 x42 Relationship added parent of 0006338
2015-09-27 05:12 timbyr Note Added: 0017369
2015-09-27 11:57 x42 Note Added: 0017370
2015-09-27 17:11 tartina Note Added: 0017380
2015-09-27 17:16 x42 Note Added: 0017381
2015-09-27 18:01 tartina Note Added: 0017383
2015-09-27 19:30 tartina Note Edited: 0017380
2015-09-28 08:41 x42 Note Added: 0017386
2015-09-28 12:49 tartina Note Added: 0017390
2015-09-28 12:49 tartina File Added: midi_note_delete_while_dragging_bt.txt
2015-09-28 13:10 x42 Note Added: 0017392
2015-09-29 01:21 timbyr Tag Attached: undo
2016-02-28 17:58 x42 Relationship added related to 0006803
2016-03-29 18:58 x42 Relationship added related to 0006836
2016-09-15 13:33 timbyr Note Added: 0018648
2016-09-16 07:17 tartina Note Added: 0018661
2016-09-16 07:19 tartina Note Edited: 0018661
2016-09-17 04:41 timbyr Note Added: 0018672