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
This commit is contained in:
Franklin Wei 2019-07-29 20:59:11 -04:00
parent 0b23348610
commit caee6c578d
2 changed files with 37 additions and 7 deletions

View file

@ -1614,6 +1614,9 @@ byte *COM_LoadFile (char *path, int usehunk)
if (h == -1) if (h == -1)
return NULL; return NULL;
if(len < 1)
rb->splashf(HZ, "suspicious length %d", len);
check_ptr = &h; check_ptr = &h;
//printf("handle %d", h); //printf("handle %d", h);
@ -1635,9 +1638,15 @@ byte *COM_LoadFile (char *path, int usehunk)
else if (usehunk == 4) else if (usehunk == 4)
{ {
if (len+1 > loadsize) if (len+1 > loadsize)
{
LOGF("LoadFile: allocating hunk b/c stackbuf too small (filelen=%d)\n", len);
buf = Hunk_TempAlloc (len+1); buf = Hunk_TempAlloc (len+1);
}
else else
{
LOGF("filelen=%d, using stack buf\n", len);
buf = loadbuf; buf = loadbuf;
}
} }
else else
Sys_Error ("COM_LoadFile: bad usehunk"); Sys_Error ("COM_LoadFile: bad usehunk");
@ -1671,16 +1680,34 @@ void COM_LoadCacheFile (char *path, struct cache_user_s *cu)
COM_LoadFile (path, 3); 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 // uses temp hunk if larger than bufsize
byte *COM_LoadStackFile (char *path, void *buffer, int bufsize) byte *COM_LoadStackFile (char *path, void *buffer, int bufsize)
{ {
byte *buf; static struct mutex m;
static int init = 0;
loadbuf = (byte *)buffer; if(!init)
loadsize = bufsize; {
buf = COM_LoadFile (path, 4); rb->mutex_init(&m);
init = 1;
return buf; }
rb->mutex_lock(&m);
byte *buf;
loadbuf = (byte *)buffer;
loadsize = bufsize;
buf = COM_LoadFile (path, 4);
rb->mutex_unlock(&m);
return buf;
} }
/* /*

View file

@ -1472,8 +1472,11 @@ void Mod_LoadAliasModel (model_t *mod, void *buffer)
version = LittleLongUnaligned (pinmodel->version); version = LittleLongUnaligned (pinmodel->version);
if (version != ALIAS_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)", Sys_Error ("%s has wrong version number (%i should be %i)",
mod->name, version, ALIAS_VERSION); mod->name, version, ALIAS_VERSION);
}
// //
// allocate space for a working header, plus all the data except the frames, // allocate space for a working header, plus all the data except the frames,