diff options
author | Franklin Wei <franklin@rockbox.org> | 2019-07-29 20:59:11 -0400 |
---|---|---|
committer | Franklin Wei <franklin@rockbox.org> | 2019-07-30 03:44:09 +0200 |
commit | caee6c578dededa7ef08305549cc0b42f3d3a444 (patch) | |
tree | 80a6eb7aca4fab5999b1604ef4bfa17104f5e9b0 /apps | |
parent | 0b23348610e7139f225d4bf5230f2604bd5db146 (diff) |
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
Diffstat (limited to 'apps')
-rw-r--r-- | apps/plugins/sdl/progs/quake/common.c | 41 | ||||
-rw-r--r-- | 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, |