Opened 2 years ago

Closed 2 years ago

#13512 closed defect (fixed)

AGS: Segfault on quitting a game

Reported by: eriktorbjorn Owned by: criezy
Priority: high Component: Engine: AGS
Version: Keywords:
Cc: Game:

Description

The current development version of ScummVM segfaults when I quit an AGS game. Unfortunately I don't know enough about it to fix it on my own. This is what I see in Valgrind with 5 Day A Stranger when pressinng Quit on its main menu:

==232807== Invalid write of size 4
==232807==    at 0xDC39AC: AGS3::ScriptViewport::Invalidate() (script_viewport.h:42)
==232807==    by 0xDC35C0: AGS3::GameState::FreeViewportsAndCameras() (game_state.cpp:822)
==232807==    by 0xDB61E2: AGS3::unload_game_file() (game.cpp:362)
==232807==    by 0xE77B21: AGS3::quit_release_data() (quit.cpp:153)
==232807==    by 0xE77D39: AGS3::quit_free() (quit.cpp:230)
==232807==    by 0xCDA4E7: AGS::AGSEngine::run() (ags.cpp:192)
==232807==    by 0x988466: runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) (main.cpp:318)
==232807==    by 0x989D16: scummvm_main (main.cpp:626)
==232807==    by 0x98586C: main (posix-main.cpp:44)
==232807==  Address 0xb7d7b68 is 8 bytes inside a block of size 16 free'd
==232807==    at 0x74B371B: operator delete(void*) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==232807==    by 0xE3BB7D: AGS3::ScriptViewport::~ScriptViewport() (script_viewport.h:30)
==232807==    by 0xE3BA03: AGS3::ScriptViewport::Dispose(char const*, bool) (script_viewport.cpp:42)
==232807==    by 0xE3723D: AGS3::ManagedObjectPool::Remove(AGS3::ManagedObjectPool::ManagedObject&, bool) (managed_object_pool.cpp:46)
==232807==    by 0xE3856A: AGS3::ManagedObjectPool::reset() (managed_object_pool.cpp:364)
==232807==    by 0xE3616A: AGS3::ccUnregisterAllObjects() (cc_dynamic_object.cpp:76)
==232807==    by 0xE7771A: AGS3::quit_shutdown_scripts() (quit.cpp:76)
==232807==    by 0xE77C78: AGS3::quit_free() (quit.cpp:204)
==232807==    by 0xCDA4E7: AGS::AGSEngine::run() (ags.cpp:192)
==232807==    by 0x988466: runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) (main.cpp:318)
==232807==    by 0x989D16: scummvm_main (main.cpp:626)
==232807==    by 0x98586C: main (posix-main.cpp:44)
==232807==  Block was alloc'd at
==232807==    at 0x74B0F2F: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==232807==    by 0xDBF60A: AGS3::GameState::CreateRoomViewport() (game_state.cpp:245)
==232807==    by 0xDBF3F0: AGS3::GameState::CreatePrimaryViewportAndCamera() (game_state.cpp:229)
==232807==    by 0xE429F4: AGS3::AGS::Engine::InitAndRegisterGameEntities(AGS3::GameSetupStruct&) (game_init.cpp:250)
==232807==    by 0xE436DD: AGS3::AGS::Engine::InitGameState(AGS3::AGS::Shared::LoadedGameEntities const&, AGS3::GameDataVersion) (game_init.cpp:383)
==232807==    by 0xE6FDC0: AGS3::load_game_file() (game_file.cpp:197)
==232807==    by 0xE68EFC: AGS3::engine_load_game_data() (engine.cpp:378)
==232807==    by 0xE6D8CC: AGS3::initialize_engine(AGS3::std::map<AGS3::AGS::Shared::String, AGS3::std::map<AGS3::AGS::Shared::String, AGS3::AGS::Shared::String, Common::Less<AGS3::AGS::Shared::String> >, Common::Less<AGS3::AGS::Shared::String> > const&) (engine.cpp:1147)
==232807==    by 0xCDA4E2: AGS::AGSEngine::run() (ags.cpp:189)
==232807==    by 0x988466: runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) (main.cpp:318)
==232807==    by 0x989D16: scummvm_main (main.cpp:626)
==232807==    by 0x98586C: main (posix-main.cpp:44)
==232807== 
==232807== Invalid write of size 4
==232807==    at 0xDC397E: AGS3::ScriptCamera::Invalidate() (script_camera.h:43)
==232807==    by 0xDC3648: AGS3::GameState::FreeViewportsAndCameras() (game_state.cpp:828)
==232807==    by 0xDB61E2: AGS3::unload_game_file() (game.cpp:362)
==232807==    by 0xE77B21: AGS3::quit_release_data() (quit.cpp:153)
==232807==    by 0xE77D39: AGS3::quit_free() (quit.cpp:230)
==232807==    by 0xCDA4E7: AGS::AGSEngine::run() (ags.cpp:192)
==232807==    by 0x988466: runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) (main.cpp:318)
==232807==    by 0x989D16: scummvm_main (main.cpp:626)
==232807==    by 0x98586C: main (posix-main.cpp:44)
==232807==  Address 0xb80c998 is 8 bytes inside a block of size 16 free'd
==232807==    at 0x74B371B: operator delete(void*) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==232807==    by 0xE39D1D: AGS3::ScriptCamera::~ScriptCamera() (script_camera.h:30)
==232807==    by 0xE39BA3: AGS3::ScriptCamera::Dispose(char const*, bool) (script_camera.cpp:42)
==232807==    by 0xE3723D: AGS3::ManagedObjectPool::Remove(AGS3::ManagedObjectPool::ManagedObject&, bool) (managed_object_pool.cpp:46)
==232807==    by 0xE3856A: AGS3::ManagedObjectPool::reset() (managed_object_pool.cpp:364)
==232807==    by 0xE3616A: AGS3::ccUnregisterAllObjects() (cc_dynamic_object.cpp:76)
==232807==    by 0xE7771A: AGS3::quit_shutdown_scripts() (quit.cpp:76)
==232807==    by 0xE77C78: AGS3::quit_free() (quit.cpp:204)
==232807==    by 0xCDA4E7: AGS::AGSEngine::run() (ags.cpp:192)
==232807==    by 0x988466: runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) (main.cpp:318)
==232807==    by 0x989D16: scummvm_main (main.cpp:626)
==232807==    by 0x98586C: main (posix-main.cpp:44)
==232807==  Block was alloc'd at
==232807==    at 0x74B0F2F: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==232807==    by 0xDBFB33: AGS3::GameState::CreateRoomCamera() (game_state.cpp:303)
==232807==    by 0xDBF454: AGS3::GameState::CreatePrimaryViewportAndCamera() (game_state.cpp:233)
==232807==    by 0xE429F4: AGS3::AGS::Engine::InitAndRegisterGameEntities(AGS3::GameSetupStruct&) (game_init.cpp:250)
==232807==    by 0xE436DD: AGS3::AGS::Engine::InitGameState(AGS3::AGS::Shared::LoadedGameEntities const&, AGS3::GameDataVersion) (game_init.cpp:383)
==232807==    by 0xE6FDC0: AGS3::load_game_file() (game_file.cpp:197)
==232807==    by 0xE68EFC: AGS3::engine_load_game_data() (engine.cpp:378)
==232807==    by 0xE6D8CC: AGS3::initialize_engine(AGS3::std::map<AGS3::AGS::Shared::String, AGS3::std::map<AGS3::AGS::Shared::String, AGS3::AGS::Shared::String, Common::Less<AGS3::AGS::Shared::String> >, Common::Less<AGS3::AGS::Shared::String> > const&) (engine.cpp:1147)
==232807==    by 0xCDA4E2: AGS::AGSEngine::run() (ags.cpp:189)
==232807==    by 0x988466: runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) (main.cpp:318)
==232807==    by 0x989D16: scummvm_main (main.cpp:626)
==232807==    by 0x98586C: main (posix-main.cpp:44)
==232807== 
WARNING: movement control not supported, mouse control can't be enabled!
***** ENGINE HAS SHUTDOWN
==232807== Mismatched free() / delete / delete []
==232807==    at 0x74B371B: operator delete(void*) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==232807==    by 0xD3D412: Common::BasePtrTrackerImpl<unsigned char>::destructObject() (ptr.h:94)
==232807==    by 0x96604B: Common::BasePtrTrackerInternal::decStrong() (ptr.h:65)
==232807==    by 0xCEDC48: Common::SharedPtr<unsigned char>::~SharedPtr() (ptr.h:178)
==232807==    by 0xCEBB1E: AGS3::std::vector<Common::SharedPtr<unsigned char> >::freeStorage(Common::SharedPtr<unsigned char>*, unsigned int) (vector.h:556)
==232807==    by 0xCE7805: AGS3::std::vector<Common::SharedPtr<unsigned char> >::~vector() (vector.h:185)
==232807==    by 0xCE3228: AGS3::Globals::~Globals() (globals.cpp:617)
==232807==    by 0xCD9E27: AGS::AGSEngine::~AGSEngine() (ags.cpp:101)
==232807==    by 0xCD9E93: AGS::AGSEngine::~AGSEngine() (ags.cpp:102)
==232807==    by 0x98854C: runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) (main.cpp:331)
==232807==    by 0x989D16: scummvm_main (main.cpp:626)
==232807==    by 0x98586C: main (posix-main.cpp:44)
==232807==  Address 0xb7db810 is 0 bytes inside a block of size 245 alloc'd
==232807==    at 0x74B220F: operator new[](unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==232807==    by 0xD376F2: AGS3::AGS::Shared::ReadDialogs(AGS3::DialogTopic*&, AGS3::std::vector<Common::SharedPtr<unsigned char> >&, AGS3::std::vector<AGS3::AGS::Shared::String>&, AGS3::std::vector<AGS3::AGS::Shared::String>&, AGS3::AGS::Shared::Stream*, AGS3::GameDataVersion, int) (main_game_file.cpp:288)
==232807==    by 0xD39AC7: AGS3::AGS::Shared::ReadGameData(AGS3::AGS::Shared::LoadedGameEntities&, AGS3::AGS::Shared::Stream*, AGS3::GameDataVersion) (main_game_file.cpp:802)
==232807==    by 0xE6FB3A: AGS3::load_game_file() (game_file.cpp:175)
==232807==    by 0xE68EFC: AGS3::engine_load_game_data() (engine.cpp:378)
==232807==    by 0xE6D8CC: AGS3::initialize_engine(AGS3::std::map<AGS3::AGS::Shared::String, AGS3::std::map<AGS3::AGS::Shared::String, AGS3::AGS::Shared::String, Common::Less<AGS3::AGS::Shared::String> >, Common::Less<AGS3::AGS::Shared::String> > const&) (engine.cpp:1147)
==232807==    by 0xCDA4E2: AGS::AGSEngine::run() (ags.cpp:189)
==232807==    by 0x988466: runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) (main.cpp:318)
==232807==    by 0x989D16: scummvm_main (main.cpp:626)
==232807==    by 0x98586C: main (posix-main.cpp:44)
=

