From 6b89322a7c188cef9926a22397cdfc4eedc6f721 Mon Sep 17 00:00:00 2001 From: Tobias Doerffel Date: Sat, 21 Jul 2007 10:40:51 +0000 Subject: [PATCH] fixed more segfaults git-svn-id: https://lmms.svn.sf.net/svnroot/lmms/trunk/lmms@496 0778d3d1-df1d-0410-868b-ea421aaaa00d --- ChangeLog | 24 +++++ configure.in | 8 +- include/effect_tab_widget.h | 15 ++- include/mixer.h | 33 +++++- include/rack_view.h | 9 +- src/core/mixer.cpp | 22 ++-- src/core/preset_preview_play_handle.cpp | 2 +- src/tracks/instrument_track.cpp | 13 +++ src/widgets/rack_view.cpp | 131 +++++++++++++----------- 9 files changed, 177 insertions(+), 80 deletions(-) diff --git a/ChangeLog b/ChangeLog index e6b0c195c..bba925095 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,27 @@ +2007-07-21 Tobias Doerffel + + * include/mixer.h: + * src/core/mixer.cpp: + added more mutexes to protect all important data-structures more + granularly and safely - fixes some potential segfaults + + * src/tracks/instrument_track.cpp: + - lock mixer while loading instrument / track-specific settings - fixes + segfault when dragging preset/instrument plugin to existing track + while playing + - remove effects in loadTrackSpecificSettings() if none were in preset + (e.g. old preset-file) + + * include/rack_view.h: + * include/effect_tab_widget.h: + * src/widgets/rack_view.cpp: + - delete all existing plugins before adding new ones in + rackView::loadSettings() + - cleanups + + * configure.in: + cleanups + 2007-07-20 Tobias Doerffel * lmms.spec.in: diff --git a/configure.in b/configure.in index 8631a5f9f..3660d045f 100644 --- a/configure.in +++ b/configure.in @@ -2,8 +2,8 @@ # Process this file with autoconf to produce a configure script. AC_PREREQ(2.50) -AC_INIT(lmms, 0.2.1-svn20070720, lmms-devel/at/lists/dot/sf/dot/net) -AM_INIT_AUTOMAKE(lmms, 0.2.1-svn20070720) +AC_INIT(lmms, 0.2.1-svn20070721, lmms-devel/at/lists/dot/sf/dot/net) +AM_INIT_AUTOMAKE(lmms, 0.2.1-svn20070721) AM_CONFIG_HEADER(config.h) @@ -52,7 +52,7 @@ AM_CONDITIONAL(BUILD_LINUX, test "$build_linux" = "true") # -fomit-frame-pointer crashes wine on Ubuntu Dapper--Danny 7/21/06 #DEFAULTFLAGS="-floop-optimize2 -fomit-frame-pointer -fgcse-sm -fgcse-las" -DEFAULTFLAGS="-O2 -g0 -fPIC" #"-floop-optimize2 -fgcse-sm -fgcse-las" +DEFAULTFLAGS="-O2 -fPIC" #"-floop-optimize2 -fgcse-sm -fgcse-las" # Tested with GCC 4.0--needs to be tested with 4.1--Danny 7/21/06 if test "x`$CC --version|head -1|cut -d\ -f3|cut -d. -f1`" = "x4" ; then @@ -545,8 +545,6 @@ lmmsdatadir="$datadir/$PACKAGE" AC_SUBST(lmmsdatadir) -#CFLAGS="$CXXFLAGS -g -O2" -#CXXFLAGS="$CXXFLAGS -g -O2" EXTRA_WARNINGS="-Wextra -Wno-unused-parameter -Winline -Wdisabled-optimization" if test "x$CXX" == "xg++" ; then CXXFLAGS="$CXXFLAGS -ansi -Wall $EXTRA_WARNINGS -fno-exceptions" diff --git a/include/effect_tab_widget.h b/include/effect_tab_widget.h index fdddb56e4..a29fc01e8 100644 --- a/include/effect_tab_widget.h +++ b/include/effect_tab_widget.h @@ -50,12 +50,12 @@ #endif #include "journalling_object.h" +#include "rack_view.h" class audioPort; class groupBox; class instrumentTrack; -class rackView; class sampleTrack; class track; @@ -80,23 +80,32 @@ public: void setupWidget( void ); + inline void deleteAllEffects( void ) + { + m_rack->deleteAllPlugins(); + } + + signals: void closed( void ); + private slots: void addEffect( void ); void setBypass( bool _state ); + protected: virtual void closeEvent( QCloseEvent * _ce ); + private: track * m_track; audioPort * m_port; - + groupBox * m_effectsGroupBox; QPushButton * m_addButton; - + rackView * m_rack; } ; diff --git a/include/mixer.h b/include/mixer.h index ba480de87..843c04286 100644 --- a/include/mixer.h +++ b/include/mixer.h @@ -137,6 +137,7 @@ public: inline void removeAudioPort( audioPort * _port ) { + lock(); vvector::iterator it = qFind( m_audioPorts.begin(), m_audioPorts.end(), _port ); @@ -144,6 +145,7 @@ public: { m_audioPorts.erase( it ); } + unlock(); } @@ -164,7 +166,9 @@ public: { if( criticalXRuns() == FALSE ) { + lockPlayHandles(); m_playHandles.push_back( _ph ); + unlockPlayHandles(); return( TRUE ); } delete _ph; @@ -173,7 +177,9 @@ public: inline void removePlayHandle( const playHandle * _ph ) { + lockPlayHandlesToRemove(); m_playHandlesToRemove.push_back( _ph ); + unlockPlayHandlesToRemove(); } inline playHandleVector & playHandles( void ) @@ -246,14 +252,33 @@ public: // methods needed by other threads to alter knob values, waveforms, etc void lock( void ) { - m_mixMutex.lock(); + m_globalMutex.lock(); } void unlock( void ) { - m_mixMutex.unlock(); + m_globalMutex.unlock(); } + void lockPlayHandles( void ) + { + m_playHandlesMutex.lock(); + } + + void unlockPlayHandles( void ) + { + m_playHandlesMutex.unlock(); + } + + void lockPlayHandlesToRemove( void ) + { + m_playHandlesToRemoveMutex.lock(); + } + + void unlockPlayHandlesToRemove( void ) + { + m_playHandlesToRemoveMutex.unlock(); + } // audio-buffer-mgm void FASTCALL bufferToPort( const sampleFrame * _buf, @@ -365,7 +390,9 @@ private: QString m_midiClientName; - QMutex m_mixMutex; + QMutex m_globalMutex; + QMutex m_playHandlesMutex; + QMutex m_playHandlesToRemoveMutex; fifo * m_fifo; diff --git a/include/rack_view.h b/include/rack_view.h index d6bc252bc..d90f79dc5 100644 --- a/include/rack_view.h +++ b/include/rack_view.h @@ -72,14 +72,19 @@ public: return( "rack" ); } + void deleteAllPlugins( void ); + + public slots: void moveUp( rackPlugin * _plugin ); void moveDown( rackPlugin * _plugin ); void deletePlugin( rackPlugin * _plugin ); - + + private: void redraw(); - + + vvector m_rackInserts; QVBoxLayout * m_mainLayout; diff --git a/src/core/mixer.cpp b/src/core/mixer.cpp index a2b15ebfa..8b8f75331 100644 --- a/src/core/mixer.cpp +++ b/src/core/mixer.cpp @@ -72,9 +72,9 @@ mixer::mixer( void ) : m_audioDev( NULL ), m_oldAudioDev( NULL ), #ifndef QT3 - m_mixMutex( QMutex::Recursive ) + m_globalMutex( QMutex::Recursive ) #else - m_mixMutex( TRUE ) + m_globalMutex( TRUE ) #endif { if( configManager::inst()->value( "mixer", "framesperaudiobuffer" @@ -188,7 +188,7 @@ bool mixer::criticalXRuns( void ) const void mixer::setClipScaling( bool _state ) { - m_mixMutex.lock(); + lock(); m_scaleClip = _state; @@ -224,7 +224,7 @@ void mixer::setClipScaling( bool _state ) m_analBuffer = 1; } - m_mixMutex.unlock(); + unlock(); } @@ -248,11 +248,13 @@ const surroundSampleFrame * mixer::renderNextBuffer( void ) // now we have to make sure no other thread does anything bad // while we're acting... - m_mixMutex.lock(); + lock(); // remove all play-handles that have to be deleted and delete // them if they still exist... // maybe this algorithm could be optimized... + lockPlayHandles(); + lockPlayHandlesToRemove(); constPlayHandleVector::iterator it_rem = m_playHandlesToRemove.begin(); while( it_rem != m_playHandlesToRemove.end() ) { @@ -267,6 +269,8 @@ const surroundSampleFrame * mixer::renderNextBuffer( void ) m_playHandlesToRemove.erase( it_rem ); } + unlockPlayHandlesToRemove(); + unlockPlayHandles(); // now swap the buffers... current buffer becomes next (last) // buffer and the next buffer becomes current (first) buffer @@ -288,6 +292,7 @@ const surroundSampleFrame * mixer::renderNextBuffer( void ) // if( criticalXRuns() == FALSE ) { + lockPlayHandles(); csize idx = 0; if( m_parallelizingLevel > 1 ) { @@ -359,6 +364,7 @@ const surroundSampleFrame * mixer::renderNextBuffer( void ) } } } + unlockPlayHandles(); engine::getSongEditor()->processNextBuffer(); @@ -380,7 +386,7 @@ const surroundSampleFrame * mixer::renderNextBuffer( void ) emit nextAudioBuffer( m_readBuf, m_framesPerAudioBuffer ); - m_mixMutex.unlock(); + unlock(); // and trigger LFOs envelopeAndLFOWidget::triggerLFO(); @@ -401,6 +407,7 @@ const surroundSampleFrame * mixer::renderNextBuffer( void ) void mixer::clear( void ) { // TODO: m_midiClient->noteOffAll(); + lockPlayHandlesToRemove(); for( playHandleVector::iterator it = m_playHandles.begin(); it != m_playHandles.end(); ++it ) { @@ -411,6 +418,7 @@ void mixer::clear( void ) m_playHandlesToRemove.push_back( *it ); } } + unlockPlayHandlesToRemove(); } @@ -558,6 +566,7 @@ void mixer::restoreAudioDevice( void ) void mixer::removePlayHandles( track * _track ) { + lockPlayHandles(); playHandleVector::iterator it = m_playHandles.begin(); while( it != m_playHandles.end() ) { @@ -571,6 +580,7 @@ void mixer::removePlayHandles( track * _track ) ++it; } } + unlockPlayHandles(); } diff --git a/src/core/preset_preview_play_handle.cpp b/src/core/preset_preview_play_handle.cpp index 95e440b2f..a76b9214c 100644 --- a/src/core/preset_preview_play_handle.cpp +++ b/src/core/preset_preview_play_handle.cpp @@ -62,7 +62,7 @@ public: m_dataMutex() { setJournalling( FALSE ); - m_previewInstrumentTrack = dynamic_cast( + m_previewInstrumentTrack = dynamic_cast( track::create( track::INSTRUMENT_TRACK, this ) ); m_previewInstrumentTrack->setJournalling( FALSE ); diff --git a/src/tracks/instrument_track.cpp b/src/tracks/instrument_track.cpp index 3691d1f0c..e7bff71bd 100644 --- a/src/tracks/instrument_track.cpp +++ b/src/tracks/instrument_track.cpp @@ -1157,6 +1157,7 @@ void instrumentTrack::loadTrackSpecificSettings( const QDomElement & _this ) { invalidateAllMyNPH(); + engine::getMixer()->lock(); setName( _this.attribute( "name" ) ); m_volumeKnob->loadSettings( _this, "vol" ); @@ -1166,6 +1167,8 @@ void instrumentTrack::loadTrackSpecificSettings( const QDomElement & _this ) m_pianoWidget->loadSettings( _this, "basenote" ); int tab = _this.attribute( "tab" ).toInt(); + bool had_fx = FALSE; + QDomNode node = _this.firstChild(); while( !node.isNull() ) { @@ -1186,6 +1189,7 @@ void instrumentTrack::loadTrackSpecificSettings( const QDomElement & _this ) else if( m_effWidget->nodeName() == node.nodeName() ) { m_effWidget->restoreState( node.toElement() ); + had_fx = TRUE; } else if( automationPattern::classNodeName() != node.nodeName() ) @@ -1208,6 +1212,11 @@ void instrumentTrack::loadTrackSpecificSettings( const QDomElement & _this ) } node = node.nextSibling(); } + if( !had_fx ) + { + m_effWidget->deleteAllEffects(); + } + engine::getMixer()->unlock(); m_tabWidget->setActiveTab( tab ); @@ -1227,8 +1236,10 @@ instrument * instrumentTrack::loadInstrument( const QString & _plugin_name ) { invalidateAllMyNPH(); + engine::getMixer()->lock(); delete m_instrument; m_instrument = instrument::instantiate( _plugin_name, this ); + engine::getMixer()->unlock(); m_tabWidget->addTab( m_instrument, tr( "PLUGIN" ), 0 ); m_tabWidget->setActiveTab( 0 ); @@ -1338,6 +1349,7 @@ void instrumentTrack::dropEvent( QDropEvent * _de ) void instrumentTrack::invalidateAllMyNPH( void ) { + engine::getMixer()->lock(); for( int i = 0; i < NOTES; ++i ) { m_notes[i] = NULL; @@ -1346,6 +1358,7 @@ void instrumentTrack::invalidateAllMyNPH( void ) // invalidate all note-play-handles linked to this channel m_processHandles.clear(); engine::getMixer()->removePlayHandles( this ); + engine::getMixer()->unlock(); } diff --git a/src/widgets/rack_view.cpp b/src/widgets/rack_view.cpp index 216597d35..c550e4339 100644 --- a/src/widgets/rack_view.cpp +++ b/src/widgets/rack_view.cpp @@ -70,12 +70,7 @@ rackView::rackView( QWidget * _parent, track * _track, audioPort * _port ) : rackView::~rackView() { - for( vvector::iterator it = m_rackInserts.begin(); - it != m_rackInserts.end(); ) - { - delete *it; - m_rackInserts.erase( it ); - } + deleteAllPlugins(); } @@ -120,6 +115,75 @@ void rackView::addEffect( effect * _e ) +void FASTCALL rackView::saveSettings( QDomDocument & _doc, + QDomElement & _this ) +{ + _this.setAttribute( "numofeffects", m_rackInserts.count() ); + for( vvector::iterator it = m_rackInserts.begin(); + it != m_rackInserts.end(); it++ ) + { + QDomElement ef = ( *it )->saveState( _doc, _this ); + ef.setAttribute( "name", + ( *it )->getEffect()->getDescriptor()->name ); + ef.setAttribute( "key", + ( *it )->getEffect()->getKey().dumpBase64() ); + } +} + + + + +void FASTCALL rackView::loadSettings( const QDomElement & _this ) +{ + deleteAllPlugins(); + + const int plugin_cnt = _this.attribute( "numofeffects" ).toInt(); + + QDomNode node = _this.firstChild(); + for( int i = 0; i < plugin_cnt; i++ ) + { + if( node.isElement() && node.nodeName() == "effect" ) + { + QDomElement cn = node.toElement(); + const QString name = cn.attribute( "name" ); + // we have this really convenient key-ctor + // which takes a QString and decodes the + // base64-data inside :-) + effectKey key( cn.attribute( "key" ) ); + addEffect( effect::instantiate( name, &key ) ); + // TODO: somehow detect if effect is sub-plugin-capable + // but couldn't load sub-plugin with requsted key + if( node.isElement() ) + { + if( m_rackInserts.last()->nodeName() == + node.nodeName() ) + { + m_rackInserts.last()->restoreState( + node.toElement() ); + } + } + } + node = node.nextSibling(); + } + +} + + + + +void rackView::deleteAllPlugins( void ) +{ + for( vvector::iterator it = m_rackInserts.begin(); + it != m_rackInserts.end(); ++it ) + { + delete *it; + } + m_rackInserts.clear(); +} + + + + void rackView::moveUp( rackPlugin * _plugin ) { if( _plugin != m_rackInserts.first() ) @@ -180,7 +244,7 @@ void rackView::deletePlugin( rackPlugin * _plugin ) #ifdef QT3 m_scrollArea->removeChild( _plugin ); #endif - + m_rackInserts.erase( qFind( m_rackInserts.begin(), m_rackInserts.end(), _plugin ) ); delete _plugin; @@ -213,59 +277,6 @@ void rackView::redraw() -void FASTCALL rackView::saveSettings( QDomDocument & _doc, - QDomElement & _this ) -{ - _this.setAttribute( "numofeffects", m_rackInserts.count() ); - for( vvector::iterator it = m_rackInserts.begin(); - it != m_rackInserts.end(); it++ ) - { - QDomElement ef = ( *it )->saveState( _doc, _this ); - ef.setAttribute( "name", - ( *it )->getEffect()->getDescriptor()->name ); - ef.setAttribute( "key", - ( *it )->getEffect()->getKey().dumpBase64() ); - } -} - - - - -void FASTCALL rackView::loadSettings( const QDomElement & _this ) -{ - const int plugin_cnt = _this.attribute( "numofeffects" ).toInt(); - - QDomNode node = _this.firstChild(); - for( int i = 0; i < plugin_cnt; i++ ) - { - if( node.isElement() && node.nodeName() == "effect" ) - { - QDomElement cn = node.toElement(); - const QString name = cn.attribute( "name" ); - // we have this really convenient key-ctor - // which takes a QString and decodes the - // base64-data inside :-) - effectKey key( cn.attribute( "key" ) ); - addEffect( effect::instantiate( name, &key ) ); - // TODO: somehow detect if effect is sub-plugin-capable - // but couldn't load sub-plugin with requsted key - if( node.isElement() ) - { - if( m_rackInserts.last()->nodeName() == - node.nodeName() ) - { - m_rackInserts.last()->restoreState( - node.toElement() ); - } - } - } - node = node.nextSibling(); - } - -} - - - #include "rack_view.moc" #endif