[Scummvm-tracker] [ScummVM :: Bugs] #14638: SWORD1: Crashes due to data races (multi-threading)
ScummVM :: Bugs
trac at scummvm.org
Wed Sep 27 18:34:36 UTC 2023
#14638: SWORD1: Crashes due to data races (multi-threading)
-------------------------+-----------------------------
Reporter: PushmePullyu | Owner: AndywinXp
Type: defect | Status: pending
Priority: normal | Component: Engine: Sword1
Version: | Resolution: fixed
Keywords: | Game: Broken Sword 1
-------------------------+-----------------------------
Comment (by PushmePullyu):
Thanks for the quick response! I just tested
!da5c3b8cd833a7de31d0e946248f1a7ab6a039ea
It is still crashing.
I wonder if the goal of improving timer accuracy will still be achieved
after adding all the necessary locks and if it is worth the added
complexity.
It might be safer to move back to a single thread and rework the main loop
instead.
Info on the crashes:
All crashes happened while repeatedly opening and closing the menu and
simultaneously moving the mouse around / over the inventory.
Crash 1 (line numbers will be slightly off due to added debugging code):
Thread 1 "scummvm" received signal SIGSEGV, Segmentation fault.
{{{
#0 SurfaceSdlGraphicsManager::SDL_SetColors(SDL_Surface*, SDL_Color*,
int, int) (this=0x1658050, surface=0x2c2f810, colors=0x16670b0,
firstcolor=0, ncolors=256)
at backends/graphics/surfacesdl/surfacesdl-graphics.cpp:2884
#1 0x00000000006d59f6 in SurfaceSdlGraphicsManager::blitCursor()
(this=0x1658050) at backends/graphics/surfacesdl/surfacesdl-
graphics.cpp:2289
#2 0x00000000006d5627 in SurfaceSdlGraphicsManager::setMouseCursor(void
const*, unsigned int, unsigned int, int, int, unsigned int, bool,
Graphics::PixelFormat const*, unsigned char const*) (this=0x1658050,
buf=0x2bdbf20, w=13, h=16, hotspotX=1, hotspotY=1, keyColor=255,
dontScale=false, format=0x279f8e0, mask=0x0) at
backends/graphics/surfacesdl/surfacesdl-graphics.cpp:2211
#3 0x00000000006b110a in ModularGraphicsBackend::setMouseCursor(void
const*, unsigned int, unsigned int, int, int, unsigned int, bool,
Graphics::PixelFormat const*, unsigned char const*)
(this=0x14b5560, buf=0x2bdbf20, w=13, h=16, hotspotX=1, hotspotY=1,
keycolor=255, dontScale=false, format=0x279f8e0, mask=0x0) at backends
/modular-backend.cpp:268
#4 0x0000000000757cfb in
Graphics::CursorManager::replaceCursor(Graphics::Surface const&, int, int,
unsigned int, bool, unsigned char const*)
(this=0x272bf10, surf=..., hotspotX=1, hotspotY=1, keycolor=255,
dontScale=false, mask=0x0) at graphics/cursorman.cpp:184
#5 0x00000000007579d5 in Graphics::CursorManager::replaceCursor(void
const*, unsigned int, unsigned int, int, int, unsigned int, bool,
Graphics::PixelFormat const*, unsigned char const*)
(this=0x272bf10, buf=0x7fffac002e1a, w=13, h=16, hotspotX=1,
hotspotY=1, keycolor=255, dontScale=false, format=0x0, mask=0x0) at
graphics/cursorman.cpp:138
#6 0x00000000005d652e in Sword1::Mouse::animate() (this=0x27c1400) at
engines/sword1/mouse.cpp:312
#7 0x00000000005d637b in Sword1::Mouse::setPointer(unsigned int, unsigned
int) (this=0x27c1400, resId=67174400, rate=0) at
engines/sword1/mouse.cpp:289
#8 0x00000000005d2164 in
Sword1::Logic::fnSetMousePointer(Sword1::Object*, int, int, int, int, int,
int, int) (this=0x277b4b0, cpt=0x0, id=65536, tag=67174400, rate=0, e=0,
f=0, z=0, x=0)
at engines/sword1/logic.cpp:1343
#9 0x00000000005cfbc4 in Sword1::Logic::interpretScript(Sword1::Object*,
int, Sword1::Header*, int, int)
(this=0x277b4b0, compact=0x0, id=65536, scriptModule=0x2800670,
scriptBase=6, scriptNum=6) at engines/sword1/logic.cpp:522
#10 0x00000000005cf81e in Sword1::Logic::runMouseScript(Sword1::Object*,
int) (this=0x277b4b0, cpt=0x0, scriptId=6) at engines/sword1/logic.cpp:477
#11 0x00000000005d5b29 in Sword1::Mouse::engine(unsigned short, unsigned
short, unsigned short) (this=0x27c1400, x=284, y=258, eventFlags=0) at
engines/sword1/mouse.cpp:156
#12 0x00000000005c0a3f in Sword1::SwordEngine::mainLoop() (this=0x27c4910)
at engines/sword1/sword1.cpp:1072
#13 0x00000000005bf800 in Sword1::SwordEngine::go() (this=0x27c4910) at
engines/sword1/sword1.cpp:738
2883 int SurfaceSdlGraphicsManager::SDL_SetColors(SDL_Surface *surface,
SDL_Color *colors, int firstcolor, int ncolors) {
2884 if (surface->format->palette) {
2885 return
!SDL_SetPaletteColors(surface->format->palette, colors, firstcolor,
ncolors) ? 1 : 0;
2886 } else {
2887 return 0;
2888 }
(gdb) p surface
$1 = (SDL_Surface *) 0x2c2f810
(gdb) p surface->format
$2 = (SDL_PixelFormat *) 0xb81b45331bbaf1d3
}}}
The following was captured later, but might have caused the crash above:
Note: The timer thread locks Screen::_screenAccessMutex, but the main
thread does not attempt to acquire it here.
{{{
Thread 1 "scummvm" hit Breakpoint 2,
SurfaceSdlGraphicsManager::setMouseCursor (this=0x17bf350, buf=0x1e3e410,
w=22, h=22, hotspotX=7, hotspotY=7, keyColor=255, dontScale=false,
format=0x1e4f900, mask=0x0) at backends/graphics/surfacesdl
/surfacesdl-graphics.cpp:2001
Main thread:
#0 SurfaceSdlGraphicsManager::setMouseCursor(void const*, unsigned int,
unsigned int, int, int, unsigned int, bool, Graphics::PixelFormat const*,
unsigned char const*)
(this=0x17bf350, buf=0x1e3e410, w=22, h=22, hotspotX=7, hotspotY=7,
keyColor=255, dontScale=false, format=0x1e4f900, mask=0x0)
at backends/graphics/surfacesdl/surfacesdl-graphics.cpp:2001
#1 0x00000000006b110a in ModularGraphicsBackend::setMouseCursor(void
const*, unsigned int, unsigned int, int, int, unsigned int, bool,
Graphics::PixelFormat const*, unsigned char const*)
(this=0x14b5560, buf=0x1e3e410, w=22, h=22, hotspotX=7, hotspotY=7,
keycolor=255, dontScale=false, format=0x1e4f900, mask=0x0) at backends
/modular-backend.cpp:268
#2 0x0000000000757cfb in
Graphics::CursorManager::replaceCursor(Graphics::Surface const&, int, int,
unsigned int, bool, unsigned char const*)
(this=0x272e400, surf=..., hotspotX=7, hotspotY=7, keycolor=255,
dontScale=false, mask=0x0) at graphics/cursorman.cpp:184
#3 0x00000000007579d5 in Graphics::CursorManager::replaceCursor(void
const*, unsigned int, unsigned int, int, int, unsigned int, bool,
Graphics::PixelFormat const*, unsigned char const*)
(this=0x272e400, buf=0x1e3fd8a, w=22, h=22, hotspotX=7, hotspotY=7,
keycolor=255, dontScale=false, format=0x0, mask=0x0) at
graphics/cursorman.cpp:138
#4 0x00000000005d652e in Sword1::Mouse::animate() (this=0x16bfac0) at
engines/sword1/mouse.cpp:312
#5 0x00000000005d637b in Sword1::Mouse::setPointer(unsigned int, unsigned
int) (this=0x16bfac0, resId=67174403, rate=0) at
engines/sword1/mouse.cpp:289
#6 0x00000000005d2164 in
Sword1::Logic::fnSetMousePointer(Sword1::Object*, int, int, int, int, int,
int, int)
(this=0x192c0d0, cpt=0x199d388, id=65546, tag=67174403, rate=0, e=0,
f=0, z=0, x=0) at engines/sword1/logic.cpp:1343
#7 0x00000000005cfbc4 in Sword1::Logic::interpretScript(Sword1::Object*,
int, Sword1::Header*, int, int)
(this=0x192c0d0, compact=0x199d388, id=65546, scriptModule=0x19d8de0,
scriptBase=5, scriptNum=5) at engines/sword1/logic.cpp:522
#8 0x00000000005cf81e in Sword1::Logic::runMouseScript(Sword1::Object*,
int) (this=0x192c0d0, cpt=0x199d388, scriptId=5) at
engines/sword1/logic.cpp:477
#9 0x00000000005d5bb5 in Sword1::Mouse::engine(unsigned short, unsigned
short, unsigned short) (this=0x16bfac0, x=174, y=159, eventFlags=0) at
engines/sword1/mouse.cpp:161
#10 0x00000000005c0a3f in Sword1::SwordEngine::mainLoop() (this=0x282e630)
at engines/sword1/sword1.cpp:1072
#11 0x00000000005bf800 in Sword1::SwordEngine::go() (this=0x282e630) at
engines/sword1/sword1.cpp:738
Timer thread:
#0 SurfaceSdlGraphicsManager::blitCursor() (this=0x17bf350) at
backends/graphics/surfacesdl/surfacesdl-graphics.cpp:2217
#1 0x00000000006d3bfd in SurfaceSdlGraphicsManager::setPalette(unsigned
char const*, unsigned int, unsigned int) (this=0x17bf350,
colors=0x7fffd8bf9870 "", start=0, num=256)
at backends/graphics/surfacesdl/surfacesdl-graphics.cpp:1803
#2 0x00000000005e2538 in Sword1::Screen::fadePalette() (this=0x192c320)
at engines/sword1/screen.cpp:294
#3 0x00000000005c0feb in Sword1::SwordEngine::fadePaletteStep()
(this=0x282e630) at engines/sword1/sword1.cpp:1189
#4 0x00000000005c1152 in Sword1::vblCallback(void*) (refCon=0x282e630) at
engines/sword1/sword1.cpp:1225
}}}
Crash 2:
WARNING: fetchRes:: resource 0 out of bounds!
Segmentation fault (core dumped)
Crash 3 (Not actually a crash, but the freelist was corrupted):
WARNING: addToFreeList: mem block is already in freeList!
On exit:
WARNING: MemMan::flush(): _memListFreeEnd is nullptr!
WARNING: MemMan::flush: Something's wrong: still 3722627 bytes alloced!
The following shows both threads accessing the freelist simultaneously,
which could have caused the corruption
(line numbers for memman.cpp and memman.h will be slightly off):
{{{
Id Target Id Frame
* 1 Thread 0x7ffff671a880 (LWP 428055) "scummvm"
Sword1::MemMan::addToFreeList (this=0x1b2f340, bsMem=0x19d3b60) at
engines/sword1/memman.cpp:111
21 Thread 0x7fffd8bfa6c0 (LWP 428076) "SDLTimer"
Sword1::MemMan::addToFreeList (this=0x1b2f340, bsMem=0x1b4dd60) at
engines/sword1/memman.cpp:111
Main thread:
#0 Sword1::MemMan::addToFreeList(Sword1::MemHandle*) (this=0x1b2f340,
bsMem=0x19d3b60) at engines/sword1/memman.cpp:111
#1 0x00000000005eebf7 in Sword1::MemMan::setCondition(Sword1::MemHandle*,
unsigned short) (this=0x1b2f340, bsMem=0x19d3b60, pCond=1) at
engines/sword1/memman.cpp:73
#2 0x00000000005d9cdc in Sword1::ResMan::resClose(unsigned int)
(this=0x19d4a90, id=16842752) at engines/sword1/resman.cpp:284
#3 0x00000000005d99ca in Sword1::ResMan::unlockScript(unsigned int)
(this=0x19d4a90, scrID=8388609) at engines/sword1/resman.cpp:237
#4 0x00000000005cf745 in Sword1::Logic::scriptManager(Sword1::Object*,
unsigned int) (this=0x1b7a380, compact=0x1baba20, id=8388609) at
engines/sword1/logic.cpp:455
#5 0x00000000005cea78 in Sword1::Logic::processLogic(Sword1::Object*,
unsigned int) (this=0x1b7a380, compact=0x1baba20, id=8388609) at
engines/sword1/logic.cpp:225
#6 0x00000000005ce855 in Sword1::Logic::engine() (this=0x1b7a380) at
engines/sword1/logic.cpp:163
#7 0x00000000005c07a2 in Sword1::SwordEngine::mainLoop() (this=0x27dcb10)
at engines/sword1/sword1.cpp:1034
#8 0x00000000005bf800 in Sword1::SwordEngine::go() (this=0x27dcb10) at
engines/sword1/sword1.cpp:738
Timer thread:
#0 Sword1::MemMan::addToFreeList(Sword1::MemHandle*) (this=0x1b2f340,
bsMem=0x1b4dd60) at engines/sword1/memman.cpp:111
#1 0x00000000005eebf7 in Sword1::MemMan::setCondition(Sword1::MemHandle*,
unsigned short) (this=0x1b2f340, bsMem=0x1b4dd60, pCond=1) at
engines/sword1/memman.cpp:73
#2 0x00000000005d9cdc in Sword1::ResMan::resClose(unsigned int)
(this=0x19d4a90, id=67305474) at engines/sword1/resman.cpp:284
#3 0x00000000005e77ca in Sword1::Screen::showFrame(unsigned short,
unsigned short, unsigned int, unsigned int, unsigned char const*, signed
char)
(this=0x1b7a5d0, x=0, y=0, resId=67305474, frameNo=1,
fadeMask=0x1289960 <Sword1::Menu::_fadeEffectTop>
"\001\a\005\003\002\004\006", fadeStatus=6 '\006')
at engines/sword1/screen.cpp:1452
#4 0x00000000005d42e6 in Sword1::MenuIcon::draw(unsigned char const*,
signed char)
(this=0x2a30520, fadeMask=0x1289960 <Sword1::Menu::_fadeEffectTop>
"\001\a\005\003\002\004\006", fadeStatus=6 '\006') at
engines/sword1/menu.cpp:88
#5 0x00000000005d4c18 in Sword1::Menu::refresh(unsigned char)
(this=0x1b3f5c0, menuType=0 '\000') at engines/sword1/menu.cpp:238
#6 0x00000000005c0fa0 in Sword1::SwordEngine::updateTopMenu()
(this=0x27dcb10) at engines/sword1/sword1.cpp:1181
#7 0x00000000005c10f1 in Sword1::vblCallback(void*) (refCon=0x27dcb10) at
engines/sword1/sword1.cpp:1215
}}}
--
Ticket URL: <https://bugs.scummvm.org/ticket/14638#comment:6>
ScummVM :: Bugs <https://bugs.scummvm.org>
ScummVM
More information about the Scummvm-tracker
mailing list