Crash when a plugin uses a cursor without calling rewind

• May 6, 2020 - 19:12
Reported version
3.4
Type
Plugins
Frequency
Once
Severity
S2 - Critical
Reproducibility
Always
Status
closed
Regression
No
Workaround
Yes
Project
MuseScore

Found in 3.4.2 and also in 3.5 alpha.
OS: macOS 10.14, Arch.: x86_64, MuseScore version (64-bit): 3.4.2.25137, revision: 148e43f

  1. create a new empty score
  2. run the attached plugin, which contains the following code
        var cursor = curScore.newCursor();
        cursor.setDuration(1, 4);
        cursor.addNote(60, false);
        cursor.addNote(64, true);

Actual result: crash. See attached crash log.

Expected result: no crash, notes are added to score.

Workaround: do cursor.rewind(Cursor.SCORE_START) after creating the cursor.

cursorTest.qml

crash.txt

Comments

• May 8, 2020 - 00:46

While crash certainly shouldn't happen I would be more cautious about the "notes are added to score" part. Without rewinding a cursor it is not clear where should these notes be added (at score start, at range selection start, at current note input cursor position or somewhere else), and explicit rewind() call (or, as possible since 3.5 version, setting Cursor object to be in sync with note input state) allows to define this explicitly and avoid possible unintended side effects from these addNote() calls. Also for a long time rewind() call logic has been used to determine a presence and boundaries of range selection (see, for example, the standard Notenames plugin), and initializing Cursor to point at some score position before rewinding it may break this logic and thus cause plugins compatibility issues.

Reply

• May 8, 2020 - 02:59

Thanks for the explanation, it makes sense. When a cursor is first created, I think you are saying the position would be "undefined" and any attempt to use the cursor would be a no-op, ideally with a message to the console so that the poor plugin developer knows what he did wrong. I didn't quite understand your comment about the Notenames plugin, since that plugin does cursor.rewind(1) at the very beginning.

Reply

• May 8, 2020 - 04:39
Status active ⇒ PR created

See https://github.com/musescore/MuseScore/pull/6059 for a fix preventing the crash.

Regarding Notenames, I meant that Cursor is used there specifically to determine presence and range of selection in a score, and for this it relies on the current rewind() behavior which leaves cursor location uninitialized if rewinding to selection start and that selection does not exist.

Reply

Fix version
3.5.0