Fix issue #1084 crash when trying delete complicated track

[donarturo11]

Fixes issue #1084 .

[donarturo11]

@terminator356 , could you review this commit, please? Thanks to this one, this program is more stable. It prevents of crashes, when object won't found in list.

[terminator356]

Hi, thanks for the patch.
Unfortunately this is not the ideal solution, and is only a 'band-aid' solution.

We really need to know who is attempting to execute the operation with an invalid iterator,
and correct the problem there.
To do this, I must ask, can you post (a link to) the actual song involved, or say a 'cut-down'
version of it that still causes the problem, or else a 'mock-up' that will cause the problem?
That way I can examine every detail such as song storage and so on.

Question: The song causing the problem: Is it an old song made before 2020?
Or is it a new song made recently?
If it is an old song, you may indeed be seeing bug 887 as you originally posted... Maybe...

Thanks.
Tim.

[donarturo11]

Question: The song causing the problem: Is it an old song made before 2020?

This song was created in summer 2022.

[donarturo11]

To do this, I must ask, can you post (a link to) the actual song involved,

muzyka_na_smyczki_noSynth.med

[donarturo11]

Unfortunately this is not the ideal solution, and is only a 'band-aid' solution.

I tested another solution:

if(_imcv == _mcvl->begin())
          _mcvl->erase(_imcv);

Too runs. I thought about submit but I don't know if _imcv can be on another place of _mcvl.
I wrote a simple text function to check, when _imcv is present in _mcvl and I noticed that when _mcvl tries to erase not existing item, program crashes. So I think that good way is to check if item is in list, then if condition is true, erase.

[donarturo11]

Which better way to check if item is valid do you propose. In my humble opinion, direct erasing shouldn't be leave, because original solution is not as safe.

[donarturo11]

I found a problematic block. Inside original song, trying to remove crashes program, but this block imported to new files, crashes after moving.
test1.mpt

[donarturo11]

@terminator356 I removed loop and used method void delMCtlVal(unsigned int tick, Part* part, int val/* = -1*/); .
Now this line looks: _mcvl->delMCtlVal(_imcv->first, _imcv->second.part, _imcv->second.val); .
Runs well. In my opinion it should be a a better solution.

[donarturo11]

I think yet about reimplement the method erase inside MidiCtrlValue. It would be as override.

[donarturo11]

@terminator356 I found yet another solution. I reverted _mcvl->erase(_imcl) in line 1282 of operation.cpp and uncommented line 35 of midictrl.h #define _MIDI_CTRL_METHODS_DEBUG_ and the program didn't crash while delete track.
I would like to ask if is any reason of optionally turn on this definition, and it won't better to remove #ifdef _MIDI_CTRL_METHODS_DEBUG_ conditional?

[donarturo11]

@terminator356 So I removed the commit with searching loop and replaced erase. Could you review the code after changes, please? If do you want to leave erase method in operations.cpp, maybe better enable debug methods as default in midictrl?
I shared entire directory on Dropbox

[donarturo11]

@terminator356 Well, what you think about latest commit: Use another method to erase item? Now it is good to merge?

[terminator356]

Hi, sorry for the delay, I am studying the matter and should have an answer this weekend.

[donarturo11]

@terminator356 Sorry if I'm asking here. Do contributors use any platform like Discord or IRC? I know that comments below issues or pull requests are not good place for such discussions. A very good example is Developer group chat in MuseScore project.

[terminator356]

Hi @donarturo11
I have found several problems in our code. I am working to fix them.
I also found some bugs that, when fixed, should fix some features that were broken for a long time.
Please wait... Thanks.

[luzpaz]

Any progress here ?

[devloop0123]

Excuse me for intruding, but the problem with donarturo11's test1.mpt is that it contains (multiple) identical simultaneous events. Removing the duplicates with
awk '!seen[$0]++' test1.mpt > test2.mpt
fixes the part.

If I create a part with a single cc event, and duplicate that event in the .mpt file, like
event tick="1536" type="1" a="1" b="102"
event tick="1536" type="1" a="1" b="102"
then attempting to do anything (moving, deleting, cutting) with this imported part crashes MusE.
If any one of those 4 event parameters is different, then there's no crash anymore.

So the problem is not the complicated track, but duplicate events.