[odamex-bug-reporter] [Bug 1195] Client Crash When Rotating a WAD

  • From: odamex-bugtracker@xxxxxxxxxx
  • To: odamex-bug-reporter@xxxxxxxxxxxxx
  • Date: Thu, 27 Oct 2016 00:49:30 +0000

http://odamex.net/bugs/show_bug.cgi?id=1195

Alexander Mayfield <alexmax2742@xxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |alexmax2742@xxxxxxxxx

--- Comment #1 from Alexander Mayfield <alexmax2742@xxxxxxxxx> ---
Created attachment 551
  --> http://odamex.net/bugs/attachment.cgi?id=551&action=edit
NULL NormalLight.next on D_Shutdown

I was able to recreate the issue without a network involved:

- Launch Odamex
- wad anthctf-beta2.wad
- map map01
- wad veloctf.wad
- Load a map or wait for the demo to start.

A patch for this crash is attached.

The details of the renderer are obscure to me.  However, something I discovered
is that there is a global variable called NormalLight of type dyncolormap_t.  A
dyncolormap_t is a linked-list structure, in that the structure contains a
'next' pointer that points to another dyncolormap_t.  Default Doom has a single
one of these structures, located at NormalLight, while some Anthology CTF maps
seemed to generate more on the fly upon loading a map, calling GetSpecialLights
in v_palette.cpp to malloc a new dyncolormap, populate it, and then attach it
to the next pointer, creating a linked-list that can be traversed.

This works great unless you don't do proper bookkeeping on the next pointers. 
However, these dyncolormaps are usually freed all-together using an invocation
of Z_FreeTags() in P_SetupLevel()....except for the first one.  The first one
is always there, located at NormalLight, so you need to NULL the next pointer
at that point to ensure that it is safe.  P_SetupLevel() actually does this
already...

...but that only is called if you're changing levels.  If you're switching WAD
files, those dyncolormaps are freed in D_Shutdown(), using Z_Close().  And we
never NULL the next pointer in NormalLight afterwards.  Thus, the next map you
load will go boom.

A patch is attached that NULL's the pointer properly.  This fixes both my
recreation of the bug and Hex's original bug.

I think there was a code-path where you might have been able to avoid a crash
if the wad you're switching to also generates dyncolormaps.  But I'm not
certain, and in any case it's irrelevant now.

-- 
You are receiving this mail because:
You are the assignee for the bug.

Other related posts: