Incorporate suggestions from Richard's review of the LLPlugin changes.

Use LLMutexLock (stack-based locker/unlocker) for the straightforward cases instead of explicit lock()/unlock().

There are still a couple of cases (one overlapping lock lifetime and two loops that unlock the mutex to call another function inside the loop) where I'm leaving explicit lock/unlock calls.

Rename LLPluginProcessParent::sPollThread to sReadThread, for consistency.

Made the LLPluginProcessParent destructor hold mIncomingQueueMutex while removing the instance from the global list -- this should prevent a possible race condition in LLPluginProcessParent::poll().

Removed a redundant check when calling LLPluginProcessParent::setUseReadThread().

1 files changed, 3 insertions, 6 deletions

diff --git a/indra/llplugin/llpluginmessagepipe.cpp b/indra/llplugin/llpluginmessagepipe.cpp
index a62534de95..89f8b44569 100644
--- a/indra/llplugin/llpluginmessagepipe.cpp
+++ b/indra/llplugin/llpluginmessagepipe.cpp
@@ -117,10 +117,9 @@ LLPluginMessagePipe::~LLPluginMessagePipe()
 bool LLPluginMessagePipe::addMessage(const std::string &message)
 {
 	// queue the message for later output
-	mOutputMutex.lock();
+	LLMutexLock lock(&mOutputMutex);
 	mOutput += message;
 	mOutput += MESSAGE_DELIMITER;	// message separator
-	mOutputMutex.unlock();
 	
 	return true;
 }
@@ -173,7 +172,7 @@ bool LLPluginMessagePipe::pumpOutput()
 		apr_status_t status;
 		apr_size_t size;
 		
-		mOutputMutex.lock();
+		LLMutexLock lock(&mOutputMutex);
 		if(!mOutput.empty())
 		{
 			// write any outgoing messages
@@ -225,7 +224,6 @@ bool LLPluginMessagePipe::pumpOutput()
 				result = false;
 			}
 		}
-		mOutputMutex.unlock();
 	}
 	
 	return result;
@@ -288,9 +286,8 @@ bool LLPluginMessagePipe::pumpInput(F64 timeout)
 				
 				if(size > 0)
 				{
-					mInputMutex.lock();
+					LLMutexLock lock(&mInputMutex);
 					mInput.append(input_buf, size);
-					mInputMutex.unlock();
 				}
 
 				if(status == APR_SUCCESS)