fixed more segfaults

git-svn-id: https://lmms.svn.sf.net/svnroot/lmms/trunk/lmms@496 0778d3d1-df1d-0410-868b-ea421aaaa00d

ChangeLog

@@ -1,3 +1,27 @@
+2007-07-21	Tobias Doerffel	<tobydox/at/users/dot/sourceforge/dot/net>
+
+	* 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	<tobydox/at/users/dot/sourceforge/dot/net>
 
 	* lmms.spec.in:

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"

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;
 
 } ;

include/mixer.h

@@ -137,6 +137,7 @@ public:
 
 	inline void removeAudioPort( audioPort * _port )
 	{
+		lock();
 		vvector<audioPort *>::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;

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<rackPlugin *> m_rackInserts;
 		
 	QVBoxLayout * m_mainLayout;

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();
 }
 
 

src/core/preset_preview_play_handle.cpp

@@ -62,7 +62,7 @@ public:
 		m_dataMutex()
 	{
 		setJournalling( FALSE );
-		m_previewInstrumentTrack =  dynamic_cast<instrumentTrack *>(
+		m_previewInstrumentTrack = dynamic_cast<instrumentTrack *>(
 					track::create( track::INSTRUMENT_TRACK,
 								this ) );
 		m_previewInstrumentTrack->setJournalling( FALSE );

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();
 }
 
 

src/widgets/rack_view.cpp

@@ -70,12 +70,7 @@ rackView::rackView( QWidget * _parent, track * _track, audioPort * _port ) :
 
 rackView::~rackView()
 {
-	for( vvector<rackPlugin *>::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<rackPlugin *>::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<rackPlugin *>::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<rackPlugin *>::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