Opened 9 years ago

Closed 9 years ago

Last modified 6 years ago

#5533 closed defect (fixed)

SCUMMVM: Crash on exit

Reported by: Templier Owned by:
Priority: normal Component: Engine: SCUMM
Keywords: Cc:
Game:

Description

Since the opengl branch merge, ScummVM crashes on exit.

The EventManager is deleted in ModularBackend before the GraphicManager (and the pointer not set to NULL), so in the SdlGraphicManager, the call to the eventManager makes it crash.

scummvm.exe!Common::EventDispatcher::unregisterObserver(Common::EventObserver * obs) Line 119 + 0x8 bytes C++
scummvm.exe!SdlGraphicsManager::~SdlGraphicsManager() Line 204 C++
scummvm.exe!SdlGraphicsManager::`scalar deleting destructor'() + 0x2b bytes C++
scummvm.exe!ModularBackend::~ModularBackend() Line 44 + 0x37 bytes C++
scummvm.exe!OSystem_SDL::~OSystem_SDL() Line 97 + 0x8 bytes C++
scummvm.exe!OSystem_Win32::~OSystem_Win32() + 0x2b bytes C++
scummvm.exe!OSystem_Win32::`scalar deleting destructor'() + 0x2b bytes C++
scummvm.exe!SDL_main(int argc, char * * argv) Line 61 + 0x36 bytes C++
scummvm.exe!WinMain(HINSTANCE__ * __formal, HINSTANCE__ * __formal, HINSTANCE__ * __formal, HINSTANCE__ * __formal) Line 42 + 0x12 bytes C++
scummvm.exe!__tmainCRTStartup() Line 275 + 0x2c bytes C
scummvm.exe!WinMainCRTStartup() Line 189 C
kernel32.dll!75923677()
[Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]
ntdll.dll!77c19d42()
ntdll.dll!77c19d15()

Swapping the two delete calls in ModularBackend destructor seems to fix it for me.

Erik Torbjorn seems to have the same problem and had to do one more step to get rid of the valgrind warnings:
"The shortest set of changes I can find that gets rid of the Valgrind warnings is to swap the order of "delete _eventManager;" and "delete _graphicsManager;" i ModularBackend::~ModularBackend(), and remove SDL_Quit() from OSystem_SDL::~OSystem_SDL(). But that last one doesn't feel right."

Ticket imported from: #3121841. Ticket imported from: bugs/5533.

Change History (9)

comment:1 by bluegr, 9 years ago

Note: since this isn't mentioned above, this is with r54556

comment:2 by eriktorbjorn, 9 years ago

These are the Valgrind warnings I get on quitting Day of the Tentacle with current SVN (r54561). Apart from the event manager/graphics manager dependency, it seems it's calling SDL_Quit() before freeing a bunch of SDL surfaces:

