View Issue Details

IDProjectCategoryView StatusLast Update
0004061The Dark ModCodingpublic14.09.2017 03:08
ReporterSteveL Assigned ToSteveL  
PrioritynormalSeveritycrashReproducibilitysometimes
Status resolvedResolutionfixed 
Product VersionTDM 2.03 
Fixed in VersionTDM 2.03 
Summary0004061: Crash when reloading or shutting map down while glass is falling.
DescriptionidBrittleFracture can cause crashes if the map is shut down (e.g. by a reload) during later phases of the fracture. The crash comes from the parent idPhysics_Static destructor when it's destroying clipmodels.

Plus, there's a performance tweak in idBrittleFracture::RemoveShard which looks suspiciously like it's opened up a memory leak, preventing clipmodel destruction when it meant only to stop a list being resorted.

I'm not sure yet whether these are the same issue.
TagsNo tags attached.

Relationships

related to 0003666 closedSteveL Touching stacked moveables causes a crash 

Activities

SteveL

SteveL

24.01.2015 07:45

reporter   ~0007385

Last edited: 24.01.2015 07:49

It *is* the same problem. There's a temporary clipmodel / memory leak and double-deletions of clipmodels if you shut down the map (or reload) while the glass is falling. Its slightly dependent on the order in which the shards fall but is reproducible 90-something % of the time.

idBrittleFracture uses more than one physics object. A single idPhysics_StaticMulti keeps track of all the clipmodels of the shards that haven't yet fallen. Shards that fall get an idPhysics_Rigidbody of their own. The idBrittleFracture entity keep track of all shards, fallen or not, in its own index. When a shard times out on the floor, its collision model is deleted as its idPhysics_Rigidbody gets destroyed. If the map gets shut down, both types of physics object delete the CMs that they own at that point.

The performance tweak added to idBrittleFracture::RemoveShard sought to save a few bytes being written each frame by using a version of idList::RemoveIndex that doesn't keep items sorted. Code:

    shards.RemoveIndex( index, false ); // Tels: false => don't bother to keep shards sorted
    physicsObj.RemoveIndex( index, false );

Unfortunately the 'false' parameter in idPhysics_StaticMulti::RemoveIndex doesn't do the same thing as it does in as idList::RemoveIndex. In the physics version, it doesn't affect the list ordering, it simply prevents the clipmodel being deleted, before using the "sorted" version of idList::RemoveIndex. This means the two lists of clipmodels -- the one held by the physics object and the one held by the idBrittleFracture entity -- get shuffled. The list of CMs "owned" by the idPhysics_StaticMulti holds the right number of items, but the wrong ones, and in a different order from the list held by the BrittleFracture.

The crash happens because the same clipmodel can end up being owned by both the idPhysics_StaticMulti and by one of the shards on the floor. The shards on the floor delete their clipmodels first, then the idPhysics_StaticMulti tries to do its list.

The situation clears itself up if you let the broken glass effect run to completion and if all the glass happens to fall out, because by then all clipmodels have been deleted and all index slots cleared in the idPhysics_StaticMulti have been nullified, at which point it doesn't matter that they got nullified in the wrong order. But while there are some shards on the floor and some shards in the window, if the map tries to shut down you'll usually get some double-deletions of clipmodels and a crash.

I've restored the original version of the function. The svn history is gone but I've verified the expected fix in doom3 code.

SteveL

SteveL

24.01.2015 07:51

reporter  

fracture.png (15,498 bytes)   
fracture.png (15,498 bytes)   
SteveL

SteveL

24.01.2015 07:57

reporter   ~0007386

NB the memory / clipmodel leak probably isn't significant. Most shards to be deleted will be on the floor so will get deleted by the rigid body physics object, instead of relying on the idPhysics_StaticMulti to do it. The damaging bit of this bug was was the shuffling of indexes meaning that both physics objects could own the same clipmodel at the same time.

At rev 6463

Issue History

Date Modified Username Field Change
24.01.2015 05:26 SteveL New Issue
24.01.2015 05:26 SteveL Status new => assigned
24.01.2015 05:26 SteveL Assigned To => SteveL
24.01.2015 07:45 SteveL Note Added: 0007385
24.01.2015 07:46 SteveL Summary idBrittleFracture has problems => Crash when reloading or shutting map down while glass is falling.
24.01.2015 07:49 SteveL Note Edited: 0007385
24.01.2015 07:51 SteveL File Added: fracture.png
24.01.2015 07:57 SteveL Note Added: 0007386
24.01.2015 07:57 SteveL Status assigned => resolved
24.01.2015 07:57 SteveL Fixed in Version => TDM 2.04
24.01.2015 07:57 SteveL Resolution open => fixed
31.01.2015 18:46 SteveL Fixed in Version TDM 2.04 => TDM 2.03
14.09.2017 03:08 nbohr1more Relationship added related to 0003666