[Scummvm-git-logs] scummvm master -> 941d22c047e1afb6b882114d2c7d3a1660cda5c0

criezy criezy at scummvm.org
Sun Oct 14 22:32:21 CEST 2018


This automated email contains information about 3 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
c39dcc57a0 OSYSTEM: Add backendInitialized() function
1e11da712b COMMON: Add mutex to protect access to the String memory pool
941d22c047 BACKENDS: Use OSystem::destroy() instead of deleting directly the g_system instance


Commit: c39dcc57a0dd77e5764592c67881de879a435d42
    https://github.com/scummvm/scummvm/commit/c39dcc57a0dd77e5764592c67881de879a435d42
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2018-10-14T21:25:02+01:00

Commit Message:
OSYSTEM: Add backendInitialized() function

Some feature, such as mutexes, are only available once the backend
has been initialized. This new function can be used to avoid using
those feature too early or too late.

Changed paths:
    common/system.cpp
    common/system.h


diff --git a/common/system.cpp b/common/system.cpp
index 51f41ec..97c8f85 100644
--- a/common/system.cpp
+++ b/common/system.cpp
@@ -49,6 +49,7 @@ OSystem::OSystem() {
 	_updateManager = nullptr;
 #endif
 	_fsFactory = nullptr;
+	_backendInitialized = false;
 }
 
 OSystem::~OSystem() {
@@ -98,6 +99,13 @@ void OSystem::initBackend() {
 	// set it.
 // 	if (!_fsFactory)
 // 		error("Backend failed to instantiate fs factory");
+
+	_backendInitialized = true;
+}
+
+void OSystem::destroy() {
+	_backendInitialized = false;
+	delete this;
 }
 
 bool OSystem::setGraphicsMode(const char *name) {
diff --git a/common/system.h b/common/system.h
index 7caf73a..405d6a9 100644
--- a/common/system.h
+++ b/common/system.h
@@ -190,11 +190,23 @@ protected:
 	 */
 	FilesystemFactory *_fsFactory;
 
+private:
+	/**
+	 * Indicate if initBackend() has been called.
+	 */
+	bool _backendInitialized;
+
 	//@}
 
 public:
 
 	/**
+	 *
+	 * Destoy this OSystem instance.
+	 */
+	void destroy();
+
+	/**
 	 * The following method is called once, from main.cpp, after all
 	 * config data (including command line params etc.) are fully loaded.
 	 *
@@ -205,6 +217,13 @@ public:
 	virtual void initBackend();
 
 	/**
+	 * Return false if initBackend() has not yet been called and true otherwise.
+	 * Some functionalities such as mutexes cannot be used until the backend
+	 * is initialized.
+	 */
+	bool backendInitialized() const { return _backendInitialized; }
+
+	/**
 	 * Allows the backend to perform engine specific init.
 	 * Called just before the engine is run.
 	 */


Commit: 1e11da712ba8359cb65b0a171c7ce83de6a17879
    https://github.com/scummvm/scummvm/commit/1e11da712ba8359cb65b0a171c7ce83de6a17879
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2018-10-14T21:25:16+01:00

Commit Message:
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

Changed paths:
    common/str.cpp
    common/str.h
    common/system.cpp


diff --git a/common/str.cpp b/common/str.cpp
index f60d7d5..8ed652f 100644
--- a/common/str.cpp
+++ b/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;
 
diff --git a/common/str.h b/common/str.h
index 5ed14b6..704114b 100644
--- a/common/str.h
+++ b/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
diff --git a/common/system.cpp b/common/system.cpp
index 97c8f85..c8023c8 100644
--- a/common/system.cpp
+++ b/common/system.cpp
@@ -105,6 +105,7 @@ void OSystem::initBackend() {
 
 void OSystem::destroy() {
 	_backendInitialized = false;
+	Common::String::releaseMemoryPoolMutex();
 	delete this;
 }
 


Commit: 941d22c047e1afb6b882114d2c7d3a1660cda5c0
    https://github.com/scummvm/scummvm/commit/941d22c047e1afb6b882114d2c7d3a1660cda5c0
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2018-10-14T21:25:33+01:00

Commit Message:
BACKENDS: Use OSystem::destroy() instead of deleting directly the g_system instance

Changed paths:
    backends/platform/3ds/main.cpp
    backends/platform/androidsdl/androidsdl-main.cpp
    backends/platform/dingux/main.cpp
    backends/platform/gph/gph-main.cpp
    backends/platform/linuxmoto/linuxmoto-main.cpp
    backends/platform/maemo/main.cpp
    backends/platform/null/null.cpp
    backends/platform/openpandora/op-main.cpp
    backends/platform/samsungtv/main.cpp
    backends/platform/sdl/amigaos/amigaos-main.cpp
    backends/platform/sdl/macosx/macosx-main.cpp
    backends/platform/sdl/posix/posix-main.cpp
    backends/platform/sdl/ps3/ps3-main.cpp
    backends/platform/sdl/psp2/psp2-main.cpp
    backends/platform/sdl/riscos/riscos-main.cpp
    backends/platform/sdl/win32/win32-main.cpp
    backends/platform/symbian/src/SymbianMain.cpp
    backends/platform/webos/main.cpp
    backends/platform/wince/wince-sdl.cpp


diff --git a/backends/platform/3ds/main.cpp b/backends/platform/3ds/main.cpp
index e9046d7..2098314 100644
--- a/backends/platform/3ds/main.cpp
+++ b/backends/platform/3ds/main.cpp
@@ -40,7 +40,7 @@ int main(int argc, char *argv[]) {
 // 		res = scummvm_main(argc, argv);
 	scummvm_main(0, nullptr);
 
-	delete dynamic_cast<_3DS::OSystem_3DS*>(g_system);
+	g_system->destroy();
 
 	// Turn on both screen backlights before exiting.
 	if (R_SUCCEEDED(gspLcdInit())) {
diff --git a/backends/platform/androidsdl/androidsdl-main.cpp b/backends/platform/androidsdl/androidsdl-main.cpp
index 364d686..304bce1 100644
--- a/backends/platform/androidsdl/androidsdl-main.cpp
+++ b/backends/platform/androidsdl/androidsdl-main.cpp
@@ -41,7 +41,7 @@ int main(int argc, char *argv[]) {
 	int res = scummvm_main(argc, argv);
 
 	// Free OSystem
-	delete (OSystem_ANDROIDSDL *)g_system;
+	g_system->destroy();
 
 	return res;
 }
diff --git a/backends/platform/dingux/main.cpp b/backends/platform/dingux/main.cpp
index 98b5058..7ec90ab 100644
--- a/backends/platform/dingux/main.cpp
+++ b/backends/platform/dingux/main.cpp
@@ -43,7 +43,7 @@ int main(int argc, char* argv[]) {
 	int res = scummvm_main(argc, argv);
 
 	// Free OSystem
-	delete (OSystem_SDL_Dingux *)g_system;
+	g_system->destroy();
 
 	return res;
 }
diff --git a/backends/platform/gph/gph-main.cpp b/backends/platform/gph/gph-main.cpp
index 5fce371..3b829de 100644
--- a/backends/platform/gph/gph-main.cpp
+++ b/backends/platform/gph/gph-main.cpp
@@ -43,7 +43,7 @@ int main(int argc, char *argv[]) {
 	int res = scummvm_main(argc, argv);
 
 	// Free OSystem
-	delete(OSystem_GPH *)g_system;
+	g_system->destroy();
 
 	return res;
 }
diff --git a/backends/platform/linuxmoto/linuxmoto-main.cpp b/backends/platform/linuxmoto/linuxmoto-main.cpp
index 507be9f..2f14e3d 100644
--- a/backends/platform/linuxmoto/linuxmoto-main.cpp
+++ b/backends/platform/linuxmoto/linuxmoto-main.cpp
@@ -36,7 +36,7 @@ int main(int argc, char *argv[]) {
 	int res = scummvm_main(argc, argv);
 
 	// Free OSystem
-	delete (OSystem_LINUXMOTO *)g_system;
+	g_system->destroy();
 
 	return res;
 }
diff --git a/backends/platform/maemo/main.cpp b/backends/platform/maemo/main.cpp
index 4735ae3..9d8b0f2 100644
--- a/backends/platform/maemo/main.cpp
+++ b/backends/platform/maemo/main.cpp
@@ -44,7 +44,7 @@ int main(int argc, char* argv[]) {
 	int res = scummvm_main(argc, argv);
 
 	// Free OSystem
-	delete (Maemo::OSystem_SDL_Maemo *)g_system;
+	g_system->destroy();
 
 	return res;
 }
diff --git a/backends/platform/null/null.cpp b/backends/platform/null/null.cpp
index be50fde..2f34ae8 100644
--- a/backends/platform/null/null.cpp
+++ b/backends/platform/null/null.cpp
@@ -137,7 +137,7 @@ int main(int argc, char *argv[]) {
 
 	// Invoke the actual ScummVM main entry point:
 	int res = scummvm_main(argc, argv);
-	delete (OSystem_NULL *)g_system;
+	g_system->destroy();
 	return res;
 }
 
diff --git a/backends/platform/openpandora/op-main.cpp b/backends/platform/openpandora/op-main.cpp
index 99026b8..09da93a 100644
--- a/backends/platform/openpandora/op-main.cpp
+++ b/backends/platform/openpandora/op-main.cpp
@@ -43,7 +43,7 @@ int main(int argc, char *argv[]) {
 	int res = scummvm_main(argc, argv);
 
 	// Free OSystem
-	delete(OSystem_OP *)g_system;
+	g_system->destroy();
 
 	return res;
 }
diff --git a/backends/platform/samsungtv/main.cpp b/backends/platform/samsungtv/main.cpp
index a390bd0..7db2d90 100644
--- a/backends/platform/samsungtv/main.cpp
+++ b/backends/platform/samsungtv/main.cpp
@@ -50,7 +50,7 @@ extern "C" int Game_Main(char *path, char *) {
 	int res = scummvm_main(0, 0);
 
 	// Free OSystem
-	delete (OSystem_SDL_SamsungTV *)g_system;
+	g_system->destroy();
 
 	return res;
 }
diff --git a/backends/platform/sdl/amigaos/amigaos-main.cpp b/backends/platform/sdl/amigaos/amigaos-main.cpp
index 7bbf8d1..ba11717 100644
--- a/backends/platform/sdl/amigaos/amigaos-main.cpp
+++ b/backends/platform/sdl/amigaos/amigaos-main.cpp
@@ -79,7 +79,7 @@ int main(int argc, char *argv[]) {
 	int res = scummvm_main(argc, argv);
 
 	// Free OSystem
-	delete (OSystem_AmigaOS *)g_system;
+	g_system->destroy();
 
 	return res;
 }
diff --git a/backends/platform/sdl/macosx/macosx-main.cpp b/backends/platform/sdl/macosx/macosx-main.cpp
index 1b9fc1b..3053d30 100644
--- a/backends/platform/sdl/macosx/macosx-main.cpp
+++ b/backends/platform/sdl/macosx/macosx-main.cpp
@@ -45,7 +45,7 @@ int main(int argc, char *argv[]) {
 	int res = scummvm_main(argc, argv);
 
 	// Free OSystem
-	delete (OSystem_MacOSX *)g_system;
+	g_system->destroy();
 
 	return res;
 }
diff --git a/backends/platform/sdl/posix/posix-main.cpp b/backends/platform/sdl/posix/posix-main.cpp
index 92354b2..7105ac6 100644
--- a/backends/platform/sdl/posix/posix-main.cpp
+++ b/backends/platform/sdl/posix/posix-main.cpp
@@ -45,7 +45,7 @@ int main(int argc, char *argv[]) {
 	int res = scummvm_main(argc, argv);
 
 	// Free OSystem
-	delete (OSystem_POSIX *)g_system;
+	g_system->destroy();
 
 	return res;
 }
diff --git a/backends/platform/sdl/ps3/ps3-main.cpp b/backends/platform/sdl/ps3/ps3-main.cpp
index 92c4a02..2777497 100644
--- a/backends/platform/sdl/ps3/ps3-main.cpp
+++ b/backends/platform/sdl/ps3/ps3-main.cpp
@@ -43,7 +43,7 @@ int main(int argc, char *argv[]) {
 	int res = scummvm_main(argc, argv);
 
 	// Free OSystem
-	delete (OSystem_PS3 *)g_system;
+	g_system->destroy();
 
 	return res;
 }
diff --git a/backends/platform/sdl/psp2/psp2-main.cpp b/backends/platform/sdl/psp2/psp2-main.cpp
index 70cc523..aca1483 100644
--- a/backends/platform/sdl/psp2/psp2-main.cpp
+++ b/backends/platform/sdl/psp2/psp2-main.cpp
@@ -56,7 +56,7 @@ int main(int argc, char *argv[]) {
 	int res = scummvm_main(argc, argv);
 
 	// Free OSystem
-	delete (OSystem_PSP2 *)g_system;
+	g_system->destroy();
 
 #ifdef __PSP2_DEBUG__
 	psp2shell_exit();
diff --git a/backends/platform/sdl/riscos/riscos-main.cpp b/backends/platform/sdl/riscos/riscos-main.cpp
index 3f7058e..c075c2d 100644
--- a/backends/platform/sdl/riscos/riscos-main.cpp
+++ b/backends/platform/sdl/riscos/riscos-main.cpp
@@ -45,7 +45,7 @@ int main(int argc, char *argv[]) {
 	int res = scummvm_main(argc, argv);
 
 	// Free OSystem
-	delete (OSystem_RISCOS *)g_system;
+	g_system->destroy();
 
 	return res;
 }
diff --git a/backends/platform/sdl/win32/win32-main.cpp b/backends/platform/sdl/win32/win32-main.cpp
index 4864347..4e9f6a4 100644
--- a/backends/platform/sdl/win32/win32-main.cpp
+++ b/backends/platform/sdl/win32/win32-main.cpp
@@ -72,7 +72,7 @@ int main(int argc, char *argv[]) {
 	int res = scummvm_main(argc, argv);
 
 	// Free OSystem
-	delete (OSystem_Win32 *)g_system;
+	g_system->destroy();
 
 	return res;
 }
diff --git a/backends/platform/symbian/src/SymbianMain.cpp b/backends/platform/symbian/src/SymbianMain.cpp
index d7ad26d..03cf407 100644
--- a/backends/platform/symbian/src/SymbianMain.cpp
+++ b/backends/platform/symbian/src/SymbianMain.cpp
@@ -89,7 +89,7 @@ int main(int argc, char *argv[]) {
 	int res = scummvm_main(argc, argv);
 
 	// Free OSystem
-	delete (OSystem_SDL_Symbian *)g_system;
+	g_system->destroy();
 
 	return res;
 }
diff --git a/backends/platform/webos/main.cpp b/backends/platform/webos/main.cpp
index 70b3fe1..48a265c 100644
--- a/backends/platform/webos/main.cpp
+++ b/backends/platform/webos/main.cpp
@@ -44,7 +44,7 @@ int main(int argc, char* argv[]) {
 	int res = scummvm_main(argc, argv);
 
 	// Free OSystem
-	delete (OSystem_SDL_WebOS *)g_system;
+	g_system->destroy();
 
 	return res;
 }
diff --git a/backends/platform/wince/wince-sdl.cpp b/backends/platform/wince/wince-sdl.cpp
index f03068f..8a19eac 100644
--- a/backends/platform/wince/wince-sdl.cpp
+++ b/backends/platform/wince/wince-sdl.cpp
@@ -237,7 +237,7 @@ int SDL_main(int argc, char **argv) {
 		res = scummvm_main(argc, argv);
 
 		// Free OSystem
-		delete(OSystem_WINCE3 *)g_system;
+		g_system->destroy();
 #if !defined(DEBUG) && !defined(__GNUC__)
 	}
 	__except(handleException(GetExceptionInformation())) {





More information about the Scummvm-git-logs mailing list