Modernize: we can use std::mutex again~

And hey, found a reentrancy bug that the AI made writing ARC support (huge surprise...)

src/imagefs.cpp

@@ -4,6 +4,7 @@
 #include <map>
 #include <fstream>
 #include <memory>
+#include <mutex>
 #include <sstream>
 
 #include "3rd_party/lodepng.h"
@@ -17,7 +18,6 @@
 #include "modpath_handler.h"
 #include "texture_packer.h"
 #include "utils.hpp"
-#include "winxp_mutex.hpp"
 
 using std::string;
 
@@ -51,10 +51,10 @@ typedef struct afp {
 
 // ifs_textures["data/graphics/ver04/logo.ifs/tex/4f754d4f424f092637a49a5527ece9bb"] will be "konami"
 static std::map<string, std::shared_ptr<image_t>, CaseInsensitiveCompare> ifs_textures;
-static CriticalSectionLock ifs_textures_mtx;
+static std::mutex ifs_textures_mtx;
 
 static std::map<std::string, std::shared_ptr<afp_t>, CaseInsensitiveCompare> afp_md5_names;
-static CriticalSectionLock afp_md5_names_mtx;
+static std::mutex afp_md5_names_mtx;
 
 
 void rapidxml_dump_to_file(const string& out, const rapidxml::xml_document<> &xml) {
@@ -182,9 +182,8 @@ bool add_images_to_list(string_set &extra_pngs, rapidxml::xml_node<> *texturelis
             image_info.height = texture->height;
 
             auto md5_path = ifs_path + "/tex/" + image_info.name_md5;
-            ifs_textures_mtx.lock();
+            std::lock_guard lock(ifs_textures_mtx);
             ifs_textures[md5_path] = std::make_shared<image_t>(std::move(image_info));
-            ifs_textures_mtx.unlock();
         }
     }
 
@@ -293,9 +292,8 @@ void parse_texturelist(HookFile &file) {
             extra_pngs.erase(image_info.name);
 
             auto md5_path = ifs_path + "/tex/" + image_info.name_md5;
-            ifs_textures_mtx.lock();
+            std::lock_guard lock(ifs_textures_mtx);
             ifs_textures[md5_path] = std::make_shared<image_t>(std::move(image_info));
-            ifs_textures_mtx.unlock();
         }
     }
 
@@ -474,7 +472,7 @@ void parse_afplist(HookFile &file) {
             // log_info("AFP %s -> %s", md5_path.c_str(), (ifs_mod_path + folder + file).c_str());
         };
 
-        afp_md5_names_mtx.lock();
+        std::lock_guard lock(afp_md5_names_mtx);
 
         add_mapping("/afp/", name->value());
         add_mapping("/afp/bsi/", name->value());
