Opened 11 years ago

Closed 11 years ago

#975 closed bug (fixed (in master))

Potion of Dragon Breath can destroy itself and cause double-delete

Reported by: takkaria Owned by:
Milestone: 3.1.2 beta Keywords:

Change History (3)

comment:1 Changed 11 years ago by d_m

  • Status changed from new to confirmed

So it seems like an easy way to fix this bug would be to destroy the "used up" item before calling effect_do(). This is not currently possible because the item is only "used up" if effect_do() returns TRUE, which may have already destroyed the item (in this case).

When looking over the code, it seems like effect_do() mostly returns false when the effect doesn't "work" (e.g. tries to lower a stat that can't be lowered, or corresponds to an undefined effect). I actually do think an item should be "used up" regardless of whether the effect works or not. Also, I think that the "undefined effect" case should crash.

Anyway, I propose fixing this by making effect_do() return void. Then the code to remove a used up potion/scroll/etc can be moved to before effect_do() and the issue will be resolved. Thoughts?

comment:2 Changed 11 years ago by d_m

Wait nevermind, the one important case where this has to work is when a scroll (or staff or whatever) needs to target something and the player decides to cancel targeting.

My change would be more draconian... once you read the scroll, you'd still use it up even if you aborted while choosing an item to enchant, or whatever.

Back to the drawing board...

comment:3 Changed 11 years ago by d_m

  • Resolution set to fixed
  • Status changed from confirmed to closed

Fixed in [fb2f4ea] (SVN r1683). I was able to replicate it and after this fix I wasn't (despite about 20 or so tries).

I have to admit that the object wiping/destruction/deallocation code fills me with fear, which is why the patch is somewhat superstitious; however, it seems to work, and I hit assertions indicating that all three of the things I tested for do occur (sometimes).

Note: See TracTickets for help on using tickets.