Change History (7)

comment:1 by lotharsm, 2 years ago

Priority: normalhigh

comment:2 by criezy, 2 years ago

I can indeed reproduce the crash with all the games I tried. This is likely a recent regression introduced when applying the upstream commits as all the games I tried used to quit properly.

Address Sanitizer reports the same issue as valgrind:

WRITE of size 4 at 0x00011bbafd98 thread T0
    #0 0x103addd58 in AGS3::ScriptViewport::Invalidate() script_viewport.h:42
    #1 0x103aeedc4 in AGS3::GameState::FreeViewportsAndCameras() game_state.cpp:822
    #2 0x103ab53e8 in AGS3::unload_game_file() game.cpp:362
    #3 0x103d546c4 in AGS3::quit_release_data() quit.cpp:153
    #4 0x103d55038 in AGS3::quit_free() quit.cpp:230
    #5 0x10381aff0 in AGS::AGSEngine::run() ags.cpp:198
    #6 0x102e12678 in runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) main.cpp:318
    #7 0x102e0e0d4 in scummvm_main main.cpp:619
    #8 0x102e0480c in main macosx-main.cpp:44
    #9 0x1a2b7942c in start+0x0 (libdyld.dylib:arm64e+0x1842c)

0x00011bbafd98 is located 8 bytes inside of 16-byte region [0x00011bbafd90,0x00011bbafda0)
freed by thread T0 here:
    #0 0x11774aacc in wrap__ZdlPv+0x74 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x4aacc)
    #1 0x103c807e0 in AGS3::ScriptViewport::Dispose(char const*, bool) script_viewport.cpp:42
    #2 0x103c729b0 in AGS3::ManagedObjectPool::Remove(AGS3::ManagedObjectPool::ManagedObject&, bool) managed_object_pool.cpp:46
    #3 0x103c76ed8 in AGS3::ManagedObjectPool::reset() managed_object_pool.cpp:364
    #4 0x103c702ec in AGS3::ccUnregisterAllObjects() cc_dynamic_object.cpp:76
    #5 0x103d53a78 in AGS3::quit_shutdown_scripts() quit.cpp:76
    #6 0x103d54cd4 in AGS3::quit_free() quit.cpp:204
    #7 0x10381aff0 in AGS::AGSEngine::run() ags.cpp:198
    #8 0x102e12678 in runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) main.cpp:318
    #9 0x102e0e0d4 in scummvm_main main.cpp:619
    #10 0x102e0480c in main macosx-main.cpp:44
    #11 0x1a2b7942c in start+0x0 (libdyld.dylib:arm64e+0x1842c)

