summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFranklin Wei <franklin@rockbox.org>2019-07-29 20:59:11 -0400
committerFranklin Wei <franklin@rockbox.org>2019-07-30 03:44:09 +0200
commitcaee6c578dededa7ef08305549cc0b42f3d3a444 (patch)
tree80a6eb7aca4fab5999b1604ef4bfa17104f5e9b0
parent0b23348610e7139f225d4bf5230f2604bd5db146 (diff)
downloadrockbox-caee6c578dededa7ef08305549cc0b42f3d3a444.tar.gz
rockbox-caee6c578dededa7ef08305549cc0b42f3d3a444.zip
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
-rw-r--r--apps/plugins/sdl/progs/quake/common.c41
-rw-r--r--apps/plugins/sdl/progs/quake/model.c3
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)
1614 if (h == -1) 1614 if (h == -1)
1615 return NULL; 1615 return NULL;
1616 1616
1617 if(len < 1)
1618 rb->splashf(HZ, "suspicious length %d", len);
1619
1617 check_ptr = &h; 1620 check_ptr = &h;
1618 1621
1619 //printf("handle %d", h); 1622 //printf("handle %d", h);
@@ -1635,9 +1638,15 @@ byte *COM_LoadFile (char *path, int usehunk)
1635 else if (usehunk == 4) 1638 else if (usehunk == 4)
1636 { 1639 {
1637 if (len+1 > loadsize) 1640 if (len+1 > loadsize)
1641 {
1642 LOGF("LoadFile: allocating hunk b/c stackbuf too small (filelen=%d)\n", len);
1638 buf = Hunk_TempAlloc (len+1); 1643 buf = Hunk_TempAlloc (len+1);
1644 }
1639 else 1645 else
1646 {
1647 LOGF("filelen=%d, using stack buf\n", len);
1640 buf = loadbuf; 1648 buf = loadbuf;
1649 }
1641 } 1650 }
1642 else 1651 else
1643 Sys_Error ("COM_LoadFile: bad usehunk"); 1652 Sys_Error ("COM_LoadFile: bad usehunk");
@@ -1671,16 +1680,34 @@ void COM_LoadCacheFile (char *path, struct cache_user_s *cu)
1671 COM_LoadFile (path, 3); 1680 COM_LoadFile (path, 3);
1672} 1681}
1673 1682
1683/*
1684 * This function was NOT originally thread-safe, leading to a race
1685 * condition between the Mod_LoadModel and S_LoadSound (which run in
1686 * different threads). Fixed with mutex lock. - FW 7/29/19
1687 */
1688
1674// uses temp hunk if larger than bufsize 1689// uses temp hunk if larger than bufsize
1675byte *COM_LoadStackFile (char *path, void *buffer, int bufsize) 1690byte *COM_LoadStackFile (char *path, void *buffer, int bufsize)
1676{ 1691{
1677 byte *buf; 1692 static struct mutex m;
1678 1693 static int init = 0;
1679 loadbuf = (byte *)buffer; 1694 if(!init)
1680 loadsize = bufsize; 1695 {
1681 buf = COM_LoadFile (path, 4); 1696 rb->mutex_init(&m);
1682 1697 init = 1;
1683 return buf; 1698 }
1699
1700 rb->mutex_lock(&m);
1701
1702 byte *buf;
1703
1704 loadbuf = (byte *)buffer;
1705 loadsize = bufsize;
1706 buf = COM_LoadFile (path, 4);
1707
1708 rb->mutex_unlock(&m);
1709
1710 return buf;
1684} 1711}
1685 1712
1686/* 1713/*
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)
1472 1472
1473 version = LittleLongUnaligned (pinmodel->version); 1473 version = LittleLongUnaligned (pinmodel->version);
1474 if (version != ALIAS_VERSION) 1474 if (version != ALIAS_VERSION)
1475 {
1476 rb->splashf(HZ*2, "Likely race condition! S_LoadSound and this use the same allocator!");
1475 Sys_Error ("%s has wrong version number (%i should be %i)", 1477 Sys_Error ("%s has wrong version number (%i should be %i)",
1476 mod->name, version, ALIAS_VERSION); 1478 mod->name, version, ALIAS_VERSION);
1479 }
1477 1480
1478// 1481//
1479// allocate space for a working header, plus all the data except the frames, 1482// allocate space for a working header, plus all the data except the frames,