Crash in debug build when dragging selected notes

• May 8, 2020 - 12:05
Reported version
3.x-dev
Type
Functional
Frequency
Once
Severity
S2 - Critical
Reproducibility
Always
Status
closed
Regression
Yes
Workaround
No
Project
MuseScore

Reproduce:

  1. Add some notes to a new score.

  2. Make a range selection of the notes

  3. Attempt to click and drag one of them

  4. Crash (from assert, so probably debug builds only)

OS: Ubuntu 20.04 LTS, Arch.: x86_64, MuseScore version (64-bit): 4.0.0, revision: 3543170

Comments

• May 8, 2020 - 12:42

libmscore/select.cpp:Ms::Selection::updateSelectedElements: "ASSERT FAILED!": !isLocked() libmscore/select.cpp 514
libmscore/select.cpp:unknown: ASSERT: "!isLocked()" in file libmscore/select.cpp, line 514

and lockReason() is "drag".

Reply

• May 9, 2020 - 09:42

I can't find any information regarding IF_ASSERT_FAILED but looking at the context I expect it shouldn't cause a crash (which it does now) but return a true and uses LOGE to print a message. So, I'm wandering the IF_ASSERT_FAILED is correct.

Reply

• May 9, 2020 - 13:59

IF_ASSERT_FAILED - It's just an assert and if in one macro. It is designed to identify situations that should not occur. Assert trigger in debug build, if for handles situation for release build.
For example:

We have such a code

void method (Type* ptr)
{
    ptr->doSomeThing();
}

We expect that ptr should not be null, therefore, we can add an assert to detect an error.

void method (Type* ptr)
{
   Q_ASSERT(ptr);
   ptr->doSomeThing();
}

But!! If an error still occurs, then the release build will crash the program, we will turn to the null pointer.
To prevent this, we must add a check

void method (Type* ptr)
{
   Q_ASSERT(ptr);
   if (!ptr) {
       qCritical() << "ptr is null";
       return;
   }
   ptr->doSomeThing();
}

IF_ASSERT_FAILED does exactly that

void method (Type* ptr)
{
   IF_ASSERT_FAILED(ptr) {
       return;
   }
   ptr->doSomeThing();
}

Reply

• May 9, 2020 - 14:05

There is a problem with the implementation of the drag.

Drag works like this:
1.start drag

for (Element* e : _score->selection().elements())
            e->startDrag(editData);
  1. draging
  2. end drag
for (Element* e : _score->selection().elements()) {
            e->endDrag(editData);

i.e. the current implementation of the drag assumes that during the drag, the selection should not change!

To control the selection change, I added a lock function
And now like this:
1.start drag

for (Element* e : _score->selection().elements())
            e->startDrag(editData);

_score->selection().lock("drag");
  1. draging
  2. end drag
for (Element* e : _score->selection().elements()) {
            e->endDrag(editData);

_score->selection().unlock("drag");

Users had crashed due to the fact that during the drag, the user changed the selection using the keyboard.
Therefore I added

            {{"next-chord",
              "prev-chord",
              "next-track",
              "prev-track",
              "next-measure",
              "prev-measure"}, [](ScoreView* cv, const QByteArray& cmd) {

                  if (cv->score()->selection().isLocked()) {
                        LOGW() << "unable exec cmd: " << cmd << ", selection locked, reason: " << cv->score()->selection().lockReason();
                        return;
                        }

Probably something similar happens in this case.

Reply

Fix version
3.5.0