From 820c5ec8baaf63e337c7700ee016d3fc7dba5003 Mon Sep 17 00:00:00 2001 From: Tobias Doerffel Date: Fri, 17 Oct 2008 22:36:08 +0000 Subject: [PATCH] added helper thread processWatcher which monitors the remote plugin process - if it terminates unexpectedly, invalidate remotePlugin so LMMS doesn't lock up - fixes crashes and lockups when using VST plugins or ZynAddSubFX plugin git-svn-id: https://lmms.svn.sf.net/svnroot/lmms/trunk/lmms@1762 0778d3d1-df1d-0410-868b-ea421aaaa00d --- ChangeLog | 9 ++++ include/remote_plugin.h | 89 +++++++++++++++++++++++++++++++++----- src/core/remote_plugin.cpp | 54 +++++++++++++++++++---- 3 files changed, 134 insertions(+), 18 deletions(-) diff --git a/ChangeLog b/ChangeLog index 16c7cba43..77cd40820 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +2008-10-17 Tobias Doerffel + + * include/remote_plugin.h: + * src/core/remote_plugin.cpp: + added helper thread processWatcher which monitors the remote plugin + process - if it terminates unexpectedly, invalidate remotePlugin so + LMMS doesn't lock up - fixes crashes and lockups when using VST + plugins or ZynAddSubFX plugin + 2008-10-16 Tobias Doerffel * src/core/piano.cpp: diff --git a/include/remote_plugin.h b/include/remote_plugin.h index 8a4ce363d..b8b9f577f 100755 --- a/include/remote_plugin.h +++ b/include/remote_plugin.h @@ -83,6 +83,7 @@ typedef int32_t key_t; #else #include #include +#include #endif // sometimes we need to exchange bigger messages (e.g. for VST parameter dumps) @@ -116,6 +117,7 @@ class shmFifo public: // constructor for master-side shmFifo() : + m_invalid( false ), m_master( true ), m_shmKey( 0 ), #ifdef USE_NATIVE_SHMEM @@ -154,6 +156,7 @@ public: static int k = 0; m_data->dataSem.semKey = ( getpid()<<10 ) + ++k; m_data->messageSem.semKey = ( getpid()<<10 ) + ++k; + m_invalid( false ), m_dataSem.setKey( QString::number( m_data->dataSem.semKey ), 1, QSystemSemaphore::Create ); m_messageSem.setKey( QString::number( @@ -178,6 +181,7 @@ public: // constructor for remote-/client-side - use _shm_key for making up // the connection to master shmFifo( key_t _shm_key ) : + m_invalid( false ), m_master( false ), m_shmKey( 0 ), #ifdef USE_NATIVE_SHMEM @@ -235,6 +239,16 @@ public: } } + inline bool isInvalid( void ) const + { + return m_invalid; + } + + void invalidate( void ) + { + m_invalid = true; + } + // do we act as master (i.e. not as remote-process?) inline bool isMaster( void ) const { @@ -244,7 +258,7 @@ public: // recursive lock inline void lock( void ) { - if( ++m_lockDepth == 1 ) + if( !isInvalid() && ++m_lockDepth == 1 ) { #ifdef LMMS_BUILD_WIN32 m_dataSem.acquire(); @@ -273,11 +287,14 @@ public: // wait until message-semaphore is available inline void waitForMessage( void ) { + if( !isInvalid() ) + { #ifdef LMMS_BUILD_WIN32 - m_messageSem.acquire(); + m_messageSem.acquire(); #else - sem_wait( m_messageSem ); + sem_wait( m_messageSem ); #endif + } } // increase message-semaphore @@ -329,6 +346,10 @@ public: inline bool messagesLeft( void ) { + if( isInvalid() ) + { + return false; + } #ifdef LMMS_BUILD_WIN32 lock(); const bool empty = ( m_data->startPtr == m_data->endPtr ); @@ -365,6 +386,11 @@ private: void read( void * _buf, int _len ) { + if( isInvalid() ) + { + memset( _buf, 0, _len ); + return; + } lock(); while( _len > m_data->endPtr - m_data->startPtr ) { @@ -387,6 +413,10 @@ private: void write( const void * _buf, int _len ) { + if( isInvalid() ) + { + return; + } lock(); while( _len > SHM_FIFO_SIZE - m_data->endPtr ) { @@ -411,6 +441,7 @@ private: unlock(); } + volatile bool m_invalid; bool m_master; key_t m_shmKey; #ifdef USE_NATIVE_SHMEM @@ -436,7 +467,6 @@ private: enum RemoteMessageIDs { IdUndefined, - IdGeneralFailure, IdInitDone, IdQuit, IdSampleRateInformation, @@ -576,16 +606,23 @@ public: protected: - const shmFifo * in( void ) const + inline const shmFifo * in( void ) const { return m_in; } - const shmFifo * out( void ) const + inline const shmFifo * out( void ) const { return m_out; } + inline void invalidate( void ) + { + m_in->invalidate(); + m_out->invalidate(); + m_in->messageSent(); + } + private: shmFifo * m_in; @@ -597,6 +634,32 @@ private: #ifndef BUILD_REMOTE_PLUGIN_CLIENT + +class remotePlugin; + +class processWatcher : public QThread +{ +public: + processWatcher( remotePlugin * ); + virtual ~processWatcher() + { + } + + + void quit( void ) + { + m_quit = true; + } + +private: + virtual void run( void ); + + remotePlugin * m_plugin; + volatile bool m_quit; + +} ; + + class EXPORT remotePlugin : public remotePluginBase { public: @@ -604,9 +667,14 @@ public: bool _wait_for_init_done = true ); virtual ~remotePlugin(); + inline bool isRunning( void ) + { + return m_process.state() != QProcess::NotRunning; + } + inline void waitForInitDone( void ) { - m_failed = waitForMessage( IdInitDone, TRUE ).id != IdInitDone; + m_failed = waitForMessage( IdInitDone, true ).id != IdInitDone; } virtual bool processMessage( const message & _m ); @@ -667,6 +735,7 @@ private: bool m_failed; QProcess m_process; + processWatcher m_watcher; QMutex m_commMutex; bool m_splitChannels; @@ -681,6 +750,7 @@ private: int m_inputCount; int m_outputCount; + friend class processWatcher; } ; #endif @@ -859,7 +929,7 @@ remotePluginBase::message remotePluginBase::waitForMessage( { return m; } - else if( m.id == IdGeneralFailure ) + else if( m.id == IdUndefined ) { return m; } @@ -913,7 +983,7 @@ bool remotePluginClient::processMessage( const message & _m ) bool reply = false; switch( _m.id ) { - case IdGeneralFailure: + case IdUndefined: return false; case IdSampleRateInformation: @@ -952,7 +1022,6 @@ bool remotePluginClient::processMessage( const message & _m ) case IdInitDone: break; - case IdUndefined: default: fprintf( stderr, "undefined message: %d\n", (int) _m.id ); diff --git a/src/core/remote_plugin.cpp b/src/core/remote_plugin.cpp index 2f9c847ea..596ec3124 100644 --- a/src/core/remote_plugin.cpp +++ b/src/core/remote_plugin.cpp @@ -38,11 +38,39 @@ +// simple helper thread monitoring our remotePlugin - if process terminates +// unexpectedly invalidate plugin so LMMS doesn't lock up +processWatcher::processWatcher( remotePlugin * _p ) : + QThread(), + m_plugin( _p ), + m_quit( false ) +{ +} + + +void processWatcher::run( void ) +{ + while( !m_quit && m_plugin->isRunning() ) + { + msleep( 200 ); + } + if( !m_quit ) + { + fprintf( stderr, + "remote plugin died! invalidating now.\n" ); + m_plugin->invalidate(); + } +} + + + + remotePlugin::remotePlugin( const QString & _plugin_executable, bool _wait_for_init_done ) : remotePluginBase( new shmFifo(), new shmFifo() ), m_failed( true ), + m_watcher( this ), m_commMutex( QMutex::Recursive ), m_splitChannels( false ), #ifdef USE_NATIVE_SHMEM @@ -65,6 +93,8 @@ remotePlugin::remotePlugin( const QString & _plugin_executable, m_process.setProcessChannelMode( QProcess::MergedChannels ); m_process.start( exec, args ); + m_watcher.start( QThread::LowestPriority ); + resizeSharedProcessingMemory(); if( _wait_for_init_done ) @@ -79,9 +109,12 @@ remotePlugin::remotePlugin( const QString & _plugin_executable, remotePlugin::~remotePlugin() { + m_watcher.quit(); + m_watcher.wait(); + if( m_failed == false ) { - if( m_process.state() == QProcess::Running ) + if( isRunning() ) { lock(); sendMessage( IdQuit ); @@ -109,11 +142,17 @@ remotePlugin::~remotePlugin() bool remotePlugin::process( const sampleFrame * _in_buf, sampleFrame * _out_buf ) { - if( m_failed ) + const fpp_t frames = engine::getMixer()->framesPerPeriod(); + + if( m_failed || !isRunning() ) { + if( _out_buf != NULL ) + { + engine::getMixer()->clearAudioBuffer( _out_buf, + frames ); + } return false; } - const fpp_t frames = engine::getMixer()->framesPerPeriod(); if( m_shm == NULL ) { @@ -287,8 +326,8 @@ bool remotePlugin::processMessage( const message & _m ) bool reply = false; switch( _m.id ) { - case IdGeneralFailure: - return( false ); + case IdUndefined: + return false; case IdInitDone: reply = true; @@ -318,7 +357,6 @@ bool remotePlugin::processMessage( const message & _m ) case IdProcessingDone: case IdQuit: - case IdUndefined: default: break; } @@ -328,11 +366,11 @@ bool remotePlugin::processMessage( const message & _m ) } unlock(); - return( true ); + return true; } - +#include "moc_remote_plugin.cxx"