From 26fe356102e89f467c7626974f9184bd2a6111a0 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Tue, 23 Jun 2026 12:46:01 -0500 Subject: [PATCH] fix remaining issues with doom; ensure music plays on subsequent launches; ensure different doom titles can be launched back to back many times without crash or memory pressure --- components/doom/prboom/am_map.c | 9 ++++++ components/doom/prboom/dbopl.c | 11 +++++--- components/doom/prboom/g_game.c | 21 ++++++++++---- components/doom/prboom/opl.c | 12 ++++++++ components/doom/prboom/p_enemy.c | 9 ++++++ components/doom/prboom/p_map.c | 8 ++++++ components/doom/prboom/p_maputl.c | 14 ++++++++- components/doom/prboom/p_setup.c | 9 ++++++ components/doom/prboom/r_bsp.c | 6 ++++ components/doom/prboom/r_fps.c | 12 ++++++++ components/doom/prboom/r_main.c | 8 ++++++ components/doom/prboom/r_plane.c | 16 +++++++++++ components/doom/prboom/r_things.c | 7 +++++ components/doom/prboom/s_sound.c | 6 ++++ components/doom/prboom/w_wad.c | 21 ++++++++++++++ components/doom/src/doom.cpp | 47 ++++++++++++++++++++++++++++++- 16 files changed, 205 insertions(+), 11 deletions(-) diff --git a/components/doom/prboom/am_map.c b/components/doom/prboom/am_map.c index 3ab48c3e..fb89558f 100644 --- a/components/doom/prboom/am_map.c +++ b/components/doom/prboom/am_map.c @@ -248,6 +248,15 @@ mpoint_t *markpoints = NULL; // where the points are int markpointnum = 0; // next point to be assigned (also number of points now) int markpointnum_max = 0; // killough 2/22/98 +// Reset the growable automap-marks array on emulator teardown (see +// R_ResetDrawSegs). The array and its count/capacity counters survive across +// launches; dropping them forces reallocation instead of reusing freed memory. +void AM_ResetMarks(void) { + markpoints = NULL; + markpointnum = 0; + markpointnum_max = 0; +} + static boolean stopped = true; // diff --git a/components/doom/prboom/dbopl.c b/components/doom/prboom/dbopl.c index f2864ef8..d624021a 100644 --- a/components/doom/prboom/dbopl.c +++ b/components/doom/prboom/dbopl.c @@ -1432,14 +1432,17 @@ void Chip__Setup(Chip *self, Bit32u rate ) { } } -static int doneTables = FALSE; void DBOPL_InitTables( void ) { int i, oct; Chip *chip = NULL; - if ( doneTables ) - return; - doneTables = TRUE; + // NOTE: no "already done" guard here. The wave/volume tables below live in + // buffers that are shared_malloc'd fresh (and zeroed) on every emulator + // launch, so they MUST be repopulated each launch. A static doneTables guard + // used to skip this on relaunch, leaving the tables all-zero -> the OPL synth + // produced pure silence (music dead on the 2nd+ launch; SFX are PCM and + // unaffected). DBOPL_InitTables is only called once per launch (from + // OPL_Init), so unconditional init is correct and cheap. #if ( DBOPL_WAVE == WAVE_HANDLER ) || ( DBOPL_WAVE == WAVE_TABLELOG ) //Exponential volume table, same as the real adlib for ( i = 0; i < 256; i++ ) { diff --git a/components/doom/prboom/g_game.c b/components/doom/prboom/g_game.c index 972910b6..9e5b7c54 100644 --- a/components/doom/prboom/g_game.c +++ b/components/doom/prboom/g_game.c @@ -252,6 +252,18 @@ int defaultskill; //note 1-based // killough 2/8/98: make corpse queue variable in size int bodyqueslot, bodyquesize; // killough 2/8/98 mobj_t **bodyque = 0; // phares 8/10/98 +// Hoisted from G_CheckSpot() to file scope so G_ResetBodyQueue can clear it on +// emulator teardown. +static int bodyque_queuesize; + +// Reset the growable corpse queue on emulator teardown (see R_ResetDrawSegs). +// `bodyque` and its size/slot counters survive across launches; dropping them +// forces G_CheckSpot to reallocate instead of reusing freed memory. +void G_ResetBodyQueue(void) { + bodyque = NULL; + bodyqueslot = 0; + bodyque_queuesize = 0; +} static const byte* G_ReadDemoHeader(const byte* demo_p, size_t size, boolean failonerror); @@ -1031,13 +1043,12 @@ static boolean G_CheckSpot(int playernum, mapthing_t *mthing) if (bodyquesize > 0) { - static int queuesize; - if (queuesize < bodyquesize) + if (bodyque_queuesize < bodyquesize) { bodyque = realloc(bodyque, bodyquesize*sizeof*bodyque); - memset(bodyque+queuesize, 0, - (bodyquesize-queuesize)*sizeof*bodyque); - queuesize = bodyquesize; + memset(bodyque+bodyque_queuesize, 0, + (bodyquesize-bodyque_queuesize)*sizeof*bodyque); + bodyque_queuesize = bodyquesize; } if (bodyqueslot >= bodyquesize) P_RemoveMobj(bodyque[bodyqueslot % bodyquesize]); diff --git a/components/doom/prboom/opl.c b/components/doom/prboom/opl.c index 54947355..4a4d2609 100644 --- a/components/doom/prboom/opl.c +++ b/components/doom/prboom/opl.c @@ -97,6 +97,18 @@ static opl_timer_t timer2 = { 3125, 0, 0, 0 }; int OPL_Init (unsigned int rate) { + // Free any buffers left over from a previous init. The emulator re-launches + // without calling OPL_Shutdown (so music keeps working on relaunch), so + // OPL_Init must release the old callback queue / mix buffer itself or they + // leak ~(sample_rate * 4) bytes every launch. + if (callback_queue) + { + OPL_Queue_Destroy(callback_queue); + callback_queue = NULL; + } + free(mix_buffer); + mix_buffer = NULL; + opl_sample_rate = rate; opl_paused = 0; pause_offset = 0; diff --git a/components/doom/prboom/p_enemy.c b/components/doom/prboom/p_enemy.c index 6a2ce84d..31d44179 100644 --- a/components/doom/prboom/p_enemy.c +++ b/components/doom/prboom/p_enemy.c @@ -2300,6 +2300,15 @@ mobj_t **braintargets; int numbraintargets_alloc; int numbraintargets; +// Reset the growable brain-target array on emulator teardown (see +// R_ResetDrawSegs). The array and its alloc/count counters survive across +// launches; dropping them forces reallocation instead of reusing freed memory. +void P_ResetBrainTargets(void) { + braintargets = NULL; + numbraintargets_alloc = 0; + numbraintargets = 0; +} + struct brain_s brain; // killough 3/26/98: global state of boss brain // killough 3/26/98: initialize icon landings at level startup, diff --git a/components/doom/prboom/p_map.c b/components/doom/prboom/p_map.c index 52994d88..023e4638 100644 --- a/components/doom/prboom/p_map.c +++ b/components/doom/prboom/p_map.c @@ -87,6 +87,14 @@ static int spechit_max; // killough int numspechit; +// Reset the growable spechit array on emulator teardown (see R_ResetDrawSegs). +// `spechit` is zone/heap-backed and only grown when spechit_max is exceeded; +// dropping the pointer and size forces reallocation on the next launch. +void P_ResetSpechit(void) { + spechit = NULL; + spechit_max = 0; +} + // Temporary holder for thing_sectorlist threads msecnode_t* sector_list = NULL; // phares 3/16/98 diff --git a/components/doom/prboom/p_maputl.c b/components/doom/prboom/p_maputl.c index 62ce91c9..4c581747 100644 --- a/components/doom/prboom/p_maputl.c +++ b/components/doom/prboom/p_maputl.c @@ -408,11 +408,23 @@ boolean P_BlockThingsIterator(int x, int y, boolean func(mobj_t*)) // 1/11/98 killough: Intercept limit removed static intercept_t *intercepts, *intercept_p; +// Hoisted from check_intercept() to file scope so P_ResetIntercepts can clear +// it on emulator teardown. +static size_t num_intercepts; + +// Reset the growable intercepts array on emulator teardown (see +// R_ResetDrawSegs). The array and its size counter survive across launches; +// dropping them forces check_intercept to reallocate instead of reusing freed +// memory. +void P_ResetIntercepts(void) { + intercepts = NULL; + intercept_p = NULL; + num_intercepts = 0; +} // Check for limit and double size if necessary -- killough static void check_intercept(void) { - static size_t num_intercepts; size_t offset = intercept_p - intercepts; if (offset >= num_intercepts) { diff --git a/components/doom/prboom/p_setup.c b/components/doom/prboom/p_setup.c index 8fbcd7f5..6b239f8a 100644 --- a/components/doom/prboom/p_setup.c +++ b/components/doom/prboom/p_setup.c @@ -162,6 +162,15 @@ size_t num_deathmatchstarts; // killough mapthing_t *deathmatch_p; mapthing_t playerstarts[MAXPLAYERS]; +// Reset the growable deathmatch-starts array on emulator teardown (see +// R_ResetDrawSegs). The array, its cursor and its count survive across +// launches; dropping them forces reallocation instead of reusing freed memory. +void P_ResetMapStarts(void) { + deathmatchstarts = NULL; + deathmatch_p = NULL; + num_deathmatchstarts = 0; +} + // // P_CheckForZDoomNodes // diff --git a/components/doom/prboom/r_bsp.c b/components/doom/prboom/r_bsp.c index 898e3848..c30fc160 100644 --- a/components/doom/prboom/r_bsp.c +++ b/components/doom/prboom/r_bsp.c @@ -56,6 +56,12 @@ drawseg_t *drawsegs; unsigned maxdrawsegs; // drawseg_t drawsegs[MAXDRAWSEGS]; // old code -- killough +// Reset the pool-backed drawseg array. Z_Close() frees the pool on emulator +// teardown, so on a re-launch these stale statics must be cleared or the grow +// guard (ds_p == drawsegs+maxdrawsegs) skips realloc and writes through freed +// pointers into reused pool memory (use-after-free / heap corruption). +void R_ResetDrawSegs(void) { drawsegs = NULL; maxdrawsegs = 0; } + // // R_ClearDrawSegs // diff --git a/components/doom/prboom/r_fps.c b/components/doom/prboom/r_fps.c index 5d9e0d49..b2759561 100644 --- a/components/doom/prboom/r_fps.c +++ b/components/doom/prboom/r_fps.c @@ -231,6 +231,18 @@ void R_UpdateInterpolations() int interpolations_max = 0; +// Reset the libc-heap-backed interpolation arrays on emulator teardown (see +// R_ResetDrawSegs). The growable arrays and their size/count statics survive +// across launches; dropping them forces R_SetInterpolation to reallocate +// instead of writing through stale pointers. +void R_ResetInterpolations(void) { + oldipos = NULL; + bakipos = NULL; + curipos = NULL; + interpolations_max = 0; + numinterpolations = 0; +} + static void R_SetInterpolation(interpolation_type_e type, void *posptr) { int i; diff --git a/components/doom/prboom/r_main.c b/components/doom/prboom/r_main.c index e1ead406..97d0207b 100644 --- a/components/doom/prboom/r_main.c +++ b/components/doom/prboom/r_main.c @@ -93,6 +93,14 @@ angle_t clipangle; int *viewangletox; +// Reset pool/shared-backed view mapping state on emulator teardown (see +// R_ResetDrawSegs). `viewangletox` is zone-allocated and only re-allocated by +// R_InitTextureMapping when NULL; without dropping the pointer the next launch +// reuses memory freed by Z_Close() (use-after-free). +void R_ResetViewMapping(void) { + viewangletox = NULL; +} + // The xtoviewangleangle[] table maps a screen pixel // to the lowest viewangle that maps back to x ranges // from clipangle to -clipangle. diff --git a/components/doom/prboom/r_plane.c b/components/doom/prboom/r_plane.c index 13918e62..7968fb80 100644 --- a/components/doom/prboom/r_plane.c +++ b/components/doom/prboom/r_plane.c @@ -75,6 +75,22 @@ visplane_t *floorplane, *ceilingplane; size_t maxopenings; int *openings,*lastopening; // dropoff overflow +// Reset pool/shared-backed plane state on emulator teardown (see +// R_ResetDrawSegs). `openings` is pool-backed; the visplane freelist statics +// `freetail`/`freehead` point into the zone/shared visplanes that Z_Close() and +// shared_mem_clear() free -- if not reset, R_ClearPlanes on the next launch +// writes through a stale `freehead` and new_visplane() derefs a stale +// `freetail` (LoadProhibited). `visplanes` itself is re-allocated fresh each +// launch by doom_init_shared_memory(), so only the pointer is dropped here. +void R_ResetPlanes(void) { + openings = NULL; lastopening = NULL; maxopenings = 0; + freetail = NULL; + freehead = &freetail; + visplanes = NULL; + floorplane = NULL; + ceilingplane = NULL; +} + // Clip values are the solid pixel bounding the range. // floorclip starts out SCREENHEIGHT // ceilingclip starts out -1 diff --git a/components/doom/prboom/r_things.c b/components/doom/prboom/r_things.c index ab514530..0c109a6c 100644 --- a/components/doom/prboom/r_things.c +++ b/components/doom/prboom/r_things.c @@ -261,6 +261,13 @@ static void R_InitSpriteDefs(const char * const * namelist) static vissprite_t *vissprites, **vissprite_ptrs; // killough static size_t num_vissprite, num_vissprite_alloc, num_vissprite_ptrs; +// Reset the pool-backed vissprite arrays (see R_ResetDrawSegs): cleared on +// emulator teardown so a re-launch reallocates instead of reusing freed memory. +void R_ResetVisSprites(void) { + vissprites = NULL; vissprite_ptrs = NULL; + num_vissprite = num_vissprite_alloc = num_vissprite_ptrs = 0; +} + // // R_InitSprites // Called at program start. diff --git a/components/doom/prboom/s_sound.c b/components/doom/prboom/s_sound.c index fcc64d80..42f1c6fa 100644 --- a/components/doom/prboom/s_sound.c +++ b/components/doom/prboom/s_sound.c @@ -143,6 +143,12 @@ void S_Init(int sfxVolume, int musicVolume) S_music[i].lumpnum = W_CheckNumForName(namebuf); } mus_paused = 0; + // Reset the "currently playing" pointer. It is a static that survives an + // emulator re-launch, and because S_music is re-allocated to the same + // address each launch, a leftover value makes S_ChangeMusic() think the + // track is already playing and skip it -> silent music on every launch + // after the first. + mus_playing = NULL; } if (sfx_enabled || mus_enabled) diff --git a/components/doom/prboom/w_wad.c b/components/doom/prboom/w_wad.c index 223dd779..3d0775f3 100644 --- a/components/doom/prboom/w_wad.c +++ b/components/doom/prboom/w_wad.c @@ -369,6 +369,27 @@ int W_GetNumForName (const char* name) // killough -- const added // does override all earlier ones. // +// Release WAD resources on teardown. The wad handles are libc FILE* that are +// NOT freed by Z_Close(), so without this they leak on every Doom re-launch. +// NOTE: do NOT free(lumpinfo) here -- this TU pulls in z_zone.h's allocator +// macros transitively (via doomstat.h -> doomdef.h), so lumpinfo is zone-backed +// and already freed by Z_Close(); freeing it again triggers +// "Z_Free: freed a pointer without ZONEID". We only drop the now-stale pointer. +void W_Done(void) +{ + for (size_t i = 0; i < numwadfiles; i++) + { + if (wadfiles[i].handle) + { + fclose((FILE *)wadfiles[i].handle); + wadfiles[i].handle = NULL; + } + } + numwadfiles = 0; + lumpinfo = NULL; // freed by Z_Close(); just clear the dangling pointer + numlumps = 0; +} + void W_Init(void) { lumpinfo = NULL; diff --git a/components/doom/src/doom.cpp b/components/doom/src/doom.cpp index c2f3ebec..da3f75a8 100644 --- a/components/doom/src/doom.cpp +++ b/components/doom/src/doom.cpp @@ -657,13 +657,58 @@ std::span get_doom_video_buffer() { return frame; } +// Defined in prboom's r_bsp.c / r_plane.c / r_things.c. These reset the +// pool-backed growable render arrays (drawsegs / openings / vissprites) whose +// size-counter statics survive Z_Close(); without resetting them a re-launch +// reuses pool memory freed on teardown (use-after-free -> heap corruption). +extern "C" { + void R_ResetDrawSegs(void); + void R_ResetPlanes(void); + void R_ResetVisSprites(void); + // Additional growable arrays / alloc-once buffers whose pointers and size + // counters survive Z_Close(); reset them so the next launch reallocates + // instead of dereferencing freed pool/heap memory (use-after-free). + void R_ResetViewMapping(void); // r_main.c : viewangletox + void R_ResetInterpolations(void);// r_fps.c : oldipos/bakipos/curipos + void P_ResetSpechit(void); // p_map.c : spechit + void P_ResetIntercepts(void); // p_maputl.c : intercepts + void P_ResetMapStarts(void); // p_setup.c : deathmatchstarts + void G_ResetBodyQueue(void); // g_game.c : bodyque + void P_ResetBrainTargets(void); // p_enemy.c : braintargets + void AM_ResetMarks(void); // am_map.c : markpoints + // (DEH support is #if 0'd out in d_deh.c, so its buffers are never allocated + // and need no reset.) + void W_Done(void); // close WAD file handles + free lumpinfo (libc-heap leak) +} + void deinit_doom() { - // stop the audio task + // stop the audio task (the only consumer of the OPL chip / mix buffer) audio_task.reset(); + // NOTE: we intentionally do NOT call music_player->shutdown() here. Doing so + // left the OPL player unable to restart, so music was silent on every launch + // after the first. The OPL mix_buffer / callback-queue leak it was meant to + // fix is now handled inside OPL_Init() (it frees its old buffers before + // re-allocating), so re-launching keeps music working and doesn't leak. // End display I_EndDisplay(); // Free memory Z_Close(); + // Z_Close() freed the pool; clear the prboom render-array statics so the + // next launch reallocates them instead of reusing freed pointers. + R_ResetDrawSegs(); + R_ResetPlanes(); + R_ResetVisSprites(); + R_ResetViewMapping(); + R_ResetInterpolations(); + P_ResetSpechit(); + P_ResetIntercepts(); + P_ResetMapStarts(); + G_ResetBodyQueue(); + P_ResetBrainTargets(); + AM_ResetMarks(); + // Close WAD file handles and free the libc-heap lumpinfo table (~72 KB), + // neither of which Z_Close() touches -- otherwise they leak every launch. + W_Done(); // reset audio state BoxEmu::get().audio_sample_rate(48000); shared_mem_clear();