#4844 closed defect (fixed)
LURE: Valgrind Memory Errors When Skorl Gatekeeper Drinks
| Reported by: | digitall | Owned by: | dreammaster |
|---|---|---|---|
| Priority: | normal | Component: | Engine: Lure |
| Version: | Keywords: | ||
| Cc: | Game: | Lure of the Temptress |
Description
When the Skorl Gatekeeper lays down to drink from the cask, Valgrind indicates a number of memory errors associated with hotspots. The console also issues the following warnings: WARNING: Hotspot 2725h is not currently active! WARNING: Hotspot 2726h is not currently active!
This is repeatable and is triggered by the following procedure: 1. Load Savegame 2. (Quickly) Use Tongs on Bung (in Cask) 3. Talk to Minnow (before he leaves room) - Am here for Selena - Tell Skorl that Cask is Leaking 4. Hide behind back right Pillar 5. Wait. Skorl Guard comes in, sees leaking cask and drinks. Error occurs.
Valgrind output is as follows : ==9090== Thread 1: ==9090== Invalid read of size 2 ==9090== at 0x850847E: Lure::Hotspot::doAction(Lure::Action, Lure::HotspotData*) (hotspots.cpp:1368) ==9090== by 0x85093FC: Lure::Hotspot::doAction() (hotspots.cpp:1239) ==9090== by 0x8509CF8: Lure::HotspotTickHandlers::standardCharacterAnimHandler(Lure::Hotspot&) (hotspots.cpp:2645) ==9090== by 0x850A5DF: Lure::HotspotTickHandlers::jailorAnimHandler(Lure::Hotspot&) (hotspots.cpp:3208) ==9090== by 0x84FDE11: Lure::Hotspot::tick() (hotspots.cpp:477) ==9090== by 0x84FC1E4: Lure::Game::tick() (game.cpp:92) ==9090== by 0x84FC28B: Lure::Game::nextFrame() (game.cpp:125) ==9090== by 0x84FC536: Lure::Game::execute() (game.cpp:182) ==9090== by 0x84CF21F: Lure::LureEngine::go() (lure.cpp:160) ==9090== by 0x84D03B4: Lure::LureEngine::run() (lure.h:102) ==9090== by 0x80565C0: runGame(PluginSubclass<MetaEngine> const*, OSystem&, Common::String const&) (main.cpp:216) ==9090== by 0x8056F69: scummvm_main (main.cpp:389) ==9090== Address 0x611748c is 36 bytes inside a block of size 2,492 free'd ==9090== at 0x4024D9A: operator delete(void*) (in /usr/lib/valgrind/x86-linux/vgpreload_memcheck.so) ==9090== by 0x84DAA45: Common::SharedPtrDeletionImpl<Lure::Hotspot>::~SharedPtrDeletionImpl() (ptr.h:48) ==9090== by 0x84D02B8: Common::SharedPtr<Lure::Hotspot>::decRef() (ptr.h:206) ==9090== by 0x84D02E8: Common::SharedPtr<Lure::Hotspot>::~SharedPtr() (ptr.h:122) ==9090== by 0x84D02FF: Common::ListInternal::Node<Common::SharedPtr<Lure::Hotspot> >::~Node() (list_intern.h:42) ==9090== by 0x84D739B: Common::List<Common::SharedPtr<Lure::Hotspot> >::erase(Common::ListInternal::NodeBase*) (list.h:241) ==9090== by 0x84D741E: Common::List<Common::SharedPtr<Lure::Hotspot> >::erase(Common::ListInternal::Iterator<Common::SharedPtr<Lure::Hotspot> >) (list.h:88) ==9090== by 0x84D3411: Lure::Resources::deactivateHotspot(unsigned short, bool) (res.cpp:680) ==9090== by 0x84E9539: Lure::Script::deactivateHotspot(unsigned short, unsigned short, unsigned short) (scripts.cpp:136) ==9090== by 0x84EA306: Lure::Script::execute(unsigned short) (scripts.cpp:1110) ==9090== by 0x8500EE1: Lure::Hotspot::npcExecScript(Lure::HotspotData*) (hotspots.cpp:2056) ==9090== by 0x8508366: Lure::Hotspot::doAction(Lure::Action, Lure::HotspotData*) (hotspots.cpp:1330)
ScummVM 1.2.0svn48453 (Apr 1 2010 03:32:24) Features compiled in: Vorbis FLAC MP3 ALSA RGB zLib FluidSynth built on Linux x86_32 2.6.31 by GCC 4.3.4
Ticket imported from: #2980384. Ticket imported from: bugs/4844.
Attachments (4)
Change History (13)
by , 15 years ago
comment:1 by , 15 years ago
Tested with : ScummVM 1.2.0svn48657 (Apr 14 2010 11:07:00) Features compiled in: Vorbis FLAC MP3 ALSA RGB zLib FluidSynth
Problem still present.
Added savegame for easier replication. Just wait for Skorl to arrive and drink, though this misses the Hotspot warnings which occur while going to hide...
comment:2 by , 15 years ago
Thanks for the savegame. Initial analysis indicated that the second problem (the warnings) were almost certainly due to at least one town NPC not being correctly disabled when you go to the castle. Not a major problem, and indeed the original (from memory) had the village NPCs still moving around even when you're in the castle. It's just the doorway hotspots are no longer active at that point, and I explicitly added warnings for these kind of things to the ScummVM module.
I need to progress to a point just before going to the castle (or if you have a handy savegame, that would be appreciated too), and I'll see about adding some extra code to disable all the town characters when you enter the castle.
by , 15 years ago
Savegame in Village, just before leaving for castle - Talk to Ewan
comment:3 by , 15 years ago
I didn't have a savegame at this point, but since Lure is fairly quick to play through, I have produced one which has been attached to this bug.
Thanks for attention so far, but though the warnings should be fixed, I'm more interested in fixing the valgrind memory issues associated with the Skorl Gatekeeper.
I'll try to work out what the exact call to hotspots.cpp:1368 is causing the invalid read.
by , 15 years ago
| Attachment: | output-fragment.txt added |
|---|
Debugging Log Fragment - Showing deletion of Hotspot 1011
comment:4 by , 15 years ago
The error is triggered by the Animation of the Skorl Gatekeeper lying down to start drinking.
This is called by Hotspot 1011 tick() which calls a NPC_EXEC_SCRIPT action. The script called, 1e49h then proceeds to call a number of opcodes including an opcode to deactivate Hotspot 1011 i.e. the calling hotspot. The deactivation function also triggers the resource manager to deallocate the hotspot object, which then triggers the issue when the debug call around hotspots.cpp:1368 attempts to access the _hotspotId data element of a deallocated object.
This is based on the attached debugging log fragment.
dreammaster : I'm not familiar enough with the engine structure, to determine exactly why the issue is happening and how to fix it e.g. Is this a script bug present in the original, an issue with the resource manager (destId?), a problem with script opcode implementation. But I hope this helps you work out a fix. Thanks.
comment:5 by , 15 years ago
That helped enormously, thanks. I've committed a change to hotspots.cpp - r48741. If you could verify it fixes the Valgrind warning, I'll then backport it to the 1.1 branch and close this ticket.
comment:6 by , 15 years ago
Tested with r48741 and savegame. No valgrind issues found and visually looks correct. Thanks for this fix.
P.S. The following warnings are still seen. Is this expected? : "WARNING: Hotspot 2725h is not currently active! WARNING: Hotspot 2726h is not currently active!"
comment:7 by , 15 years ago
An earlier fix I added should resolve this problem, by de-activating all the active NPC characters in the village when you enter the castle (previously the game was still moving them around in the background). This means of course, that it won't automatically get applied on any existing savegames from when the player is already inside the castle. Since it's only a warning message, though, I don't consider this any issue for existing savegames.
comment:8 by , 15 years ago
| Status: | new → closed |
|---|
comment:9 by , 15 years ago
| Resolution: | → fixed |
|---|
Savegame for replication