==29405== Invalid read of size 4
==29405== at 0x8CA063F: Common::List<Common::EventDispatcher::ObserverEntry>::begin() (list.h:212)
==29405== by 0x8CA0422: Common::EventDispatcher::unregisterObserver(Common::EventObserver*) (EventDispatcher.cpp:118)
==29405== by 0x8BC8D76: SdlGraphicsManager::~SdlGraphicsManager() (sdl-graphics.cpp:202)
==29405== by 0x8BBCD02: ModularBackend::~ModularBackend() (modular-backend.cpp:44)
==29405== by 0x804E2DF: OSystem_SDL::~OSystem_SDL() (sdl.cpp:97)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405== Address 0x5305274 is 20 bytes inside a block of size 76 free'd
==29405== at 0x4023881: operator delete(void*) (vg_replace_malloc.c:387)
==29405== by 0x8BBE3D5: DefaultEventManager::~DefaultEventManager() (default-events.cpp:70)
==29405== by 0x8BBCCE0: ModularBackend::~ModularBackend() (modular-backend.cpp:43)
==29405== by 0x804E2DF: OSystem_SDL::~OSystem_SDL() (sdl.cpp:97)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405==
==29405== Invalid read of size 4
==29405== at 0x4359BBD: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x8BCA97B: SdlGraphicsManager::unloadGFXMode() (sdl-graphics.cpp:852)
==29405== by 0x8BC8D81: SdlGraphicsManager::~SdlGraphicsManager() (sdl-graphics.cpp:204)
==29405== by 0x8BBCD02: ModularBackend::~ModularBackend() (modular-backend.cpp:44)
==29405== by 0x804E2DF: OSystem_SDL::~OSystem_SDL() (sdl.cpp:97)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405== Address 0x53aa8a8 is 56 bytes inside a block of size 60 free'd
==29405== at 0x4023B6A: free (vg_replace_malloc.c:366)
==29405== by 0x4359C45: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x435C10D: SDL_VideoQuit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x43337B2: SDL_QuitSubSystem (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x433384D: SDL_Quit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x804E2D4: OSystem_SDL::~OSystem_SDL() (sdl.cpp:96)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405==
==29405== Invalid write of size 4
==29405== at 0x4359BC5: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x8BCA97B: SdlGraphicsManager::unloadGFXMode() (sdl-graphics.cpp:852)
==29405== by 0x8BC8D81: SdlGraphicsManager::~SdlGraphicsManager() (sdl-graphics.cpp:204)
==29405== by 0x8BBCD02: ModularBackend::~ModularBackend() (modular-backend.cpp:44)
==29405== by 0x804E2DF: OSystem_SDL::~OSystem_SDL() (sdl.cpp:97)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405== Address 0x53aa8a8 is 56 bytes inside a block of size 60 free'd
==29405== at 0x4023B6A: free (vg_replace_malloc.c:366)
==29405== by 0x4359C45: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x435C10D: SDL_VideoQuit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x43337B2: SDL_QuitSubSystem (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x433384D: SDL_Quit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x804E2D4: OSystem_SDL::~OSystem_SDL() (sdl.cpp:96)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405==
==29405== Invalid read of size 4
==29405== at 0x4359BE0: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x8BCA97B: SdlGraphicsManager::unloadGFXMode() (sdl-graphics.cpp:852)
==29405== by 0x8BC8D81: SdlGraphicsManager::~SdlGraphicsManager() (sdl-graphics.cpp:204)
==29405== by 0x8BBCD02: ModularBackend::~ModularBackend() (modular-backend.cpp:44)
==29405== by 0x804E2DF: OSystem_SDL::~OSystem_SDL() (sdl.cpp:97)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405== Address 0x53aa89c is 44 bytes inside a block of size 60 free'd
==29405== at 0x4023B6A: free (vg_replace_malloc.c:366)
==29405== by 0x4359C45: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x435C10D: SDL_VideoQuit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x43337B2: SDL_QuitSubSystem (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x433384D: SDL_Quit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x804E2D4: OSystem_SDL::~OSystem_SDL() (sdl.cpp:96)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405==
==29405== Invalid read of size 4
==29405== at 0x4359BE7: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x8BCA97B: SdlGraphicsManager::unloadGFXMode() (sdl-graphics.cpp:852)
==29405== by 0x8BC8D81: SdlGraphicsManager::~SdlGraphicsManager() (sdl-graphics.cpp:204)
==29405== by 0x8BBCD02: ModularBackend::~ModularBackend() (modular-backend.cpp:44)
==29405== by 0x804E2DF: OSystem_SDL::~OSystem_SDL() (sdl.cpp:97)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405== Address 0x53aa870 is 0 bytes inside a block of size 60 free'd
==29405== at 0x4023B6A: free (vg_replace_malloc.c:366)
==29405== by 0x4359C45: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x435C10D: SDL_VideoQuit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x43337B2: SDL_QuitSubSystem (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x433384D: SDL_Quit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x804E2D4: OSystem_SDL::~OSystem_SDL() (sdl.cpp:96)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405==
==29405== Invalid read of size 4
==29405== at 0x4359BEE: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x8BCA97B: SdlGraphicsManager::unloadGFXMode() (sdl-graphics.cpp:852)
==29405== by 0x8BC8D81: SdlGraphicsManager::~SdlGraphicsManager() (sdl-graphics.cpp:204)
==29405== by 0x8BBCD02: ModularBackend::~ModularBackend() (modular-backend.cpp:44)
==29405== by 0x804E2DF: OSystem_SDL::~OSystem_SDL() (sdl.cpp:97)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405== Address 0x53aa874 is 4 bytes inside a block of size 60 free'd
==29405== at 0x4023B6A: free (vg_replace_malloc.c:366)
==29405== by 0x4359C45: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x435C10D: SDL_VideoQuit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x43337B2: SDL_QuitSubSystem (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x433384D: SDL_Quit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x804E2D4: OSystem_SDL::~OSystem_SDL() (sdl.cpp:96)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405==
==29405== Invalid read of size 4
==29405== at 0x4359C04: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x8BCA97B: SdlGraphicsManager::unloadGFXMode() (sdl-graphics.cpp:852)
==29405== by 0x8BC8D81: SdlGraphicsManager::~SdlGraphicsManager() (sdl-graphics.cpp:204)
==29405== by 0x8BBCD02: ModularBackend::~ModularBackend() (modular-backend.cpp:44)
==29405== by 0x804E2DF: OSystem_SDL::~OSystem_SDL() (sdl.cpp:97)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405== Address 0x53aa8a0 is 48 bytes inside a block of size 60 free'd
==29405== at 0x4023B6A: free (vg_replace_malloc.c:366)
==29405== by 0x4359C45: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x435C10D: SDL_VideoQuit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x43337B2: SDL_QuitSubSystem (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x433384D: SDL_Quit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x804E2D4: OSystem_SDL::~OSystem_SDL() (sdl.cpp:96)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405==
==29405== Invalid read of size 4
==29405== at 0x4359C1A: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x8BCA97B: SdlGraphicsManager::unloadGFXMode() (sdl-graphics.cpp:852)
==29405== by 0x8BC8D81: SdlGraphicsManager::~SdlGraphicsManager() (sdl-graphics.cpp:204)
==29405== by 0x8BBCD02: ModularBackend::~ModularBackend() (modular-backend.cpp:44)
==29405== by 0x804E2DF: OSystem_SDL::~OSystem_SDL() (sdl.cpp:97)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405== Address 0x53aa88c is 28 bytes inside a block of size 60 free'd
==29405== at 0x4023B6A: free (vg_replace_malloc.c:366)
==29405== by 0x4359C45: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x435C10D: SDL_VideoQuit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x43337B2: SDL_QuitSubSystem (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x433384D: SDL_Quit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x804E2D4: OSystem_SDL::~OSystem_SDL() (sdl.cpp:96)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405==
==29405== Invalid read of size 4
==29405== at 0x4359C2D: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x8BCA97B: SdlGraphicsManager::unloadGFXMode() (sdl-graphics.cpp:852)
==29405== by 0x8BC8D81: SdlGraphicsManager::~SdlGraphicsManager() (sdl-graphics.cpp:204)
==29405== by 0x8BBCD02: ModularBackend::~ModularBackend() (modular-backend.cpp:44)
==29405== by 0x804E2DF: OSystem_SDL::~OSystem_SDL() (sdl.cpp:97)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405== Address 0x53aa884 is 20 bytes inside a block of size 60 free'd
==29405== at 0x4023B6A: free (vg_replace_malloc.c:366)
==29405== by 0x4359C45: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x435C10D: SDL_VideoQuit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x43337B2: SDL_QuitSubSystem (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x433384D: SDL_Quit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x804E2D4: OSystem_SDL::~OSystem_SDL() (sdl.cpp:96)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405==
==29405== Invalid read of size 4
==29405== at 0x4359C34: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x8BCA97B: SdlGraphicsManager::unloadGFXMode() (sdl-graphics.cpp:852)
==29405== by 0x8BC8D81: SdlGraphicsManager::~SdlGraphicsManager() (sdl-graphics.cpp:204)
==29405== by 0x8BBCD02: ModularBackend::~ModularBackend() (modular-backend.cpp:44)
==29405== by 0x804E2DF: OSystem_SDL::~OSystem_SDL() (sdl.cpp:97)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405== Address 0x53aa870 is 0 bytes inside a block of size 60 free'd
==29405== at 0x4023B6A: free (vg_replace_malloc.c:366)
==29405== by 0x4359C45: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x435C10D: SDL_VideoQuit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x43337B2: SDL_QuitSubSystem (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x433384D: SDL_Quit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x804E2D4: OSystem_SDL::~OSystem_SDL() (sdl.cpp:96)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405==
==29405== Invalid free() / delete / delete[]
==29405== at 0x4023B6A: free (vg_replace_malloc.c:366)
==29405== by 0x4359C67: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x8BCA97B: SdlGraphicsManager::unloadGFXMode() (sdl-graphics.cpp:852)
==29405== by 0x8BC8D81: SdlGraphicsManager::~SdlGraphicsManager() (sdl-graphics.cpp:204)
==29405== by 0x8BBCD02: ModularBackend::~ModularBackend() (modular-backend.cpp:44)
==29405== by 0x804E2DF: OSystem_SDL::~OSystem_SDL() (sdl.cpp:97)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405== Address 0x53aa938 is 0 bytes inside a block of size 128,000 free'd
==29405== at 0x4023B6A: free (vg_replace_malloc.c:366)
==29405== by 0x4359C67: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x435C10D: SDL_VideoQuit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x43337B2: SDL_QuitSubSystem (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x433384D: SDL_Quit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x804E2D4: OSystem_SDL::~OSystem_SDL() (sdl.cpp:96)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405==
==29405== Invalid free() / delete / delete[]
==29405== at 0x4023B6A: free (vg_replace_malloc.c:366)
==29405== by 0x4359C45: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x8BCA97B: SdlGraphicsManager::unloadGFXMode() (sdl-graphics.cpp:852)
==29405== by 0x8BC8D81: SdlGraphicsManager::~SdlGraphicsManager() (sdl-graphics.cpp:204)
==29405== by 0x8BBCD02: ModularBackend::~ModularBackend() (modular-backend.cpp:44)
==29405== by 0x804E2DF: OSystem_SDL::~OSystem_SDL() (sdl.cpp:97)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)
==29405== Address 0x53aa870 is 0 bytes inside a block of size 60 free'd
==29405== at 0x4023B6A: free (vg_replace_malloc.c:366)
==29405== by 0x4359C45: SDL_FreeSurface (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x435C10D: SDL_VideoQuit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x43337B2: SDL_QuitSubSystem (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x433384D: SDL_Quit (in /usr/lib/libSDL-1.2.so.0.11.3)
==29405== by 0x804E2D4: OSystem_SDL::~OSystem_SDL() (sdl.cpp:96)
==29405== by 0x804FD3D: OSystem_POSIX::~OSystem_POSIX() (posix.h:35)
==29405== by 0x804FB1D: main (posix-main.cpp:49)

comment:3 by Templier, 9 years ago

Note: The crash in SdlGraphicsManager::unloadGFXMode() line 852 (due to SDL_Quit() being called earlier) seems to be intermittent. It didn't happen to me at first and then reappeared. Whether I start a game or just load the menu doesn't seem to change anything.

comment:4 by fingolfin, 9 years ago

Owner: set to fingolfin

comment:5 by fingolfin, 9 years ago

Hopefully this is fixed in trunk now, please let me know.

comment:6 by bluegr, 9 years ago

Seems to be fixed in the trunk (at least it is on Windows). I no longer get crashes when exiting

comment:7 by eriktorbjorn, 9 years ago

The Valgrind warnings seem to be gone, too.

comment:8 by fingolfin, 9 years ago

Resolution: fixed
Status: newclosed

comment:9 by Strangerke, 6 years ago

Component: --Unset--Engine: SCUMM
Owner: fingolfin removed
Note: See TracTickets for help on using tickets.