Skip to content

Commit

Permalink
Replace every use of the foreach macro with a C++11 range-based for loop
Browse files Browse the repository at this point in the history
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 a range-based for loop prevents an
unnecessary copy from being made and ensures this race condition isn't
triggered.
  • Loading branch information
Fastigium committed Mar 11, 2016
1 parent b8000df commit dc7798e
Show file tree
Hide file tree
Showing 16 changed files with 27 additions and 50 deletions.
8 changes: 4 additions & 4 deletions include/InstrumentPlayHandle.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ class EXPORT InstrumentPlayHandle : public PlayHandle
do
{
nphsLeft = false;
foreach( const NotePlayHandle * cnph, nphv )
for( const NotePlayHandle * constNotePlayHandle : nphv )
{
NotePlayHandle * nph = const_cast<NotePlayHandle *>( cnph );
if( nph->state() != ThreadableJob::Done && ! nph->isFinished() )
NotePlayHandle * notePlayHandle = const_cast<NotePlayHandle *>( constNotePlayHandle );
if( notePlayHandle->state() != ThreadableJob::Done && ! notePlayHandle->isFinished() )
{
nphsLeft = true;
nph->process();
notePlayHandle->process();
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions plugins/MidiExport/MidiExport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,15 @@ bool MidiExport::tryExport( const TrackContainer::TrackList &tracks, int tempo,
uint8_t buffer[BUFFER_SIZE];
uint32_t size;

foreach( Track* track, tracks ) if( track->type() == Track::InstrumentTrack ) nTracks++;
for( const Track* track : tracks ) if( track->type() == Track::InstrumentTrack ) nTracks++;

// midi header
MidiFile::MIDIHeader header(nTracks);
size = header.writeToBuffer(buffer);
midiout.writeRawData((char *)buffer, size);

// midi tracks
foreach( Track* track, tracks )
for( Track* track : tracks )
{
DataFile dataFile( DataFile::SongProject );
MidiFile::MIDITrack<BUFFER_SIZE> mtrack;
Expand Down
2 changes: 1 addition & 1 deletion plugins/zynaddsubfx/ZynAddSubFx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ void ZynAddSubFxInstrument::loadSettings( const QDomElement & _this )
m_pluginMutex.unlock();

m_modifiedControllers.clear();
foreach( const QString & c, _this.attribute( "modifiedcontrollers" ).split( ',' ) )
for( const QString & c : _this.attribute( "modifiedcontrollers" ).split( ',' ) )
{
if( !c.isEmpty() )
{
Expand Down
2 changes: 1 addition & 1 deletion src/core/AutomatableModel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ void AutomatableModel::unlinkModels( AutomatableModel* model1, AutomatableModel*

void AutomatableModel::unlinkAllModels()
{
foreach( AutomatableModel* model, m_linkedModels )
for( AutomatableModel* model : m_linkedModels )
{
unlinkModels( this, model );
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/ComboBoxModel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ void ComboBoxModel::addItem( const QString& item, PixmapLoader* loader )
void ComboBoxModel::clear()
{
setRange( 0, 0 );
foreach( const Item& i, m_items )
for( const Item& i : m_items )
{
delete i.second;
}
Expand Down
14 changes: 7 additions & 7 deletions src/core/FxMixer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ FxChannel::~FxChannel()

inline void FxChannel::processed()
{
foreach( FxRoute * receiverRoute, m_sends )
for( const FxRoute * receiverRoute : m_sends )
{
if( receiverRoute->receiver()->m_muted == false )
{
Expand Down Expand Up @@ -121,7 +121,7 @@ void FxChannel::doProcessing()

if( m_muted == false )
{
foreach( FxRoute * senderRoute, m_receives )
for( FxRoute * senderRoute : m_receives )
{
FxChannel * sender = senderRoute->sender();
FloatModel * sendModel = senderRoute->amount();
Expand Down Expand Up @@ -293,7 +293,7 @@ void FxMixer::deleteChannel( int index )
tracks += Engine::getSong()->tracks();
tracks += Engine::getBBTrackContainer()->tracks();

foreach( Track* t, tracks )
for( Track* t : tracks )
{
if( t->type() == Track::InstrumentTrack )
{
Expand Down Expand Up @@ -335,11 +335,11 @@ void FxMixer::deleteChannel( int index )
m_fxChannels[i]->m_channelIndex = i;

// now check all routes and update names of the send models
foreach( FxRoute * r, m_fxChannels[i]->m_sends )
for( FxRoute * r : m_fxChannels[i]->m_sends )
{
r->updateName();
}
foreach( FxRoute * r, m_fxChannels[i]->m_receives )
for( FxRoute * r : m_fxChannels[i]->m_receives )
{
r->updateName();
}
Expand Down Expand Up @@ -528,7 +528,7 @@ FloatModel * FxMixer::channelSendModel( fx_ch_t fromChannel, fx_ch_t toChannel )
const FxChannel * from = m_fxChannels[fromChannel];
const FxChannel * to = m_fxChannels[toChannel];

foreach( FxRoute * route, from->m_sends )
for( FxRoute * route : from->m_sends )
{
if( route->receiver() == to )
{
Expand Down Expand Up @@ -578,7 +578,7 @@ void FxMixer::masterMix( sampleFrame * _buf )
// also instantly add all muted channels as they don't need to care about their senders, and can just increment the deps of
// their recipients right away.
MixerWorkerThread::resetJobQueue( MixerWorkerThread::JobQueue::Dynamic );
foreach( FxChannel * ch, m_fxChannels )
for( FxChannel * ch : m_fxChannels )
{
ch->m_muted = ch->m_muteModel.value();
if( ch->m_muted ) // instantly "process" muted channels
Expand Down
13 changes: 1 addition & 12 deletions src/core/NotePlayHandle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -301,17 +301,6 @@ void NotePlayHandle::play( sampleFrame * _working_buffer )
}
}

// play sub-notes (e.g. chords)
// handled by mixer now
/* foreach( NotePlayHandle * n, m_subNotes )
{
n->play( _working_buffer );
if( n->isFinished() )
{
NotePlayHandleManager::release( n );
}
}*/

// update internal data
m_totalFramesPlayed += framesThisPeriod;
unlock();
Expand Down Expand Up @@ -369,7 +358,7 @@ void NotePlayHandle::noteOff( const f_cnt_t _s )
m_released = true;

// first note-off all sub-notes
foreach( NotePlayHandle * n, m_subNotes )
for( NotePlayHandle * n : m_subNotes )
{
n->lock();
n->noteOff( _s );
Expand Down
2 changes: 1 addition & 1 deletion src/core/audio/AudioPort.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ void AudioPort::doProcessing()
BufferManager::clear( m_portBuffer, fpp );

//qDebug( "Playhandles: %d", m_playHandles.size() );
foreach( PlayHandle * ph, m_playHandles ) // now we mix all playhandle buffers into the audioport buffer
for( PlayHandle * ph : m_playHandles ) // now we mix all playhandle buffers into the audioport buffer
{
if( ph->buffer() )
{
Expand Down
2 changes: 1 addition & 1 deletion src/core/fft_helpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ int compressbands(float *absspec_buffer, float *compressedband, int num_old, int

ratio=(float)usefromold/(float)num_new;

// foreach new subband
// for each new subband
for ( i=0; i<num_new; i++ )
{
compressedband[i]=0;
Expand Down
2 changes: 1 addition & 1 deletion src/gui/EffectSelectDialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ void EffectSelectDialog::rowChanged( const QModelIndex & _idx,
subLayout->setSpacing( 0 );
m_currentSelection.desc->subPluginFeatures->
fillDescriptionWidget( subWidget, &m_currentSelection );
foreach( QWidget * w, subWidget->findChildren<QWidget *>() )
for( QWidget * w : subWidget->findChildren<QWidget *>() )
{
if( w->parent() == subWidget )
{
Expand Down
2 changes: 1 addition & 1 deletion src/gui/FxMixerView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ void FxMixerView::deleteUnusedChannels()
{
// check if an instrument references to the current channel
bool empty=true;
foreach( Track* t, tracks )
for( Track* t : tracks )
{
if( t->type() == Track::InstrumentTrack )
{
Expand Down
4 changes: 2 additions & 2 deletions src/gui/MainWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ MainWindow::MainWindow() :

#if ! defined(LMMS_BUILD_APPLE)
QFileInfoList drives = QDir::drives();
foreach( const QFileInfo & drive, drives )
for( const QFileInfo & drive : drives )
{
root_paths += drive.absolutePath();
}
Expand Down Expand Up @@ -602,7 +602,7 @@ void MainWindow::finalize()
gui->songEditor()->parentWidget()->show();

// reset window title every time we change the state of a subwindow to show the correct title
foreach( QMdiSubWindow * subWindow, workspace()->subWindowList() )
for( const QMdiSubWindow * subWindow : workspace()->subWindowList() )
{
connect( subWindow, SIGNAL( windowStateChanged(Qt::WindowStates,Qt::WindowStates) ), this, SLOT( resetWindowTitle() ) );
}
Expand Down
2 changes: 1 addition & 1 deletion src/gui/editors/BBEditor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ void BBTrackContainerView::addAutomationTrack()

void BBTrackContainerView::removeBBView(int bb)
{
foreach( TrackView* view, trackViews() )
for( TrackView* view : trackViews() )
{
view->getTrackContentWidget()->removeTCOView( bb );
}
Expand Down
4 changes: 2 additions & 2 deletions src/gui/widgets/AutomatableButton.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ void automatableButtonGroup::activateButton( AutomatableButton * _btn )
m_buttons.indexOf( _btn ) != -1 )
{
model()->setValue( m_buttons.indexOf( _btn ) );
foreach( AutomatableButton * btn, m_buttons )
for( AutomatableButton * btn : m_buttons )
{
btn->update();
}
Expand All @@ -261,7 +261,7 @@ void automatableButtonGroup::updateButtons()
{
model()->setRange( 0, m_buttons.size() - 1 );
int i = 0;
foreach( AutomatableButton * btn, m_buttons )
for( AutomatableButton * btn : m_buttons )
{
btn->model()->setValue( i == model()->value() );
++i;
Expand Down
12 changes: 0 additions & 12 deletions src/tracks/BBTrack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -664,17 +664,5 @@ void BBTrackView::clickedTrackLabel()
{
Engine::getBBTrackContainer()->setCurrentBB( m_bbTrack->index() );
gui->getBBEditor()->show();
/* foreach( bbTrackView * tv,
trackContainerView()->findChildren<bbTrackView *>() )
{
tv->m_trackLabel->update();
}*/

}







2 changes: 1 addition & 1 deletion src/tracks/InstrumentTrack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@ InstrumentTrackView::~InstrumentTrackView()
InstrumentTrackWindow * InstrumentTrackView::topLevelInstrumentTrackWindow()
{
InstrumentTrackWindow * w = NULL;
foreach( QMdiSubWindow * sw,
for( const QMdiSubWindow * sw :
gui->mainWindow()->workspace()->subWindowList(
QMdiArea::ActivationHistoryOrder ) )
{
Expand Down

0 comments on commit dc7798e

Please sign in to comment.