- Fixed: Memory leaks due to the old CSPtrTypeArray and CSObjArray co…

…ntainers, replaced with new ones using smart pointers as wrappers.

- Fixed: Possible random errors caused by reading some internal data before it was initialized during server startup (CCrypto::client_keys, holding SphereCrypt data, and ThreadHolder, holding threads data).
- Fixed: CSString instances becoming invalid after calling the Clear method.

CMakeLists.txt

@@ -7,6 +7,7 @@ SET (CMAKE_NO_GIT_REVISION	false CACHE BOOL "Do not try to retrieve the current
 
 SET (WIN32_SPAWN_CONSOLE	false CACHE BOOL "Spawn an additional console, useful for debugging.")
 SET (USE_ASAN			false CACHE BOOL "Enable AddressSanitizer.")
+SET (USE_MSAN			false CACHE BOOL "Enable MemorySanitizer.")
 SET (USE_LSAN			false CACHE BOOL "Enable LeakSanitizer.")
 SET (USE_UBSAN			false CACHE BOOL "Enable Undefined Behavior Sanitizer.")
 

Changelog.txt

@@ -3380,3 +3380,8 @@ Additionally, the problem of zig-zag issue following in the South direction has
 
 08-10-2023, Nolok & xwerswoodx
 - Fixed: Setting P inside the @Click trigger made it being called endlessly.
+
+16-10-2023, Nolok
+- Fixed: Memory leaks due to the old CSPtrTypeArray and CSObjArray containers, replaced with new ones using smart pointers as wrappers. 
+- Fixed: Possible random errors caused by reading some internal data before it was initialized during server startup (CCrypto::client_keys, holding SphereCrypt data, and ThreadHolder, holding threads data).
+- Fixed: CSString instances becoming invalid after calling the Clear method.

src/CMakeSources.cmake

@@ -255,13 +255,14 @@ src/common/sphere_library/CSTime.cpp
 src/common/sphere_library/CSTime.h
 src/common/sphere_library/CSWindow.cpp
 src/common/sphere_library/CSWindow.h
-src/common/sphere_library/container_ops.h
+src/common/sphere_library/scontainer_ops.h
 src/common/sphere_library/smap.h
 src/common/sphere_library/smutex.h
 src/common/sphere_library/smutex.cpp
 src/common/sphere_library/squeues.h
 src/common/sphere_library/sresetevents.cpp
 src/common/sphere_library/sresetevents.h
+src/common/sphere_library/sptr_containers.h
 src/common/sphere_library/sstacks.h
 src/common/sphere_library/sstring.cpp
 src/common/sphere_library/sstring.h

src/common/CException.cpp

@@ -281,8 +281,8 @@ void _cdecl Signal_Terminate(int sig = 0) // If shutdown is initialized
     }
 
     g_Serv.SetExitFlag(SIGABRT);
-    for (size_t i = 0; i < ThreadHolder::getActiveThreads(); ++i)
-        ThreadHolder::getThreadAt(i)->terminate(false);
+    for (size_t i = 0; i < ThreadHolder::get()->getActiveThreads(); ++i)
+        ThreadHolder::get()->getThreadAt(i)->terminate(false);
     //exit(EXIT_FAILURE);
 }
 

src/common/CException.h

@@ -115,7 +115,7 @@ class CAssert : public CSError
 #endif
 
 // _EXC_CAUGHT
-#define _EXC_CAUGHT static_cast<AbstractSphereThread *>(ThreadHolder::current())->exceptionCaught()
+#define _EXC_CAUGHT static_cast<AbstractSphereThread *>(ThreadHolder::get()->current())->exceptionCaught()
 
 /*--- Main (non SUB) macros ---*/
 

src/common/CExpression.cpp

@@ -48,15 +48,15 @@ dword ahextoi( lpctstr pszStr ) // Convert hex string to integer
 		}
 		else if ( !bHex && ( ch == '.' ) )
 		{
-			pszStr++;
+			++ pszStr;
 			continue;
 		}
 		else
 			break;
 
 		val *= ( bHex ? 0x10 : 10 );
 		val += ch;