previously allocated by thread T0 here:
    #0 0x11774a6b4 in wrap__Znwm+0x74 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x4a6b4)
    #1 0x103adc7f8 in AGS3::GameState::CreateRoomViewport() game_state.cpp:245
    #2 0x103adc3a8 in AGS3::GameState::CreatePrimaryViewportAndCamera() game_state.cpp:229
    #3 0x103c97570 in AGS3::AGS::Engine::InitAndRegisterGameEntities(AGS3::GameSetupStruct&) game_init.cpp:250
    #4 0x103c9a2d4 in AGS3::AGS::Engine::InitGameState(AGS3::AGS::Shared::LoadedGameEntities const&, AGS3::GameDataVersion) game_init.cpp:383
    #5 0x103d312e4 in AGS3::load_game_file() game_file.cpp:197
    #6 0x103d167bc in AGS3::engine_load_game_data() engine.cpp:378
    #7 0x103d24620 in AGS3::initialize_engine(AGS3::std::map<AGS3::AGS::Shared::String, AGS3::std::map<AGS3::AGS::Shared::String, AGS3::AGS::Shared::String, Common::Less<AGS3::AGS::Shared::String> >, Common::Less<AGS3::AGS::Shared::String> > const&) engine.cpp:1147
    #8 0x10381afec in AGS::AGSEngine::run() ags.cpp:195

comment:3 by criezy, 2 years ago

I have now confirmed that the same crash occurs with current master in upstream AGS. I will gather a bit more information and report the bug to them.

comment:5 by eriktorbjorn, 2 years ago

The upstreams bug is marked as having been fixed by https://github.com/adventuregamestudio/ags/pull/1680

comment:6 by criezy, 2 years ago

It is indeed, and I verified it. But I am getting our code base up to date with upstream AGS, and I am still a few weeks away from this commit. I am hoping to get there by this weekend.

comment:7 by criezy, 2 years ago

Owner: set to criezy
Resolution: fixed
Status: newclosed

The fix from upstream has now been merged in both master and branch-2-6.

Note: See TracTickets for help on using tickets.