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
diff --git a/apps/plugins/sdl/progs/quake/common.c b/apps/plugins/sdl/progs/quake/common.c
index 881c01e..2d79854 100644
--- a/apps/plugins/sdl/progs/quake/common.c
+++ b/apps/plugins/sdl/progs/quake/common.c
@@ -1614,6 +1614,9 @@
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 @@
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 @@
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 5ac6dc6..7648590 100644
--- a/apps/plugins/sdl/progs/quake/model.c
+++ b/apps/plugins/sdl/progs/quake/model.c
@@ -1472,8 +1472,11 @@
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,