🐞 Synchronize SFileReadFile access

StormLib read function is not thread-safe: https://github.com/ladislav-zezula/StormLib/issues/175

Source/DiabloUI/art.cpp

@@ -19,7 +19,7 @@ constexpr unsigned PcxPaletteSize = 1 + NumPaletteColors * 3;
 bool LoadPcxMeta(HANDLE handle, int &width, int &height, std::uint8_t &bpp)
 {
 	PCXHeader pcxhdr;
-	if (!SFileReadFile(handle, &pcxhdr, PcxHeaderSize, nullptr, nullptr)) {
+	if (!SFileReadFileThreadSafe(handle, &pcxhdr, PcxHeaderSize)) {
 		return false;
 	}
 	width = SDL_SwapLE16(pcxhdr.Xmax) - SDL_SwapLE16(pcxhdr.Xmin) + 1;
@@ -41,7 +41,7 @@ bool LoadPcxPixelsAndPalette(HANDLE handle, int width, int height, std::uint8_t
 	// We read 1 extra byte because it delimits the palette.
 	const unsigned readSize = pixelDataSize + (has256ColorPalette ? PcxPaletteSize : 0);
 	std::unique_ptr<BYTE[]> fileBuffer { new BYTE[readSize] };
-	if (!SFileReadFile(handle, fileBuffer.get(), readSize, nullptr, nullptr)) {
+	if (!SFileReadFileThreadSafe(handle, fileBuffer.get(), readSize)) {
 		return false;
 	}
 	const unsigned xSkip = bufferPitch - width;

Source/engine.cpp

@@ -730,7 +730,7 @@ std::unique_ptr<BYTE[]> LoadFileInMem(const char *pszName, DWORD *pdwFileLen)
 
 	std::unique_ptr<BYTE[]> buf { new BYTE[fileLen] };
 
-	SFileReadFile(file, buf.get(), fileLen, nullptr, nullptr);
+	SFileReadFileThreadSafe(file, buf.get(), fileLen);
 	SFileCloseFile(file);
 
 	return buf;
@@ -757,7 +757,7 @@ DWORD LoadFileWithMem(const char *pszName, BYTE *p)
 		app_fatal("Zero length SFILE:\n%s", pszName);
 	}
 
-	SFileReadFile(hsFile, p, dwFileLen, nullptr, nullptr);
+	SFileReadFileThreadSafe(hsFile, p, dwFileLen);
 	SFileCloseFile(hsFile);
 
 	return dwFileLen;

Source/palette.cpp

@@ -127,7 +127,7 @@ void LoadPalette(const char *pszFileName)
 	assert(pszFileName);
 
 	SFileOpenFile(pszFileName, &pBuf);
-	SFileReadFile(pBuf, (char *)PalData, sizeof(PalData), nullptr, nullptr);
+	SFileReadFileThreadSafe(pBuf, (char *)PalData, sizeof(PalData));
 	SFileCloseFile(pBuf);
 
 	for (i = 0; i < 256; i++) {

Source/pfile.cpp

@@ -145,7 +145,7 @@ static std::unique_ptr<uint8_t[]> pfile_read_archive(HANDLE archive, const char
 		return nullptr;
 
 	std::unique_ptr<uint8_t[]> buf { new uint8_t[*pdwLen] };
-	if (!SFileReadFile(file, buf.get(), *pdwLen, &nread, nullptr))
+	if (!SFileReadFileThreadSafe(file, buf.get(), *pdwLen, &nread))
 		return nullptr;
 	SFileCloseFile(file);
 

Source/sound.cpp

@@ -47,7 +47,7 @@ void LoadMusic(HANDLE handle)
 #else
 	int bytestoread = SFileGetFileSize(handle, 0);
 	musicBuffer = new char[bytestoread];
-	SFileReadFile(handle, musicBuffer, bytestoread, NULL, 0);
+	SFileReadFileThreadSafe(handle, musicBuffer, bytestoread);
 	SFileCloseFile(handle);
 
 	SDL_RWops *musicRw = SDL_RWFromConstMem(musicBuffer, bytestoread);
@@ -173,7 +173,7 @@ std::unique_ptr<TSnd> sound_file_load(const char *path, bool stream)
 		}
 		DWORD dwBytes = SFileGetFileSize(file, nullptr);
 		auto wave_file = MakeArraySharedPtr<std::uint8_t>(dwBytes);
-		SFileReadFile(file, wave_file.get(), dwBytes, nullptr, nullptr);
+		SFileReadFileThreadSafe(file, wave_file.get(), dwBytes);
 		error = snd->DSB.SetChunk(wave_file, dwBytes);
 		SFileCloseFile(file);
 	}

Source/storm/storm.cpp

@@ -13,6 +13,7 @@
 #include "utils/paths.h"
 #include "utils/stubs.h"
 #include "utils/log.hpp"
+#include "utils/sdl_mutex.h"
 
 // Include Windows headers for Get/SetLastError.
 #if defined(_WIN32)
@@ -34,6 +35,13 @@ std::string *SBasePath = nullptr;
 
 } // namespace
 
+bool SFileReadFileThreadSafe(HANDLE hFile, void *buffer, DWORD nNumberOfBytesToRead, DWORD *read, int *lpDistanceToMoveHigh)
+{
+	static SDL_mutex *Mutex = SDL_CreateMutex();
+	SDLMutexLockGuard lock(Mutex);
+	return SFileReadFile(hFile, buffer, nNumberOfBytesToRead, read, lpDistanceToMoveHigh);
+}
+
 radon::File &getIni()
 {
 	static radon::File ini(paths::ConfigPath() + "diablo.ini");

Source/storm/storm.h

@@ -321,6 +321,10 @@ bool SFileEnableDirectAccess(bool enable);
 
 // Additions to Storm API:
 
+// Locks ReadFile under a mutex.
+// See https://github.com/ladislav-zezula/StormLib/issues/175
+bool SFileReadFileThreadSafe(HANDLE hFile, void *buffer, DWORD nNumberOfBytesToRead, DWORD *read = nullptr, int *lpDistanceToMoveHigh = nullptr);
+
 // Sets the file's 64-bit seek position.
 inline std::uint64_t SFileSetFilePointer(HANDLE hFile, std::int64_t offset, int whence)
 {

Source/storm/storm_file_wrapper.cpp

@@ -13,7 +13,7 @@ extern "C" {
 ssize_t SFileCookieRead(void *cookie, char *buf, size_t nbytes)
 {
 	DWORD numRead = 0;
-	if (!SFileReadFile(static_cast<HANDLE>(cookie), buf, nbytes, &numRead, nullptr)) {
+	if (!SFileReadFileThreadSafe(static_cast<HANDLE>(cookie), buf, nbytes, &numRead)) {
 		const DWORD errCode = SErrGetLastError();
 		if (errCode != STORM_ERROR_HANDLE_EOF) {
 			Log("SFileRwRead error: {} ERROR CODE {}", (unsigned int)nbytes, (unsigned int)errCode);

Source/storm/storm_sdl_rw.cpp

@@ -60,7 +60,7 @@ static int SFileRwRead(struct SDL_RWops *context, void *ptr, int size, int maxnu
 #endif
 {
 	DWORD numRead = 0;
-	if (!SFileReadFile(SFileRwGetHandle(context), ptr, maxnum * size, &numRead, nullptr)) {
+	if (!SFileReadFileThreadSafe(SFileRwGetHandle(context), ptr, maxnum * size, &numRead)) {
 		const DWORD errCode = SErrGetLastError();
 		if (errCode != STORM_ERROR_HANDLE_EOF) {
 			Log("SFileRwRead error: {} {} ERROR CODE {}", (unsigned int)size, (unsigned int)maxnum, (unsigned int)errCode);

Source/storm/storm_svid.cpp

@@ -157,7 +157,7 @@ bool SVidPlayBegin(const char *filename, int flags, HANDLE *video)
 #else
 	int bytestoread = SFileGetFileSize(*video, nullptr);
 	SVidBuffer = std::unique_ptr<uint8_t[]> { new uint8_t[bytestoread] };
-	SFileReadFile(*video, SVidBuffer.get(), bytestoread, nullptr, nullptr);
+	SFileReadFileThreadSafe(*video, SVidBuffer.get(), bytestoread);
 	SFileCloseFile(*video);
 	*video = nullptr;
 	SVidSMK = smk_open_memory(SVidBuffer.get(), bytestoread);