Deleting a StaffTypeChange causes a crash in macOS

• Oct 20, 2019 - 17:29
Reported version
3.2
Type
Functional
Frequency
Once
Severity
S2 - Critical
Reproducibility
Always
Status
closed
Regression
No
Workaround
No
Project

This happens any time the user attempts to delete a StaffTypeChange, no matter where it occurs.

The bug is not specific to macOS, but other compilers are more forgiving if you try to advance an iterator when it already points to end().

The problem is with this bit of code in Staff::localSpatiumChanged(), from commit dee8662:

      auto i = _staffTypeList.find(tick.ticks());
      ++i;

The problem is that by the time this code is executed, the StaffType will already have been removed from the StaffTypeList, so the find() call will return end(). Fortunately, the upper_bound() function does what we want. Replacing those two lines with

      auto i = _staffTypeList.upper_bound(tick.ticks());

makes the code work the way it should.

A special case is if the user tries to delete a StaffTypeChange that has been placed on the first measure. A StaffTypeChange on the first measure is really a reference to the Staff's initial StaffType, so removing it should not cause the initial StaffType to be removed from the StaffTypeList. Currently, attempting this will result in a crash on macOS and a hang on Linux. When I try this in Windows, the clefs become misplaced, and the program crashes when I tried to add it back or open the "Staff/Part Properties..." dialog. The fix is to make sure not to call Staff::removeStaffType(tick()) if tick().isZero(). If we do this, then we can safely remove StaffTypeChange elements that have been placed on the first measure.

If a StaffTypeChange on the first measure is understood to be a reference to the Staff's initial StaffType, then it should come as no surprise that if you were to add a StaffTypeChange to the first measure, and then modify some of its properties, a subsequent deletion of the StaffTypeChange will not cause those properties to revert to what they were before the StaffTypeChange was added.


Comments

Thanks for this! FWIW I see sporadic unreproducible crashes on Windows too deleting staff type changes, maybe this is why.

Iterators are still new to me, so I appreciate the info on best practices, too!

Fix version
3.3.1