-		pszStr ++;
+		++ pszStr;
 	}
 	return val;
 }

src/common/CPointBase.cpp

@@ -1,4 +1,4 @@
-#include "../common/sphere_library/container_ops.h"
+#include "../common/sphere_library/scontainer_ops.h"
 #include "../game/items/CItem.h"
 #include "../game/CSector.h"
 #include "../game/CServer.h"

src/common/crypto/CCrypto.cpp

@@ -20,7 +20,40 @@
 
 /*		Login keys from SphereCrypt.ini		*/
 
-std::vector<CCryptoClientKey> CCrypto::client_keys;
+CCryptoKeysHolder* CCryptoKeysHolder::get() noexcept
+{
+	static CCryptoKeysHolder instance;
+	return &instance;
+}
+
+void CCryptoKeysHolder::LoadKeyTable(CScript& s)
+{
+	ADDTOCALLSTACK("CCrypto::LoadKeyTable");
+	client_keys.clear();
+
+	// Always add nocrypt
+	addNoCryptKey();
+
+	while (s.ReadKeyParse())
+	{
+		CCryptoClientKey c;
+		c.m_client = ahextoi(s.GetKey());
+		c.m_key_1 = s.GetArgVal();
+		c.m_key_2 = s.GetArgVal();
+		c.m_EncType = (ENCRYPTION_TYPE)s.GetArgVal();
+		client_keys.emplace_back(std::move(c));
+	}
+}
+
+void CCryptoKeysHolder::addNoCryptKey(void)
+{
+	ADDTOCALLSTACK("CCrypto::addNoCryptKey");
+	CCryptoClientKey c{};
+	c.m_EncType = ENC_NONE;
+	client_keys.emplace_back(std::move(c));
+}
+
+// --
 
 void CCrypto::SetClientVersion( dword iVer )
 {
@@ -87,35 +120,6 @@ ENCRYPTION_TYPE CCrypto::GetEncryptionType() const
 	return m_GameEnc;
 }
 
-void CCrypto::LoadKeyTable(CScript & s)
-{
-	ADDTOCALLSTACK("CCrypto::LoadKeyTable");
-	client_keys.clear();
-
-	// Always add nocrypt
-	addNoCryptKey();
-
-	while ( s.ReadKeyParse() )
-	{
-		CCryptoClientKey c;
-		c.m_client = ahextoi( s.GetKey() );
-		c.m_key_1 = s.GetArgVal();
-		c.m_key_2 = s.GetArgVal();
-		c.m_EncType = (ENCRYPTION_TYPE)s.GetArgVal();
-		client_keys.emplace_back(c);
-	}
-}
-
-void CCrypto::addNoCryptKey(void)
-{
-	ADDTOCALLSTACK("CCrypto::addNoCryptKey");
-	CCryptoClientKey c;
-	c.m_client = 0;
-	c.m_key_1 = 0;
-	c.m_key_2 = 0;
-	c.m_EncType = ENC_NONE;
-	client_keys.emplace_back(c);
-}
 
 // ---------------------------------------------------------------------------------------------------------------
 // ===============================================================================================================
