Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#4811 closed defect (fixed)

NIPPON: Assertion at Dough's House (regression)

Reported by: raziel- Owned by: peres
Priority: high Component: Engine: Parallaction
Keywords: Cc:
Game: Nippon Safes

Description

Features compiled in: Vorbis FLAC MP3 RGB zLib

When Dough is holding the letter from the mailbox to read it i get an assertion "label < label.size()" failed: file "engines/parallaction/graphics.cpp", line 565
No save position due to bug item #2969234

Nippon Safes Inc. (Multi-lingual/Amiga)

Amiga OS4
gcc (GCC) 4.2.4 (adtools build 20090118)

Ticket imported from: #2969257. Ticket imported from: bugs/4811.

Attachments (3)

nippon.000 (156 bytes) - added by raziel- 9 years ago.
Open Mailbox, take letter, crash :-)
nippon.005 (341 bytes) - added by raziel- 9 years ago.
Walk inside "Lucky Look" and wave around the poster
labels.patch (11.0 KB) - added by peres 9 years ago.
A rough solution to the problem

Download all attachments as: .zip

Change History (18)

comment:1 Changed 9 years ago by jvprat

Owner: set to peres

Changed 9 years ago by raziel-

Attachment: nippon.000 added

Open Mailbox, take letter, crash :-)

comment:2 Changed 9 years ago by raziel-

The very same happens when Donna is holding the Suzy Yong poster inside "Lucky Look"

Changed 9 years ago by raziel-

Attachment: nippon.005 added

Walk inside "Lucky Look" and wave around the poster

comment:3 Changed 9 years ago by wjp

The label rendered for the text "examine the poster"' is freed when entering "Lucky Look" (in changeLocation, even two or three times). In fact all inventory labels are destroyed when changing locations, but it only triggers asserts when you have more inventory items than there are labels in the new room. (Since the label IDs are restarted at 0.)

comment:4 Changed 9 years ago by Kirben

Summary: NIPPON: Assertion at Dough's HouseNIPPON: Assertion at Dough's House (regression)

comment:5 Changed 9 years ago by Kirben

This is another regression, which was specifically caused by revision 32873.

comment:6 Changed 9 years ago by Kirben

Priority: normalhigh

Changed 9 years ago by peres

Attachment: labels.patch added

A rough solution to the problem

comment:7 Changed 9 years ago by peres

I attached a patch to fix the issue, by letting the zones to whom the label belongs to manage their lifecycle.

I tried both the attached savegames and I got no crashes, so at least the bug is patched. What I could not do is checking for memory leaks, that are likely to be.

comment:8 Changed 9 years ago by wjp

Nice work. This fixes the crashes for me too.

Looking over the patch, I think it would leak the ad-hoc created labels for things like the test result screen. (Or at least I do not see where the labels created there are freed.)

A quick valgrind session shows label leaks on the language screen.

comment:9 Changed 9 years ago by fingolfin

What is the status of this item?

comment:10 Changed 9 years ago by peres

I committed an enhanced version of the patch on r48811. I could not perform any leak checking with valgrind, and won't be able until I go back home in 10 days, so I did not backport it to the release branch.

comment:11 Changed 9 years ago by peres

This bug has been fixed in revision 48811 and 48827. The patch has been backported to the release branch of version 1.1.1 in r48828. The leaks have been plugged, except for the labels allocated by _c_testResults, which is invoked at most once per run.

Should this be closed or just priority lowered?

comment:12 Changed 9 years ago by fingolfin

Resolution: fixed
Status: newclosed

comment:13 Changed 9 years ago by wjp

r48837 fixes the memory leaks.

comment:14 Changed 9 years ago by wjp

Resolution: fixed

comment:15 Changed 9 years ago by wjp

Resolution: fixed
Note: See TracTickets for help on using tickets.