View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0006602 | ardour | bugs | public | 2015-09-21 09:27 | 2016-09-17 04:41 |
| Reporter | x42 | Assigned To | |||
| Priority | normal | Severity | major | Reproducibility | always |
| Status | new | Resolution | open | ||
| Product Version | 4.X git (version in description) | ||||
| Summary | 0006602: crash in undo if two operations are performed at the same time | ||||
| Description | Start 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 Information | current: Ardour 4.2-398-g77ee3d1 | ||||
| Tags | undo | ||||
|
|
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.. |
|
|
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. |
|
|
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. |
|
|
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. |
|
|
I agree. Maybe keyboard shortcuts should be inactive when other operations are in progress. |
|
|
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) |
|
|
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
|
|
|
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... |
|
|
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. |
|
|
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? |
|
|
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. |
| 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 |