@@ -227,9 +231,10 @@ char* CCrypto::WriteClientVer( char * pcStr, uint uiBufLen) const
 bool CCrypto::SetClientVerEnum( dword iVer, bool bSetEncrypt )
 {
 	ADDTOCALLSTACK("CCrypto::SetClientVerEnum");
-	for (size_t i = 0; i < client_keys.size(); ++i )
+	CCryptoKeysHolder* keys_holder = CCryptoKeysHolder::get();
+	for (size_t i = 0; i < keys_holder->client_keys.size(); ++i )
 	{
-		CCryptoClientKey & key = client_keys[i];
+		CCryptoClientKey & key = keys_holder->client_keys[i];
 
 		if ( iVer == key.m_client )
 		{
@@ -244,10 +249,12 @@ bool CCrypto::SetClientVerEnum( dword iVer, bool bSetEncrypt )
 bool CCrypto::SetClientVerIndex( size_t iVer, bool bSetEncrypt )
 {
 	ADDTOCALLSTACK("CCrypto::SetClientVerIndex");
-	if ( iVer >= client_keys.size() )
+	CCryptoKeysHolder* keys_holder = CCryptoKeysHolder::get();
+
+	if ( iVer >= keys_holder->client_keys.size() )
 		return false;
 
-	CCryptoClientKey & key = client_keys[iVer];
+	CCryptoClientKey & key = keys_holder->client_keys[iVer];
 
 	SetClientVersion(key.m_client);
 	SetMasterKeys(key.m_key_1, key.m_key_2); // Hi - Lo
@@ -292,9 +299,11 @@ bool CCrypto::SetClientVer( lpctstr pszVersion )
 
 CCrypto::CCrypto()
 {
+	CCryptoKeysHolder* keys_holder = CCryptoKeysHolder::get();
+
 	// Always at least one crypt code, for non encrypted clients!
-	if ( ! client_keys.size() )
-		addNoCryptKey();
+	if ( !keys_holder->client_keys.size() )
+		keys_holder->addNoCryptKey();
 
 	m_fInit = false;
 	m_fRelayPacket = false;
@@ -605,7 +614,7 @@ bool CCrypto::LoginCryptStart( dword dwIP, const byte * pEvent, uint inLen )
     ASSERT(inLen <= MAX_BUFFER);
     std::unique_ptr<byte[]> pRaw = std::make_unique<byte[]>(MAX_BUFFER);
     memcpy(pRaw.get(), pEvent, inLen);
-	
+
 	m_seed = dwIP;
 	SetConnectType( CONNECT_LOGIN );
 
@@ -615,9 +624,11 @@ bool CCrypto::LoginCryptStart( dword dwIP, const byte * pEvent, uint inLen )
 	SetClientVerIndex(0);
 	SetCryptMask(tmp_CryptMaskHi, tmp_CryptMaskLo);
 
+	CCryptoKeysHolder* keys_holder = CCryptoKeysHolder::get();
+
 	for (uint i = 0, iAccountNameLen = 0;;)
 	{
-		if ( i >= client_keys.size() )
+		if ( i >= keys_holder->client_keys.size() )
 		{
 			// Unknown client !!! Set as unencrypted and let Sphere do the rest.
 #ifdef DEBUG_CRYPT_MSGS
@@ -746,9 +757,10 @@ bool CCrypto::GameCryptStart( dword dwIP, const byte * pEvent, uint inLen )
         const dword tmp_CryptMaskHi = ((( m_seed) ^ 0x43210000) >> 16) | (((~m_seed) ^ 0xabcdffff) & 0xffff0000);
         SetClientVerIndex(0);
 
+		CCryptoKeysHolder* keys_holder = CCryptoKeysHolder::get();
         for (size_t i = 0;;)
         {
-            if ( i >= client_keys.size() )
+            if ( i >= keys_holder->client_keys.size() )
             {
                 // Unknown client !!! Set as unencrypted and let Sphere do the rest.
 #ifdef DEBUG_CRYPT_MSGS
@@ -782,7 +794,7 @@ bool CCrypto::GameCryptStart( dword dwIP, const byte * pEvent, uint inLen )
                 if ( pRaw[34] == 0x00 && pRaw[64] == 0x00)
                 {
                     bOut = true;    // Ok the new detected encryption is ok (legit post-login packet: 0x91)
-                    SetCryptMask(tmp_CryptMaskHi, tmp_CryptMaskLo); 
+                    SetCryptMask(tmp_CryptMaskHi, tmp_CryptMaskLo);
                     break;
                 }
             }

src/common/crypto/CCrypto.h

@@ -81,15 +81,25 @@ struct CCryptoClientKey
 // ===============================================================================================================
 
 
-struct CCrypto
+struct CCryptoKeysHolder
 {
-public:
+	static CCryptoKeysHolder* get() noexcept;
+
 	union CCryptoKey		// For internal encryption calculations, it has nothing to do with SphereCrypt.ini
 	{
-		#define CRYPT_GAMESEED_LENGTH	8
+#define CRYPT_GAMESEED_LENGTH	8
 		byte  u_cKey[CRYPT_GAMESEED_LENGTH];
 		dword u_iKey[2];
 	};
+
+	std::vector<CCryptoClientKey> client_keys;
+
+	void LoadKeyTable(CScript& s);
+	void addNoCryptKey(void);
+};
+
+struct CCrypto
+{
 private:
 	bool m_fInit;
 	bool m_fRelayPacket;
@@ -109,12 +119,6 @@ struct CCrypto
 
 	//static const word packet_size[0xde];
 
-public:
-	static std::vector<CCryptoClientKey> client_keys;
-
-	static void LoadKeyTable(CScript & s);
-	static void addNoCryptKey(void);
-
 	// --------------- Generic -----------------------------------
 	//static int GetPacketSize(byte packet);
 	// --------------- EOF Generic -------------------------------
@@ -150,11 +154,11 @@ struct CCrypto
 	int	m_gameBlockPos;		// 0-7
 	size_t	m_gameStreamPos;	// use this to track the 21K move to the new Blowfish m_gameTable.
 private:
-	CCrypto::CCryptoKey m_Key;
+	CCryptoKeysHolder::CCryptoKey m_Key;
 private:
 	void InitSeed( int iTable );
 	static void InitTables();
-	static void PrepareKey( CCrypto::CCryptoKey & key, int iTable );
+	static void PrepareKey(CCryptoKeysHolder::CCryptoKey & key, int iTable );
 	bool DecryptBlowFish( byte * pOutput, const byte * pInput, size_t outLen, size_t inLen );
 	byte DecryptBFByte( byte bEnc );
 	void InitBlowFish();

src/common/crypto/CCryptoBlowFish.cpp

@@ -7,7 +7,7 @@ dword sm_dwCodingData[CRYPT_GAMEKEY_COUNT][18+1024];	// to be filled by InitTabl
 bool CCrypto::sm_fTablesReady = false;
 
 
-void CCrypto::PrepareKey( CCrypto::CCryptoKey & key, int iTable )	// static
+void CCrypto::PrepareKey(CCryptoKeysHolder::CCryptoKey & key, int iTable )	// static
 {
 	ADDTOCALLSTACK("CCrypto::PrepareKey");
 	const dword *pCodes = sm_dwCodingData[iTable];
@@ -42,7 +42,7 @@ void CCrypto::InitTables()		// static
 			sm_dwCodingData[i][j] ^= code[j%3];
 		}
 
-		CCrypto::CCryptoKey tmpKey;
+		CCryptoKeysHolder::CCryptoKey tmpKey;
 		tmpKey.u_iKey[0] = tmpKey.u_iKey[1] = 0;
 
 		for(j=0; j<0x412; j+=2)

src/common/resource/CResourceBase.cpp

@@ -383,24 +383,38 @@ int CResourceBase::ResourceGetIndexType( RES_TYPE restype, lpctstr pszName, word
 	return rid.GetResIndex();
 }
 
-CResourceDef * CResourceBase::ResourceGetDef(const CResourceID& rid) const
+std::weak_ptr<CResourceDef> CResourceBase::ResourceGetDefRef(const CResourceID& rid) const
 {
-	ADDTOCALLSTACK("CResourceBase::ResourceGetDef");
+	ADDTOCALLSTACK("CResourceBase::ResourceGetDefRef");
 	if ( ! rid.IsValidResource() )
-		return nullptr;
+		return {};
 	size_t index = m_ResHash.FindKey( rid );
 	if ( index == SCONT_BADINDEX )
-		return nullptr;
-	return m_ResHash.GetAt( rid, index );
+		return {};
+	return m_ResHash.GetWeakPtrAt( rid, index ).lock();
 }
 
-CScriptObj * CResourceBase::ResourceGetDefByName( RES_TYPE restype, lpctstr pszName, word wPage )
+std::weak_ptr<CResourceDef> CResourceBase::ResourceGetDefRefByName( RES_TYPE restype, lpctstr pszName, word wPage )
 {
-    ADDTOCALLSTACK("CResourceBase::ResourceGetDefByName");
+    ADDTOCALLSTACK("CResourceBase::ResourceGetDefRefByName");
     // resolve a name to the actual resource def.
-    CResourceID res = ResourceGetID(restype, pszName);
+    CResourceID res = ResourceGetID(restype, pszName, wPage);
     res.m_wPage = wPage ? wPage : RES_PAGE_ANY;   // Create a CResourceID with page == RES_PAGE_ANY: search independently from the page
-    return ResourceGetDef(res);
+    return ResourceGetDefRef(res);
+}
+
+CResourceDef* CResourceBase::ResourceGetDef(const CResourceID& rid) const
+{
+	ADDTOCALLSTACK("CResourceBase::ResourceGetDef");
+	std::shared_ptr<CResourceDef> ret = ResourceGetDefRef(rid).lock();
+	return ret ? ret.get() : nullptr;
+}
+
+CResourceDef* CResourceBase::ResourceGetDefByName(RES_TYPE restype, lpctstr pszName, word wPage)
+{
+	ADDTOCALLSTACK("CResourceBase::ResourceGetDefByName");
+	std::shared_ptr<CResourceDef> ret = ResourceGetDefRefByName(restype, pszName, wPage).lock();
+	return ret ? ret.get() : nullptr;
 }
 
 //*******************************************************
@@ -412,8 +426,13 @@ bool CResourceBase::ResourceLock( CResourceLock & s, const CResourceID& rid )
 	// Lock a referenced resource object.
 	if ( ! rid.IsValidUID() )
 		return false;
-	CResourceLink * pResourceLink = dynamic_cast <CResourceLink *>( ResourceGetDef( rid ));
-	if ( pResourceLink )
-		return pResourceLink->ResourceLock(s);
+	std::shared_ptr<CResourceDef> pResourceDefRef = ResourceGetDefRef(rid).lock();
+	if (pResourceDefRef)
+	{
+		CResourceLink* pResourceLink = dynamic_cast <CResourceLink*>(pResourceDefRef.get());
+		if (pResourceLink)
+			return pResourceLink->ResourceLock(s);
+	}
+
 	return false;
 }

src/common/resource/CResourceBase.h

@@ -44,7 +44,6 @@ class CResourceBase : public CScriptObj
 	CResourceID ResourceGetID( RES_TYPE restype, lpctstr ptcName, word wPage = 0 );
 	CResourceID ResourceGetIDType( RES_TYPE restype, lpctstr pszName, word wPage = 0 );
 	int ResourceGetIndexType( RES_TYPE restype, lpctstr pszName, word wPage = 0 );
-	CScriptObj * ResourceGetDefByName( RES_TYPE restype, lpctstr pszName, word wPage = 0 );
 	bool ResourceLock( CResourceLock & s, const CResourceID& rid );
 	bool ResourceLock( CResourceLock & s, RES_TYPE restype, lpctstr pszName )
 	{
@@ -54,7 +53,10 @@ class CResourceBase : public CScriptObj
 	CResourceScript * FindResourceFile( lpctstr pszTitle );
 	CResourceScript * LoadResourcesAdd( lpctstr pszNewName );
 
-	virtual CResourceDef * ResourceGetDef( const CResourceID& rid ) const;
+	std::weak_ptr<CResourceDef> ResourceGetDefRef(const CResourceID& rid) const;
+	CResourceDef * ResourceGetDef( const CResourceID& rid ) const;
+	std::weak_ptr<CResourceDef> ResourceGetDefRefByName(RES_TYPE restype, lpctstr pszName, word wPage = 0);
+	CResourceDef* ResourceGetDefByName(RES_TYPE restype, lpctstr pszName, word wPage = 0);
 	virtual bool OpenResourceFind( CScript &s, lpctstr pszFilename, bool fCritical = true );
 	virtual bool LoadResourceSection( CScript * pScript ) = 0;