I can't seem to reproduce it any more. Perhaps a lingering side-effect of using the -x option to load a savegame earlier? That gives me Valgrind warnings, as noted in bug #2057194.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
No, it doesn't have anything to do with the -x option. It just crashed again when loading another savegame. This time it did create a core dump, but it doesn't seem to contain any useful information.
Some sort of memory corruption going on? Or some thread-unsafety?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Hopefully, it might be fixed with revision #34023, as loading from the command line did not initialize the chapter number correctly. But then again, I could be wrong. Since it's hard to reproduce this bug, I'm not sure what could be causing it, perhaps memory corruption or thread unsafety as you mentioned. Could you try turning the music off and retest, in case that's the problem?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
As I said, it turned out that it happened even if I didn't use the -x option when I started ScummVM. But it only happened a few times yesterday, and not yet today - even before the change - so it's hard to say whether or not it's gone.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
It just happened again - this time with a savegame from Ted's chapter - so the fresh recompile didn't help. (Which, I guess, is a relief in some ways.) Glibc claimed to have detected memory corruption. As before, the generated core dump did not contain any useful information.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
And now it's happened at least once when loading a savegame right after escaping the intro. That doesn't leave too much time for memory to get corrupted, I guess.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I've managed to catch a Valgrind warning when loading a saved game:
==1643== Invalid write of size 4
==1643== at 0x418AA00: snd_seq_event_output_buffer (in /usr/lib/libasound.so.2.0.0)
==1643== by 0x418AABB: snd_seq_event_output (in /usr/lib/libasound.so.2.0.0)
==1643== by 0x86191DD: MidiDriver_ALSA::send_event(int) (alsa.cpp:237)
==1643== by 0x861964D: MidiDriver_ALSA::send(unsigned) (alsa.cpp:172)
==1643== by 0x85EE58E: MidiChannel_MPU401::controlChange(unsigned char, unsigned char) (mpu401.cpp:67)
==1643== by 0x83D4383: MidiChannel::volume(unsigned char) (mididrv.h:255)
==1643== by 0x84A5CF6: Saga::MusicPlayer::setVolume(int) (music.cpp:254)
==1643== by 0x84A5E15: Saga::Music::setVolume(int, int) (music.cpp:413)
==1643== by 0x8491F17: Saga::SagaEngine::load(char const*) (saveload.cpp:345)
==1643== by 0x84C2F86: Saga::Interface::setLoad(Saga::PanelButton*) (interface.cpp:1155)
==1643== by 0x84C304F: Saga::Interface::handleLoadUpdate(Common::Point const&) (interface.cpp:1128)
==1643== by 0x84C3F42: Saga::Interface::update(Common::Point const&, int) (interface.cpp:1788)
==1643== Address 0x6be816c is not stack'd, malloc'd or (recently) free'd
There were other, similar ones, too.
Another one I've seen once, but which I don't think is related to this crash (because it was accompanied by a "WARNING: spriteList.spriteCount <= spriteNumber!" message) was this one, and many similar:
==1643== Conditional jump or move depends on uninitialised value(s)
==1643== at 0x80B6D83: int CLIP<int>(int, int, int) (util.h:44)
==1643== by 0x84A154C: Saga::Sprite::drawClip(Saga::Surface*, Common::Rect const&, Common::Point const&, int, int, unsigned char const*) (sprite.cpp:225)
==1643== by 0x84A1B9D: Saga::Sprite::draw(Saga::Surface*, Common::Rect const&, Saga::SpriteList&, int, Common::Point const&, int) (sprite.cpp:269)
==1643== by 0x84BFF07: Saga::Interface::draw() (interface.cpp:828)
==1643== by 0x8497128: Saga::Interface::setFadeMode(int) (interface.h:201)
==1643== by 0x8494BAA: Saga::Scene::loadScene(Saga::LoadSceneParams*) (scene.cpp:637)
==1643== by 0x8495D52: Saga::Scene::changeScene(short, int, Saga::SceneTransitionType, int) (scene.cpp:508)
==1643== by 0x84919A1: Saga::SagaEngine::load(char const*) (saveload.cpp:270)
==1643== by 0x84C2F86: Saga::Interface::setLoad(Saga::PanelButton*) (interface.cpp:1155)
==1643== by 0x84C304F: Saga::Interface::handleLoadUpdate(Common::Point const&) (interface.cpp:1128)
==1643== by 0x84C3F42: Saga::Interface::update(Common::Point const&, int) (interface.cpp:1788)
==1643== by 0x84D4780: Saga::Render::drawScene() (render.cpp:163)
I guess I'll have to add a few debug printf()s around the music handling.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I've added mutex locking to the setVolume() function. (I did that in the Tinsel engine a while back to fix some mysterious problems, but didn't make the connection until now.) Maybe it will fix this crash, as well?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Buggy savegame
Logged In: YES
user_id=577918
Originator: YES
It doesn't happen always, though. I'm still trying to catch it in a debugger or core file or something...
Logged In: YES
user_id=577918
Originator: YES
I can't seem to reproduce it any more. Perhaps a lingering side-effect of using the -x option to load a savegame earlier? That gives me Valgrind warnings, as noted in bug #2057194.
Logged In: YES
user_id=577918
Originator: YES
No, it doesn't have anything to do with the -x option. It just crashed again when loading another savegame. This time it did create a core dump, but it doesn't seem to contain any useful information.
Some sort of memory corruption going on? Or some thread-unsafety?
Logged In: YES
user_id=991970
Originator: NO
Can you test if this still occurs?
Logged In: YES
user_id=577918
Originator: YES
It was happening yesterday. Has any change been made so that it won't happen today?
(It's hard to test, because it's so unpredictable.)
Logged In: YES
user_id=991970
Originator: NO
Hopefully, it might be fixed with revision #34023, as loading from the command line did not initialize the chapter number correctly. But then again, I could be wrong. Since it's hard to reproduce this bug, I'm not sure what could be causing it, perhaps memory corruption or thread unsafety as you mentioned. Could you try turning the music off and retest, in case that's the problem?
Logged In: YES
user_id=577918
Originator: YES
As I said, it turned out that it happened even if I didn't use the -x option when I started ScummVM. But it only happened a few times yesterday, and not yet today - even before the change - so it's hard to say whether or not it's gone.
Logged In: YES
user_id=577918
Originator: YES
It just happened again - this time with a savegame from Ted's chapter - so the fresh recompile didn't help. (Which, I guess, is a relief in some ways.) Glibc claimed to have detected memory corruption. As before, the generated core dump did not contain any useful information.
Logged In: YES
user_id=577918
Originator: YES
And now it's happened at least once when loading a savegame right after escaping the intro. That doesn't leave too much time for memory to get corrupted, I guess.
Logged In: YES
user_id=577918
Originator: YES
I've managed to catch a Valgrind warning when loading a saved game:
==1643== Invalid write of size 4
==1643== at 0x418AA00: snd_seq_event_output_buffer (in /usr/lib/libasound.so.2.0.0)
==1643== by 0x418AABB: snd_seq_event_output (in /usr/lib/libasound.so.2.0.0)
==1643== by 0x86191DD: MidiDriver_ALSA::send_event(int) (alsa.cpp:237)
==1643== by 0x861964D: MidiDriver_ALSA::send(unsigned) (alsa.cpp:172)
==1643== by 0x85EE58E: MidiChannel_MPU401::controlChange(unsigned char, unsigned char) (mpu401.cpp:67)
==1643== by 0x83D4383: MidiChannel::volume(unsigned char) (mididrv.h:255)
==1643== by 0x84A5CF6: Saga::MusicPlayer::setVolume(int) (music.cpp:254)
==1643== by 0x84A5E15: Saga::Music::setVolume(int, int) (music.cpp:413)
==1643== by 0x8491F17: Saga::SagaEngine::load(char const*) (saveload.cpp:345)
==1643== by 0x84C2F86: Saga::Interface::setLoad(Saga::PanelButton*) (interface.cpp:1155)
==1643== by 0x84C304F: Saga::Interface::handleLoadUpdate(Common::Point const&) (interface.cpp:1128)
==1643== by 0x84C3F42: Saga::Interface::update(Common::Point const&, int) (interface.cpp:1788)
==1643== Address 0x6be816c is not stack'd, malloc'd or (recently) free'd
There were other, similar ones, too.
Another one I've seen once, but which I don't think is related to this crash (because it was accompanied by a "WARNING: spriteList.spriteCount <= spriteNumber!" message) was this one, and many similar:
==1643== Conditional jump or move depends on uninitialised value(s)
==1643== at 0x80B6D83: int CLIP<int>(int, int, int) (util.h:44)
==1643== by 0x84A154C: Saga::Sprite::drawClip(Saga::Surface*, Common::Rect const&, Common::Point const&, int, int, unsigned char const*) (sprite.cpp:225)
==1643== by 0x84A1B9D: Saga::Sprite::draw(Saga::Surface*, Common::Rect const&, Saga::SpriteList&, int, Common::Point const&, int) (sprite.cpp:269)
==1643== by 0x84BFF07: Saga::Interface::draw() (interface.cpp:828)
==1643== by 0x8497128: Saga::Interface::setFadeMode(int) (interface.h:201)
==1643== by 0x8494BAA: Saga::Scene::loadScene(Saga::LoadSceneParams*) (scene.cpp:637)
==1643== by 0x8495D52: Saga::Scene::changeScene(short, int, Saga::SceneTransitionType, int) (scene.cpp:508)
==1643== by 0x84919A1: Saga::SagaEngine::load(char const*) (saveload.cpp:270)
==1643== by 0x84C2F86: Saga::Interface::setLoad(Saga::PanelButton*) (interface.cpp:1155)
==1643== by 0x84C304F: Saga::Interface::handleLoadUpdate(Common::Point const&) (interface.cpp:1128)
==1643== by 0x84C3F42: Saga::Interface::update(Common::Point const&, int) (interface.cpp:1788)
==1643== by 0x84D4780: Saga::Render::drawScene() (render.cpp:163)
I guess I'll have to add a few debug printf()s around the music handling.
Logged In: YES
user_id=577918
Originator: YES
I've added mutex locking to the setVolume() function. (I did that in the Tinsel engine a while back to fix some mysterious problems, but didn't make the connection until now.) Maybe it will fix this crash, as well?
Logged In: YES
user_id=577918
Originator: YES
It's far from conclusive, and may be just wishful thinking, but I haven't been able to reproduce the crash since I added the mutex locking.
Logged In: YES
user_id=991970
Originator: NO
This seems to be caused by the same issue as bug #2058408
Since you haven't been able to reproduce this with the mutex lock, I'm tempted to lower the priority of these two bugs to 1, or close them altogether
Logged In: YES
user_id=991970
Originator: NO
I'll close this for now, as it seems to be fixed