RemoteVST: process all remaining messages after the process has quit

[ghost]

If the process exits, keep processing the remaining messages. Some might be debug messages or a msg that the vst loaded was 32 bits.

(cherry-picked msvc/ng)

[Wallacoloo]

It seems to me there's a possibility for a race condition:

1. m_plugin->messagesLeft() returns false.
2. VST posts new message and then exits.
3. m_plugin->isRunning() returns false.
4. Loop aborts even though there are pending messages.

I think the conditional should just be rearranged so that m_plugin->messagesLeft() is only checked after m_plugin->isRunning():

while( !m_quit && (m_plugin->isRunning() || m_plugin->messagesLeft()) )

I'm not very familiar with the area of the code though, so don't hesitate to point out why I might be wrong about this.

[ghost]

heh :) good catch, thanks. (I wanted to write something about why you were wrong but then realized you were right)

[Wallacoloo]

Thanks for the patch! I went ahead and made the adjustment mentioned above, and committed that to master: 4fd8ecd

Credited you by your original username, but if there was any other metadata in the original commit, it was probably lost since I had to reauthor the commit (couldn't pull from the original repo since it's been deleted).

[lukas-w]

@Wallacoloo It looks like this change is ineffective because of a second invalidate() call here:

lmms/src/core/RemotePlugin.cpp

Lines 454 to 460 in 392c753

	void RemotePlugin::processFinished( int exitCode,
	QProcess::ExitStatus exitStatus )
	{
	#ifndef SYNC_WITH_SHM_FIFO
	invalidate();
	#endif
	}

I ran into IdVstBasDllFormat messages not being processed when trying to implement 64bit VSTs on Linux. I feel like that invalidate() call can just be removed because ProcessWatcher will call invalidate() eventually, but I'm not actually sure about the reason @jasp00 added it there, nor do I understand what the #ifdef is for.

Do you have any idea how to approach this?

[Wallacoloo]

@lukas-w Interesting. I didn't dig around to figure out where we decide whether or not to build with SHM_FIFO.

I don't know enough about this area. I'm going to assume you're building with SHM_FIFO undefined, in which case it looks like we're communicating with the remote plugin using sockets.

If we try reading from the socket after the plugin has hung up, can we ever get data from it? I think we can - I think that data still hangs around after the process has exited. If that's the case, I don't understand why we can't still read queued messages from an "invalid" RemotePlugin. Can we just remove the if (!isInvalid()) check from the top of RemotePluginBase::read?

	void read( void * _buf, int _len )
	{
                /* REMOVE THIS CHECK
		if( isInvalid() )
		{
			memset( _buf, 0, _len );
			return;
		}
                */
		lock();
		while( isInvalid() == false &&  //< keep this check to avoid deadlocks
				_len > m_data->endPtr - m_data->startPtr )
		{
			unlock();
#ifndef LMMS_BUILD_WIN32
			usleep( 5 );
#endif
			lock();
		}
		fastMemCpy( _buf, m_data->data + m_data->startPtr, _len );
		m_data->startPtr += _len;
		// nothing left?
		if( m_data->startPtr == m_data->endPtr )
		{
			// then reset to 0
			m_data->startPtr = m_data->endPtr = 0;
		}
		unlock();
	}

On the other hand, if the socket can no longer be read after the remote plugin exits, then we're out of luck. We would need to somehow make sure remote plugins wait until the host reads all messages before exiting - which wouldn't be easy to do if the plugin crashes / etc.

Like I said, I don't know much about this area of LMMS.