diff --git a/src/dobject.cpp b/src/dobject.cpp index 63c7bb688d..68f3aab8ec 100644 --- a/src/dobject.cpp +++ b/src/dobject.cpp @@ -301,42 +301,17 @@ DObject::~DObject () PClass *type = GetClass(); if (!(ObjectFlags & OF_Cleanup) && !PClass::bShutdown) { - DObject **probe; - - if (!(ObjectFlags & OF_YesReallyDelete)) + if (!(ObjectFlags & (OF_YesReallyDelete|OF_Released))) { Printf("Warning: '%s' is freed outside the GC process.\n", type != NULL ? type->TypeName.GetChars() : "==some object=="); } - // Find all pointers that reference this object and NULL them. - StaticPointerSubstitution(this, NULL); - - // Now unlink this object from the GC list. - for (probe = &GC::Root; *probe != NULL; probe = &((*probe)->ObjNext)) + if (!(ObjectFlags & OF_Released)) { - if (*probe == this) - { - *probe = ObjNext; - if (&ObjNext == GC::SweepPos) - { - GC::SweepPos = probe; - } - break; - } - } - - // If it's gray, also unlink it from the gray list. - if (this->IsGray()) - { - for (probe = &GC::Gray; *probe != NULL; probe = &((*probe)->GCNext)) - { - if (*probe == this) - { - *probe = GCNext; - break; - } - } + // Find all pointers that reference this object and NULL them. + StaticPointerSubstitution(this, NULL); + Release(); } } @@ -347,6 +322,41 @@ DObject::~DObject () } } +void DObject::Release() +{ + DObject **probe; + + // Unlink this object from the GC list. + for (probe = &GC::Root; *probe != NULL; probe = &((*probe)->ObjNext)) + { + if (*probe == this) + { + *probe = ObjNext; + if (&ObjNext == GC::SweepPos) + { + GC::SweepPos = probe; + } + break; + } + } + + // If it's gray, also unlink it from the gray list. + if (this->IsGray()) + { + for (probe = &GC::Gray; *probe != NULL; probe = &((*probe)->GCNext)) + { + if (*probe == this) + { + *probe = GCNext; + break; + } + } + } + ObjNext = nullptr; + GCNext = nullptr; + ObjectFlags |= OF_Released; +} + //========================================================================== // // diff --git a/src/dobject.h b/src/dobject.h index 7c2fd4c224..82051e3bd7 100644 --- a/src/dobject.h +++ b/src/dobject.h @@ -468,6 +468,9 @@ public: Class = NULL; } + // Releases the object from the GC, letting the caller care of any maintenance. + void Release(); + // For catching Serialize functions in derived classes // that don't call their base class. void CheckIfSerialized () const; diff --git a/src/dobjgc.cpp b/src/dobjgc.cpp index ca57e6aca0..e6ff9c734f 100644 --- a/src/dobjgc.cpp +++ b/src/dobjgc.cpp @@ -277,7 +277,9 @@ static DObject **SweepList(DObject **p, size_t count, size_t *finalize_count) void Mark(DObject **obj) { DObject *lobj = *obj; - if (lobj != NULL) + + assert(lobj == nullptr || !(lobj->ObjectFlags & OF_Released)); + if (lobj != nullptr && !(lobj->ObjectFlags & OF_Released)) { if (lobj->ObjectFlags & OF_EuthanizeMe) { @@ -551,6 +553,8 @@ void Barrier(DObject *pointing, DObject *pointed) assert(pointing == NULL || (pointing->IsBlack() && !pointing->IsDead())); assert(pointed->IsWhite() && !pointed->IsDead()); assert(State != GCS_Finalize && State != GCS_Pause); + assert(!(pointed->ObjectFlags & OF_Released)); // if a released object gets here, something must be wrong. + if (pointed->ObjectFlags & OF_Released) return; // don't do anything with non-GC'd objects. // The invariant only needs to be maintained in the propagate state. if (State == GCS_Propagate) { diff --git a/src/dobjtype.cpp b/src/dobjtype.cpp index 3c556da3bc..f0e7b2fc1c 100644 --- a/src/dobjtype.cpp +++ b/src/dobjtype.cpp @@ -185,18 +185,6 @@ PType::~PType() { } -//========================================================================== -// -// PType :: PropagateMark -// -//========================================================================== - -size_t PType::PropagateMark() -{ - size_t marked = Symbols.MarkSymbols(); - return marked + Super::PropagateMark(); -} - //========================================================================== // // PType :: WriteValue @@ -2413,7 +2401,7 @@ PField *PStruct::AddField(FName name, PType *type, DWORD flags) if (Symbols.AddSymbol(field) == nullptr) { // name is already in use - delete field; + field->Destroy(); return nullptr; } Fields.Push(field); @@ -2444,18 +2432,6 @@ PField *PStruct::AddNativeField(FName name, PType *type, size_t address, DWORD f return field; } -//========================================================================== -// -// PStruct :: PropagateMark -// -//========================================================================== - -size_t PStruct::PropagateMark() -{ - GC::MarkArray(Fields); - return Fields.Size() * sizeof(void*) + Super::PropagateMark(); -} - //========================================================================== // // NewStruct diff --git a/src/dobjtype.h b/src/dobjtype.h index cfae5e1694..6c07b047bc 100644 --- a/src/dobjtype.h +++ b/src/dobjtype.h @@ -171,8 +171,6 @@ public: const char *DescriptiveName() const; - size_t PropagateMark(); - static void StaticInit(); }; @@ -520,8 +518,6 @@ public: virtual PField *AddField(FName name, PType *type, DWORD flags=0); virtual PField *AddNativeField(FName name, PType *type, size_t address, DWORD flags = 0, int bitvalue = 0); - size_t PropagateMark(); - void WriteValue(FSerializer &ar, const char *key,const void *addr) const override; bool ReadValue(FSerializer &ar, const char *key,void *addr) const override; void SetDefaultValue(void *base, unsigned offset, TArray *specials) const override; diff --git a/src/scripting/backend/codegen.cpp b/src/scripting/backend/codegen.cpp index a3dc066e98..910dba19f3 100644 --- a/src/scripting/backend/codegen.cpp +++ b/src/scripting/backend/codegen.cpp @@ -6558,23 +6558,10 @@ FxStackVariable::FxStackVariable(PType *type, int offset, const FScriptPosition FxStackVariable::~FxStackVariable() { - // Q: Is this good or bad? Needs testing if this is fine or better left to the GC anyway. DObject's destructor is anything but cheap. membervar->ObjectFlags |= OF_YesReallyDelete; delete membervar; } -//========================================================================== -// -// -//========================================================================== - -void FxStackVariable::ReplaceField(PField *newfield) -{ - membervar->ObjectFlags |= OF_YesReallyDelete; - delete membervar; - membervar = newfield; -} - //========================================================================== // // diff --git a/src/scripting/backend/codegen.h b/src/scripting/backend/codegen.h index c64ec20941..abbcbd54e3 100644 --- a/src/scripting/backend/codegen.h +++ b/src/scripting/backend/codegen.h @@ -1417,7 +1417,6 @@ class FxStackVariable : public FxMemberBase public: FxStackVariable(PType *type, int offset, const FScriptPosition&); ~FxStackVariable(); - void ReplaceField(PField *newfield); FxExpression *Resolve(FCompileContext&); bool RequestAddress(FCompileContext &ctx, bool *writable); ExpEmit Emit(VMFunctionBuilder *build); diff --git a/src/scripting/symbols.cpp b/src/scripting/symbols.cpp index 68b9d522c9..b6adb33e76 100644 --- a/src/scripting/symbols.cpp +++ b/src/scripting/symbols.cpp @@ -149,34 +149,19 @@ PSymbolTable::~PSymbolTable () //========================================================================== // -// -// -//========================================================================== - -size_t PSymbolTable::MarkSymbols() -{ - size_t count = 0; - MapType::Iterator it(Symbols); - MapType::Pair *pair; - - while (it.NextPair(pair)) - { - GC::Mark(pair->Value); - count++; - } - return count * sizeof(*pair); -} - -//========================================================================== -// -// +// this must explicitly delete all content because the symbols have +// been released from the GC. // //========================================================================== void PSymbolTable::ReleaseSymbols() { - // The GC will take care of deleting the symbols. We just need to - // clear our references to them. + auto it = GetIterator(); + MapType::Pair *pair; + while (it.NextPair(pair)) + { + delete pair->Value; + } Symbols.Clear(); } @@ -243,6 +228,7 @@ PSymbol *PSymbolTable::AddSymbol (PSymbol *sym) return nullptr; } Symbols.Insert(sym->SymbolName, sym); + sym->Release(); // no more GC, please! return sym; } @@ -257,6 +243,7 @@ void PSymbolTable::RemoveSymbol(PSymbol *sym) auto mysym = Symbols.CheckKey(sym->SymbolName); if (mysym == nullptr || *mysym != sym) return; Symbols.Remove(sym->SymbolName); + delete sym; } //========================================================================== @@ -265,20 +252,20 @@ void PSymbolTable::RemoveSymbol(PSymbol *sym) // //========================================================================== -PSymbol *PSymbolTable::ReplaceSymbol(PSymbol *newsym) +void PSymbolTable::ReplaceSymbol(PSymbol *newsym) { // If a symbol with a matching name exists, take its place and return it. PSymbol **symslot = Symbols.CheckKey(newsym->SymbolName); if (symslot != nullptr) { PSymbol *oldsym = *symslot; + delete oldsym; *symslot = newsym; - return oldsym; } // Else, just insert normally and return nullptr since there was no // symbol to replace. + newsym->Release(); // no more GC, please! Symbols.Insert(newsym->SymbolName, newsym); - return nullptr; } //========================================================================== @@ -306,18 +293,6 @@ PNamespace::PNamespace(int filenum, PNamespace *parent) // //========================================================================== -size_t PNamespace::PropagateMark() -{ - GC::Mark(Parent); - return Symbols.MarkSymbols() + 1; -} - -//========================================================================== -// -// -// -//========================================================================== - FNamespaceManager::FNamespaceManager() { GlobalNamespace = nullptr; @@ -370,6 +345,7 @@ size_t FNamespaceManager::MarkSymbols() void FNamespaceManager::ReleaseSymbols() { + RemoveSymbols(); GlobalNamespace = nullptr; AllNamespaces.Clear(); } diff --git a/src/scripting/symbols.h b/src/scripting/symbols.h index af5bfb80e9..3c9d1f8f32 100644 --- a/src/scripting/symbols.h +++ b/src/scripting/symbols.h @@ -192,8 +192,6 @@ struct PSymbolTable PSymbolTable(PSymbolTable *parent); ~PSymbolTable(); - size_t MarkSymbols(); - // Sets the table to use for searches if this one doesn't contain the // requested symbol. void SetParentTable (PSymbolTable *parent); @@ -218,7 +216,7 @@ struct PSymbolTable // Similar to AddSymbol but always succeeds. Returns the symbol that used // to be in the table with this name, if any. - PSymbol *ReplaceSymbol(PSymbol *sym); + void ReplaceSymbol(PSymbol *sym); void RemoveSymbol(PSymbol *sym); @@ -255,7 +253,6 @@ public: PNamespace() {} PNamespace(int filenum, PNamespace *parent); - size_t PropagateMark(); }; struct FNamespaceManager