Replace every use of the foreach macro with an iterator loop

[Fastigium]

This prevents a race condition with Qt5. A foreach loop makes a copy of its
Qt container, increasing the reference count to the container's internal
data. Qt5 often asserts isDetached(), which requires the reference count to
be <= 1. This assertion fails when the foreach loop increases the reference
count at exactly the wrong moment. Using an iterator loop prevents an
unnecessary copy from being made and ensures this race condition isn't
triggered.

[tresf]

-   // foreach new subband
+   // for each new subband

bahahaha

[Fastigium]

I wondered if anyone would spot that one 😁. I really wanted grep -r foreach to be clean 😋

[Fastigium]

Added the in progress label as I discovered this PR introduces a new crash, namely #2659. Not making a copy of the Qt container you're working on can cause crashes, too, apparently 😁. I could fix it by reverting one foreach removal, but that'd allow the Qt5 assert crash to happen again. I'd much prefer to fix this differently.

Testing still welcome, though. This crash only happens under pretty specific circumstances, so the PR should be crash-free otherwise.

[michaelgregorius]

Is it possible to iterate Qt containers using the "new" C+11 for loops? Example:

for( NotePlayHandle const * cnph : nphv )
{
  // Do stuff with cnph
}

This would be more compact and also make the resulting code more understandable. Personally I don't really like tons of references to *it in the body of for loops. In the example above you'd immediately know what's the type of cnph (which should be called notePlayHandle anyway).

[midi-pascal]

@michaelgregorius Does this means a compiler that supports C++ 11? Not sure everyone has one.
I have gcc 4.8.4 and I don't know if it supports it.

[grejppi]

@midi-pascal C++11 for loops are used in numerous places in LMMS already, so if your compiler can still build LMMS they should be fine. 👍

[midi-pascal]

@grejppi Ok then. I was just not sure. I just want to be sure I will be able to continue to contribute to my beloved lmms 👍

[Fastigium]

@michaelgregorius A most sensible suggestion! After reading up on them, I think they should work fine. They will call detach() where this PR's iterators will not, but thankfully C++11 for does not call the copy constructor. So using C++11 for instead of const_iterator is in the same category as using container[i] instead of container.at(i): harmless as long as you never use the copy constructor.

Before I do all the grunt work again, though, I would like to confirm that this foreach removal fixes the assert crashes it's supposed to fix. @Umcaruje do you have time for another stress test? I'll see if I can compile LMMS using Qt5 and reproduce the crashes, too.

Oh and @midi-pascal: it appears that C++11 for support was added in GCC 4.6 😊

[Umcaruje]

@Umcaruje do you have time for another stress test?

Yeah, I'll do one when I get back from my german test, that's in about a couple of hours.

[Fastigium]

@Umcaruje Awesome, best of luck with the test!

[Umcaruje]

Ok, so I stress tested it by moving the channels rapidly, it passed. I left it in the background playing, but so far so good, no crashes.

[Fastigium]

@Umcaruje Thanks, and good to hear! Can confirm that it passes with the PR, but crashes frequently on master. I'll make another PR with C++11 for when time permits. Crossing fingers that that one will be crash-free, too (apart from #2659, of course 😁)

[tresf]

👏

[Umcaruje]

Ok, I left LMMS idle for a couple of hours and it didn't crash. Amazing job @Fastigium !

[Umcaruje]

Ok, I removed a channel that was sending to another channel while playing and got a segfault:
https://gist.github.com/Umcaruje/3d52ca07aed1646905b2

But this is a completely different crash, I'll see if I can reliably reproduce it on both master and your branch, and will post another issue.

[Fastigium]

Thanks for the encouragement 😊!

And @Umcaruje: channel removal crashes are on my list via #1679, that's a wholly different beast.

[Umcaruje]

Well, I found a Qt5 specific bug, that can always be reproduced: #2667

That might help in unraveling it

[Fastigium]

I finished the C++11 for PR: #2669. Feedback welcome!

[Fastigium]

Closing because #2669 fixes things even better 😊