Menu

#1260 Crash on Undo Add Layer

None
closed
notation (302)
1
2022-06-14
2010-02-27
No

Draw a segment. Open in notation view. Click Add Layer button. Undo. BOOM!

Must fix before releasing again!

Discussion

  • Thorsten Alteholz

    In HeadersGroup::slotSetCurrentSegment() the m_scene->getCurrentStaff() delivers a null pointer and BOOM.

    getCurrentStaff() becomes invalid in NotationScene::segmentRemoved() after m_staffs.erase(i) (thats what it should do)

    I tried to use setCurrentStaff() afterwards and could avoid the initial BOOM. Unfortunately now any other action in the notation editor gave a crash.

    I am lost now and hope anybody with more insight can go on ...

    Thorsten

     
  • Yves Guillemot

    Yves Guillemot - 2010-03-29

    The real problem seems related to the segment creation rather than to the undo action:

    Test 1:
    - Draw a segment on track 1
    - Draw a second segment at the same place on the same track (I draw it on the track 2 then I move it).
    - Open the two segments in notation
    - Delete the second one from the main window : no problem.

    Test 2:
    - Draw a segment on track 1
    - Open it in notation
    - Add a layer with the button
    - Delete the new layer segment from the main window : crash.

    So, from the notation editor point of view, there is a difference between the segment created with the add layer button and a "normal" segment.

    I had a first quick look to the add layer code, but didn't see anything obviously wrong.

    I will try to have a more extensive look in the next days.

    Yves

     
  • Yves Guillemot

    Yves Guillemot - 2010-04-04

    I was wrong : the crashes are related to the undo (or trigged by the undo).

    Two crash types should be fixed now :

    1 - Crash when deleting the last segment of the notation view and when this last segment is the current one. That was always the case when undoing a add layer done from an editor opened on an unique segment.
    To fix it, when the current segment is going to be deleted, the first segment of the view is now set as the current one.

    2 - When the preceding crash is fixed, a new crash occurs when entering a note on the remaining segment after an undo.
    The problem is related to the chord name ruler which maintains a list of the segments.
    A deleted segment is not removed from this list, and the chord ruler is sometimes referencing it. This is true whatever way is used to delete the segment, but, for some reason, the undo activates the crash while the delete from the main window does not.
    Probably some other tools was also keeping a pointer to deleted segments, but I saw only crashes coming from the chord name ruler.
    The fix is to always regenerate the whole notation widget when a segment is deleted.

    Now at least a third crash may occur :

    1 - open an empty segment in notation
    2 - add a layer
    3 - select the initial segment with the changer tool
    4 - select the selection tool
    5 - select some rests
    6 - undo the add layer
    7 - select the pencil tool : boom !

    I have no fix for this one still.

    Yves

     
  • Yves Guillemot

    Yves Guillemot - 2010-04-05

    That last crash should now be fixed in rev. 11862.

    Yves

     
  • D. Michael McIntyre

    I ran through all scenarios discussed here, and all crashes appear to be fixed. Nice work, Yves!

     
  • Mark R. Rubin

    Mark R. Rubin - 2022-04-23

    12 years later and this bug has returned from the dead:

    1. Create segment.
    2. Notation editor.
    3. Add note (required, no crash if not).
    4. Add Layer.
    5. Undo.
    (gdb) where 4
    #0  0x000000000086eeae in Rosegarden::Composition::getBarNumber(long) const ()
    #1  0x000000000086f8c9 in Rosegarden::Composition::getBarRangeForTime(long) const ()
    #2  0x000000000060c5a7 in Rosegarden::NotationStaff::getEndTime() const ()
    #3  0x000000000064f5b8 in Rosegarden::NotationScene::getCursorCoordinates(long) const ()
    (More stack frames follow...)
    

    composition is null here:

    timeT NotationStaff::getEndTime() const
    {
        Composition *composition = getSegment().getComposition();
        return composition->getBarEndForTime
            (getSegment().getEndMarkerTime() - 1);
    }
    

    Tested in 21.12 and 22.06 [abdf22] and [8c1c6b].

     

    Related

    Commit: [8c1c6b]
    Commit: [abdf22]

  • Philip Leishman

    Philip Leishman - 2022-04-23

    This came back with the pointer update after undo.
    Unfortunately the connection of the sceneNeedsRebuilding signal is a queued connection (never liked using those). This means the pointer update comes after the segment (layer in this case) is deleted but before the notation scene has been rebuilt. The result is an invalid staff in the scene.
    I think I have fixed this by deleting the invalid staff in NotationScene::segmentRemoved
    See merge request.

     
    • Mark R. Rubin

      Mark R. Rubin - 2022-04-23

      Thanks, Philip. Tested and didn't get any crashes.

       
  • Ted Felix

    Ted Felix - 2022-04-26
    • status: closed --> feedback
    • Group: --> None
    • Priority: 9 --> 1
     
  • Ted Felix

    Ted Felix - 2022-04-26

    Found a related crash. Present in 21.12...

    • Create a Segment
    • Press "N" to open in notation
    • Segment > Add Layer
    • Switch back to the main window.
    • Select the top (original, gray) segment only.
    • Press delete.
    • Crash
     
  • Ted Felix

    Ted Felix - 2022-04-26

    Definitely better. Pushed fix as [cf0a28]. Please test latest git.

    Still crashing with new "delete original segment" procedure above. Let me know if you want me to move this to a new bug report.

     

    Related

    Commit: [cf0a28]

  • Philip Leishman

    Philip Leishman - 2022-04-26

    We can stay with this bug report. I will look into the "delete original segment" crash

     
  • Philip Leishman

    Philip Leishman - 2022-04-28

    I was having problems with this one - getting strange results from the debugger !!!
    Then I saw that m_segments was declared again in NotationView.h - it is declared protected in the base class EditViewBase. Very strange.
    Anyway I just removed the NotationView declaration and the crash went away.
    Can this have any disadvantages ? It is certainly bad practice to redeclare a protected item.
    Please merge

     
  • Ted Felix

    Ted Felix - 2022-04-29

    Looks good. Merged as [53f4f7]. Please test latest git.

     

    Related

    Commit: [53f4f7]

  • Ted Felix

    Ted Felix - 2022-06-14
    • status: feedback --> closed
     

Log in to post a comment.