From 080e769c768241d55ed8b847647637baaad09c0e Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Fri, 15 Jul 2011 13:26:36 +0000 Subject: [PATCH] - removed all asserts from the interpolation objects' Destroy methods. The condition will not be true after a failed savegame write occured. - fixed: D_ErrorCleanup must clear 'savegamerestore'. - fixed: Cleaning up when loading a savegame failed while restoring the thinker list did not work. There were two issues: * removed the asserts in GC::SweepList because they get triggered by thinkers that were not fully initialized during loading. * AActor::UnlinkFromWorld may not assume that the sector list has been initialized when this function is called. SVN r3274 (trunk) --- src/d_main.cpp | 1 + src/dobjgc.cpp | 6 ++++-- src/dthinker.cpp | 13 ++---------- src/p_maputl.cpp | 40 ++++++++++++++++++++---------------- src/r_data/r_interpolate.cpp | 6 ------ 5 files changed, 29 insertions(+), 37 deletions(-) diff --git a/src/d_main.cpp b/src/d_main.cpp index 62f58d5b3..a90a813f4 100644 --- a/src/d_main.cpp +++ b/src/d_main.cpp @@ -912,6 +912,7 @@ void D_Display () void D_ErrorCleanup () { + savegamerestore = false; screen->Unlock (); bglobal.RemoveAllBots (true); D_QuitNetGame (); diff --git a/src/dobjgc.cpp b/src/dobjgc.cpp index 6fae559c3..ba69f58f7 100644 --- a/src/dobjgc.cpp +++ b/src/dobjgc.cpp @@ -242,8 +242,10 @@ static DObject **SweepList(DObject **p, size_t count, size_t *finalize_count) // be in a thinker list, then I need to add write barriers for every time a // thinker pointer is changed. This seems easier and perfectly reasonable, since // a live thinker that isn't on a thinker list isn't much of a thinker. - assert(!curr->IsKindOf(RUNTIME_CLASS(DThinker)) || (curr->ObjectFlags & OF_Sentinel)); - assert(!curr->IsKindOf(RUNTIME_CLASS(DInterpolation))); + + // However, this can happen during deletion of the thinker list while cleaning up + // from a savegame error so we can't assume that any thinker that gets here is an error. + curr->Destroy(); } curr->ObjectFlags |= OF_Cleanup; diff --git a/src/dthinker.cpp b/src/dthinker.cpp index a4f4db27e..85c5fb4bc 100644 --- a/src/dthinker.cpp +++ b/src/dthinker.cpp @@ -192,19 +192,10 @@ void DThinker::SerializeAll(FArchive &arc, bool hubLoad) statcount--; } } - catch (class CDoomError &err) + catch (class CDoomError &) { bSerialOverride = false; - - // DestroyAllThinkers cannot be called here. It will try to delete the corrupted - // object table left behind by the serializer and crash. - // Trying to continue is not an option here because the garbage collector will - // crash the next time it runs. - // Even making this a fatal error will crash but at least the message can be seen - // before the crash - which is not the case with all other options. - - //DestroyAllThinkers(); - I_FatalError("%s", err.GetMessage()); + DestroyAllThinkers(); throw; } bSerialOverride = false; diff --git a/src/p_maputl.cpp b/src/p_maputl.cpp index 9cd2a34b8..91c0e1b7c 100644 --- a/src/p_maputl.cpp +++ b/src/p_maputl.cpp @@ -246,26 +246,30 @@ void AActor::UnlinkFromWorld () // pointers, allows head node pointers to be treated like everything else AActor **prev = sprev; AActor *next = snext; - if ((*prev = next)) // unlink from sector list - next->sprev = prev; - snext = NULL; - sprev = (AActor **)(size_t)0xBeefCafe; // Woo! Bug-catching value! - // phares 3/14/98 - // - // Save the sector list pointed to by touching_sectorlist. - // In P_SetThingPosition, we'll keep any nodes that represent - // sectors the Thing still touches. We'll add new ones then, and - // delete any nodes for sectors the Thing has vacated. Then we'll - // put it back into touching_sectorlist. It's done this way to - // avoid a lot of deleting/creating for nodes, when most of the - // time you just get back what you deleted anyway. - // - // If this Thing is being removed entirely, then the calling - // routine will clear out the nodes in sector_list. + if (prev != NULL) // prev will be NULL if this actor gets deleted due to cleaning up from a broken savegame + { + if ((*prev = next)) // unlink from sector list + next->sprev = prev; + snext = NULL; + sprev = (AActor **)(size_t)0xBeefCafe; // Woo! Bug-catching value! - sector_list = touching_sectorlist; - touching_sectorlist = NULL; //to be restored by P_SetThingPosition + // phares 3/14/98 + // + // Save the sector list pointed to by touching_sectorlist. + // In P_SetThingPosition, we'll keep any nodes that represent + // sectors the Thing still touches. We'll add new ones then, and + // delete any nodes for sectors the Thing has vacated. Then we'll + // put it back into touching_sectorlist. It's done this way to + // avoid a lot of deleting/creating for nodes, when most of the + // time you just get back what you deleted anyway. + // + // If this Thing is being removed entirely, then the calling + // routine will clear out the nodes in sector_list. + + sector_list = touching_sectorlist; + touching_sectorlist = NULL; //to be restored by P_SetThingPosition + } } if (!(flags & MF_NOBLOCKMAP)) diff --git a/src/r_data/r_interpolate.cpp b/src/r_data/r_interpolate.cpp index 5655adf20..3ca3b557c 100644 --- a/src/r_data/r_interpolate.cpp +++ b/src/r_data/r_interpolate.cpp @@ -406,12 +406,10 @@ void DSectorPlaneInterpolation::Destroy() { if (ceiling) { - assert(sector->interpolations[sector_t::CeilingMove] == this); sector->interpolations[sector_t::CeilingMove] = NULL; } else { - assert(sector->interpolations[sector_t::FloorMove] == this); sector->interpolations[sector_t::FloorMove] = NULL; } @@ -570,12 +568,10 @@ void DSectorScrollInterpolation::Destroy() { if (ceiling) { - assert(sector->interpolations[sector_t::CeilingScroll] == this); sector->interpolations[sector_t::CeilingScroll] = NULL; } else { - assert(sector->interpolations[sector_t::FloorScroll] == this); sector->interpolations[sector_t::FloorScroll] = NULL; } Super::Destroy(); @@ -661,7 +657,6 @@ DWallScrollInterpolation::DWallScrollInterpolation(side_t *_side, int _part) void DWallScrollInterpolation::Destroy() { - assert(side->textures[part].interpolation == this); side->textures[part].interpolation = NULL; Super::Destroy(); } @@ -746,7 +741,6 @@ DPolyobjInterpolation::DPolyobjInterpolation(FPolyObj *po) void DPolyobjInterpolation::Destroy() { - assert(poly->interpolation == this); poly->interpolation = NULL; Super::Destroy(); }