@@ -485,24 +483,21 @@ void parse_afplist(HookFile &file) {
         while(ss >> index) {
             add_mapping("/geo/", std::string(name->value()) + "_shape" + index);
         }
-
-        afp_md5_names_mtx.unlock();
     }
 
     log_verbose("Mapped %d AFP filenames", mapped);
 }
 
 std::optional<std::tuple<std::string, std::shared_ptr<image_t>>> lookup_png_from_md5(HookFile &file) {
-    ifs_textures_mtx.lock();
+    std::unique_lock lock(ifs_textures_mtx);
     auto tex_search = ifs_textures.find(file.norm_path);
     if (tex_search == ifs_textures.end()) {
-        ifs_textures_mtx.unlock();
         return std::nullopt;
     }
 
     //log_misc("Mapped file %s is found!", norm_path.c_str());
     auto tex = tex_search->second;
-    ifs_textures_mtx.unlock(); // is it safe to unlock this early? Time will tell...
+    lock.unlock(); // is it safe to unlock this early? Time will tell...
 
     // remove the /tex/, it's nicer to navigate
     auto png_path = find_first_modfile(tex->ifs_mod_path + "/" + tex->name + ".png");
@@ -540,16 +535,15 @@ void handle_texture(HookFile &file) {
 }
 
 std::optional<std::string> lookup_afp_from_md5(HookFile &file) {
-    afp_md5_names_mtx.lock();
+    std::unique_lock lock(afp_md5_names_mtx);
     auto afp_search = afp_md5_names.find(file.norm_path);
     if (afp_search == afp_md5_names.end()) {
-        afp_md5_names_mtx.unlock();
         return std::nullopt;
     }
 
     //log_misc("Mapped file %s is found!", norm_path.c_str());
     auto afp = afp_search->second;
-    afp_md5_names_mtx.unlock(); // is it safe to unlock this early? Time will tell...
+    lock.unlock(); // is it safe to unlock this early? Time will tell...
 
     return find_first_modfile(afp->mod_path);
 }

src/log.cpp

@@ -1,8 +1,8 @@
+#include <mutex>
 #include <stdio.h>
 
 #include "config.hpp"
 #include "log.hpp"
-#include "winxp_mutex.hpp"
 
 #define SUPPRESS_PRINTF
 
@@ -13,7 +13,7 @@ void stdout_log(char level, const char *fmt, va_list args) {
 }
 
 static void log_to_file(char level, const char* fmt, va_list args) {
-    static CriticalSectionLock log_mutex;
+    static std::mutex log_mutex;
     static FILE* logfile = NULL;
     static bool tried_to_open = false;
 #ifndef SUPPRESS_PRINTF
@@ -21,7 +21,7 @@ static void log_to_file(char level, const char* fmt, va_list args) {
 #endif
     // don't reopen every time: slow as shit
     if (!tried_to_open) {
-        log_mutex.lock();
+        std::lock_guard lock(log_mutex);
 
         if (!logfile) {
             // default to ifs_hook.log because we need *something* in the case
@@ -30,10 +30,9 @@ static void log_to_file(char level, const char* fmt, va_list args) {
             logfile = fopen(path, "w");
         }
         tried_to_open = true;
-        log_mutex.unlock();
     }
     if (logfile) {
-        log_mutex.lock();
+        std::lock_guard lock(log_mutex);
 
         fprintf(logfile, "%c:", level);
         vfprintf(logfile, fmt, args);
@@ -41,8 +40,6 @@ static void log_to_file(char level, const char* fmt, va_list args) {
 
         if(config.developer_mode || level == 'F')
             fflush(logfile);
-
-        log_mutex.unlock();
     }
 }
 

src/modpath_handler.cpp

@@ -9,7 +9,6 @@
 #include "log.hpp"
 #include "utils.hpp"
 #include "avs.h"
-#include "winxp_mutex.hpp"
 
 using std::nullopt;
 
@@ -98,9 +97,10 @@ void modpath_debug_add_folder(const string &folder) {
     game_folders.push_back(folder + "/");
 }
 
-optional<string> normalise_path(const string &_path) {
+optional<string> normalise_path(const string &_path, bool demangle) {
     auto path = _path;
-    ramfs_demangler_demangle_if_possible(path);
+    if (demangle)
+        ramfs_demangler_demangle_if_possible(path);
 
     auto data_pos = string_find_icase(path, "data/");
     auto other_pos = string::npos;

src/modpath_handler.h

@@ -19,7 +19,7 @@ void modpath_debug_add_folder(const string &folder);
 void cache_mods(void);
 vector<string> available_mods();
 // mutates source string to be all lowercase
-optional<string> normalise_path(const string &path);
+optional<string> normalise_path(const string &path, bool demangle = true);
 optional<string> find_first_modfile(const string &norm_path);
 optional<string> find_first_modfolder(const string &norm_path);
 vector<string> find_all_modfile(const string &norm_path);

src/ramfs_demangler.cpp

@@ -33,6 +33,7 @@
 #include <algorithm>
 #include <cstring>
 #include <map>
+#include <mutex>
 #include <unordered_map>
 #include <optional>
 
@@ -42,7 +43,6 @@
 #include "log.hpp"
 #include "modpath_handler.h"
 #include "utils.hpp"
-#include "winxp_mutex.hpp"
 
 using namespace std;
 
@@ -63,7 +63,7 @@ static tsl::htrie_map<char, string> mangling_map;
 // Basename ("foo.ifs") -> demangled inner path ("arc/.../foo_arc/.../foo.ifs")
 static unordered_map<string, string> arc_inner_by_basename;
 
-static CriticalSectionLock mangling_mtx;
+static std::mutex mangling_mtx;
 
 // since we call this from a function that is already taking the lock
 static void ramfs_demangler_demangle_if_possible_nolock(std::string& raw_path);
@@ -75,7 +75,7 @@ void ramfs_demangler_on_fs_open(const std::string& path, AVS_FILE open_result) {
 		return;
 	}
 
-	mangling_mtx.lock();
+	std::lock_guard lock(mangling_mtx);
 
 	auto existing_info = cleanup_map.find(path);
 	if (existing_info != cleanup_map.end()) {
@@ -104,12 +104,10 @@ void ramfs_demangler_on_fs_open(const std::string& path, AVS_FILE open_result) {
 	};
 	cleanup_map[path] = cleanup;
 	open_file_map[open_result] = path;
-
-	mangling_mtx.unlock();
 }
 
 void ramfs_demangler_register_arc_inner_ifs(const std::string& basename, const std::string& demangled_path) {
-	mangling_mtx.lock();
+	std::lock_guard lock(mangling_mtx);
 	auto existing = arc_inner_by_basename.find(basename);
 	if (existing != arc_inner_by_basename.end() && existing->second != demangled_path) {
 		log_warning("arc demangle: basename collision for '%s' (%s vs %s), later one wins",
@@ -117,11 +115,10 @@ void ramfs_demangler_register_arc_inner_ifs(const std::string& basename, const s
 	}
 	arc_inner_by_basename[basename] = demangled_path;
 	log_verbose("arc inner basename '%s' -> %s", basename.c_str(), demangled_path.c_str());
-	mangling_mtx.unlock();
 }
 
 void ramfs_demangler_on_fs_read(AVS_FILE context, void* dest) {
-	mangling_mtx.lock();
+	std::lock_guard lock(mangling_mtx);
 
 	auto find = open_file_map.find(context);
 	if (find != open_file_map.end()) {
@@ -135,25 +132,21 @@ void ramfs_demangler_on_fs_read(AVS_FILE context, void* dest) {
 			cleanup->second.buffer = dest;
 		}
 	}
-
-	mangling_mtx.unlock();
 }
 
 void ramfs_demangler_on_fs_mount(const char* mountpoint, const char* fsroot, const char* fstype, const char* flags) {
-	mangling_mtx.lock();
+	std::lock_guard lock(mangling_mtx);
 
 	if (!strcmp(fstype, "ramfs")) {
 		void* buffer;
 
 		if (!flags) {
 			log_verbose("ramfs has no flags?");
-			mangling_mtx.unlock();
 			return;
 		}
 		const char* baseptr = strstr(flags, "base=");
 		if (!baseptr) {
 			log_verbose("ramfs has no base pointer?");
-			mangling_mtx.unlock();
 			return;
 		}
 
@@ -234,26 +227,22 @@ void ramfs_demangler_on_fs_mount(const char* mountpoint, const char* fsroot, con
 			// the result is something normalise_path can use.
 			string root = (string)fsroot;
 			ramfs_demangler_demangle_if_possible_nolock(root);
-			if (normalise_path(root)) {
+			if (normalise_path(root, /* demangle */ false)) {
 				log_verbose("imagefs mount mapped to %s", root.c_str());
 				mangling_map[mountpoint] = root;
 			}
 		}
 	}
-
-	mangling_mtx.unlock();
 }
 
 void ramfs_demangler_demangle_if_possible(std::string& raw_path) {
-	mangling_mtx.lock();
+	std::lock_guard lock(mangling_mtx);
 
 	auto search = mangling_map.longest_prefix(raw_path);
 	if (search != mangling_map.end()) {
 		// log_verbose("can demangle %s to %s", search.key().c_str(), search->c_str());
 		string_replace(raw_path, search.key().c_str(), search->c_str());
 	}
-
-	mangling_mtx.unlock();
 }
 
 static void ramfs_demangler_demangle_if_possible_nolock(std::string& raw_path) {

src/winxp_mutex.hpp

@@ -1,58 +0,0 @@
-#pragma once
-
-#include <windows.h>
-
-// This class is a lightweight replacement for std::mutex on Windows platforms.
-// std::mutex does not work on Windows XP SP2 with the latest VC++ libraries,
-// because it utilizes the Concurrency Runtime that is only supported on Windows
-// XP SP3 and above.
-
-// mon addition: avoid std::lock_guard. It uses thread local storage and is just, in general, pain to compile properly.
-// This sucks, because RAII is awesome.
-
-class CriticalSectionLock {
-public:
-	CriticalSectionLock() { InitializeCriticalSection(&critical_section_); }
-	~CriticalSectionLock() { DeleteCriticalSection(&critical_section_); }
-	void lock() { EnterCriticalSection(&critical_section_); }
-	void unlock() { LeaveCriticalSection(&critical_section_); }
-
-private:
-	CRITICAL_SECTION critical_section_;
-};
-
-/**
-Copyright 2008 Google Inc.  All rights reserved.
-
-Redistribution and use in source and binary forms, with or without
-modification, are permitted provided that the following conditions are
-met:
-
-	* Redistributions of source code must retain the above copyright
-notice, this list of conditions and the following disclaimer.
-	* Redistributions in binary form must reproduce the above
-copyright notice, this list of conditions and the following disclaimer
-in the documentation and/or other materials provided with the
-distribution.
-	* Neither the name of Google Inc. nor the names of its
-contributors may be used to endorse or promote products derived from
-this software without specific prior written permission.
-
-THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-Code generated by the Protocol Buffer compiler is owned by the owner
-of the input file used when generating it.  This code is not
-standalone and requires a support library to be linked with it.  This
-support library is itself covered by the above license.
-
-**/