COMMON: Add mutex to protect access to the String memory pool

This fixes a crash due to concurrent access to the global MemoryPool
used by the String class when String objects are used simultaneously
from several threads (as is for example the case when enabling the
cloud features).

See bug #10524: Thread safety issue with MemoryPool

common/str.cpp

@@ -25,10 +25,36 @@
 #include "common/memorypool.h"
 #include "common/str.h"
 #include "common/util.h"
+#include "common/mutex.h"
 
 namespace Common {
 
 MemoryPool *g_refCountPool = nullptr; // FIXME: This is never freed right now
+MutexRef g_refCountPoolMutex = nullptr;
+
+void lockMemoryPoolMutex() {
+	// The Mutex class can only be used once g_system is set and initialized,
+	// but we may use the String class earlier than that (it is for example
+	// used in the OSystem_POSIX constructor). However in those early stages
+	// we can hope we don't have multiple threads either.
+	if (!g_system || !g_system->backendInitialized())
+		return;
+	if (!g_refCountPoolMutex)
+		g_refCountPoolMutex = g_system->createMutex();
+	g_system->lockMutex(g_refCountPoolMutex);
+}
+
+void unlockMemoryPoolMutex() {
+	if (g_refCountPoolMutex)
+		g_system->unlockMutex(g_refCountPoolMutex);
+}
+
+void String::releaseMemoryPoolMutex() {
+	if (g_refCountPoolMutex){
+		g_system->deleteMutex(g_refCountPoolMutex);
+		g_refCountPoolMutex = nullptr;
+	}
+}
 
 static uint32 computeCapacity(uint32 len) {
 	// By default, for the capacity we use the next multiple of 32
@@ -173,12 +199,14 @@ void String::ensureCapacity(uint32 new_size, bool keep_old) {
 void String::incRefCount() const {
 	assert(!isStorageIntern());
 	if (_extern._refCount == nullptr) {
+		lockMemoryPoolMutex();
 		if (g_refCountPool == nullptr) {
 			g_refCountPool = new MemoryPool(sizeof(int));
 			assert(g_refCountPool);
 		}
 
 		_extern._refCount = (int *)g_refCountPool->allocChunk();
+		unlockMemoryPoolMutex();
 		*_extern._refCount = 2;
 	} else {
 		++(*_extern._refCount);
@@ -196,8 +224,10 @@ void String::decRefCount(int *oldRefCount) {
 		// The ref count reached zero, so we free the string storage
 		// and the ref count storage.
 		if (oldRefCount) {
+			lockMemoryPoolMutex();
 			assert(g_refCountPool);
 			g_refCountPool->freeChunk(oldRefCount);
+			unlockMemoryPoolMutex();
 		}
 		delete[] _str;
 

common/str.h

@@ -47,6 +47,8 @@ class String {
 public:
 	static const uint32 npos = 0xFFFFFFFF;
 
+	static void releaseMemoryPoolMutex();
+
 	typedef char          value_type;
 	/**
 	 * Unsigned version of the underlying type. This can be used to cast

common/system.cpp

@@ -105,6 +105,7 @@ void OSystem::initBackend() {
 
 void OSystem::destroy() {
 	_backendInitialized = false;
+	Common::String::releaseMemoryPoolMutex();
 	delete this;
 }