From caee6c578dededa7ef08305549cc0b42f3d3a444 Mon Sep 17 00:00:00 2001
From: Franklin Wei <franklin@rockbox.org>
Date: Mon, 29 Jul 2019 20:59:11 -0400
Subject: [PATCH] quake: fix race condition

COM_LoadStackFile was not thread-safe since it relied on a global variable
to pass the loadbuf parameter to COM_LoadFile. This was causing mysterious
crashes when model loading and audio mixing were happening simultaneously.

Change-Id: I505c5ef0ed49d0c4aa4b11cfed05647c75b5b40d
---
 apps/plugins/sdl/progs/quake/common.c | 41 ++++++++++++++++++++++-----
 apps/plugins/sdl/progs/quake/model.c  |  3 ++
 2 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/apps/plugins/sdl/progs/quake/common.c b/apps/plugins/sdl/progs/quake/common.c
index 881c01e22e..2d7985496a 100644
--- a/apps/plugins/sdl/progs/quake/common.c
+++ b/apps/plugins/sdl/progs/quake/common.c
@@ -1614,6 +1614,9 @@ byte *COM_LoadFile (char *path, int usehunk)
 	if (h == -1)
 		return NULL;
 
+        if(len < 1)
+            rb->splashf(HZ, "suspicious length %d", len);
+
         check_ptr = &h;
         
         //printf("handle %d", h);
@@ -1635,9 +1638,15 @@ byte *COM_LoadFile (char *path, int usehunk)
 	else if (usehunk == 4)
 	{
 		if (len+1 > loadsize)
+                {
+                    LOGF("LoadFile: allocating hunk b/c stackbuf too small (filelen=%d)\n", len);
 			buf = Hunk_TempAlloc (len+1);
+                }
 		else
+                {
+                    LOGF("filelen=%d, using stack buf\n", len);
 			buf = loadbuf;
+                }
 	}
 	else
 		Sys_Error ("COM_LoadFile: bad usehunk");
@@ -1671,16 +1680,34 @@ void COM_LoadCacheFile (char *path, struct cache_user_s *cu)
 	COM_LoadFile (path, 3);
 }
 
+/*
+ * This function was NOT originally thread-safe, leading to a race
+ * condition between the Mod_LoadModel and S_LoadSound (which run in
+ * different threads). Fixed with mutex lock. - FW 7/29/19
+ */
+
 // uses temp hunk if larger than bufsize
 byte *COM_LoadStackFile (char *path, void *buffer, int bufsize)
 {
-	byte    *buf;
-	
-	loadbuf = (byte *)buffer;
-	loadsize = bufsize;
-	buf = COM_LoadFile (path, 4);
-	
-	return buf;
+    static struct mutex m;
+    static int init = 0;
+    if(!init)
+    {
+        rb->mutex_init(&m);
+        init = 1;
+    }
+
+    rb->mutex_lock(&m);
+
+    byte    *buf;
+
+    loadbuf = (byte *)buffer;
+    loadsize = bufsize;
+    buf = COM_LoadFile (path, 4);
+
+    rb->mutex_unlock(&m);
+
+    return buf;
 }
 
 /*
diff --git a/apps/plugins/sdl/progs/quake/model.c b/apps/plugins/sdl/progs/quake/model.c
index 5ac6dc6cb2..7648590db4 100644
--- a/apps/plugins/sdl/progs/quake/model.c
+++ b/apps/plugins/sdl/progs/quake/model.c
@@ -1472,8 +1472,11 @@ void Mod_LoadAliasModel (model_t *mod, void *buffer)
 
 	version = LittleLongUnaligned (pinmodel->version);
 	if (version != ALIAS_VERSION)
+        {
+            rb->splashf(HZ*2, "Likely race condition! S_LoadSound and this use the same allocator!");
 		Sys_Error ("%s has wrong version number (%i should be %i)",
 				 mod->name, version, ALIAS_VERSION);
+        }
 
 //
 // allocate space for a working header, plus all the data except the frames,