diff --git a/bindings/jupyroot/python/JupyROOT/helpers/cppcompleter.py b/bindings/jupyroot/python/JupyROOT/helpers/cppcompleter.py index b9f622c3292f5..f8ec85031148b 100644 --- a/bindings/jupyroot/python/JupyROOT/helpers/cppcompleter.py +++ b/bindings/jupyroot/python/JupyROOT/helpers/cppcompleter.py @@ -75,6 +75,7 @@ class CppCompleter(object): ... print(suggestion) TROOT::IsA TROOT::IsBatch + TROOT::IsDestructed TROOT::IsEqual TROOT::IsEscaped TROOT::IsExecutingMacro diff --git a/bindings/pyroot_legacy/JupyROOT/helpers/cppcompleter.py b/bindings/pyroot_legacy/JupyROOT/helpers/cppcompleter.py index 5ab9d96f2df41..aa966442387f2 100644 --- a/bindings/pyroot_legacy/JupyROOT/helpers/cppcompleter.py +++ b/bindings/pyroot_legacy/JupyROOT/helpers/cppcompleter.py @@ -87,6 +87,7 @@ class CppCompleter(object): ... print(suggestion) TROOT::IsA TROOT::IsBatch + TROOT::IsDestructed TROOT::IsEqual TROOT::IsEscaped TROOT::IsExecutingMacro diff --git a/core/base/inc/TObject.h b/core/base/inc/TObject.h index 08d2f865b58c8..3942f0f5e7ddc 100644 --- a/core/base/inc/TObject.h +++ b/core/base/inc/TObject.h @@ -33,6 +33,10 @@ class TObjArray; class TMethod; class TTimer; +namespace ROOT { +namespace Internal { + bool DeleteChangesMemoryImpl(); +}} class TObject { @@ -163,6 +167,16 @@ class TObject { virtual Int_t Write(const char *name = nullptr, Int_t option = 0, Int_t bufsize = 0); virtual Int_t Write(const char *name = nullptr, Int_t option = 0, Int_t bufsize = 0) const; + /// IsDestructed + /// + /// \note This function must be non-virtual as it can be used on destructed (but + /// not yet modified) memory. This is used for example in TClonesArray to record + /// the element that have been destructed but not deleted and thus are ready for + /// re-use (by operator new with placement). + /// + /// \return true if this object's destructor has been run. + Bool_t IsDestructed() const { return !TestBit(kNotDeleted); } + //----- operators void *operator new(size_t sz) { return TStorage::ObjectAlloc(sz); } void *operator new[](size_t sz) { return TStorage::ObjectAllocArray(sz); } @@ -226,6 +240,7 @@ class TObject { static void SetObjectStat(Bool_t stat); friend class TClonesArray; // needs to reset kNotDeleted in fBits + friend bool ROOT::Internal::DeleteChangesMemoryImpl(); ClassDef(TObject,1) //Basic ROOT object }; @@ -365,4 +380,31 @@ namespace cling { std::string printValue(TObject *val); } +namespace ROOT { + +namespace Internal { + bool DeleteChangesMemory(); +} // Internal + +namespace Detail { + + +/// @brief Check if the TObject's memory has been deleted. +/// @warning This should be only used for error mitigation as the answer is only +/// sometimes correct. It actually just checks whether the object has been +/// deleted, so this will falsely return true for an object that has +/// been destructed but its memory has not been deleted. This will return an +/// undefined value if the memory is re-used between the deletion and the check. +/// i.e. This is useful to prevent a segmentation fault in case where the problem +/// can be detected when the deletion and the usage are 'close-by' +/// @warning In enviroment where delete taints (changes) the memory, this function +/// always returns false as the marker left by ~TObject will be overwritten. +/// @param obj The memory to check +/// @return true if the object has been destructed and it can be inferred that it has been deleted +R__ALWAYS_INLINE bool HasBeenDeleted(const TObject *obj) { + return !ROOT::Internal::DeleteChangesMemory() && obj->IsDestructed(); +} + +}} // ROOT::Details + #endif diff --git a/core/base/src/TObject.cxx b/core/base/src/TObject.cxx index 4496d881ca78d..523690b925cf4 100644 --- a/core/base/src/TObject.cxx +++ b/core/base/src/TObject.cxx @@ -56,6 +56,76 @@ Bool_t TObject::fgObjectStat = kTRUE; ClassImp(TObject); +namespace ROOT { +namespace Internal { + +// Return true if delete changes/poisons/taints the memory. +// +// Detect whether operator delete taints the memory. If it does, we can not rely +// on TestBit(kNotDeleted) to check if the memory has been deleted (but in case, +// like TClonesArray, where we know the destructor will be called but not operator +// delete, we can still use it to detect the cases where the destructor was called. + +bool DeleteChangesMemoryImpl() +{ + static constexpr UInt_t kGoldenUUID = 0x00000021; + static constexpr UInt_t kGoldenbits = 0x03000000; + + TObject *o = new TObject; + o->SetUniqueID(kGoldenUUID); + UInt_t *o_fuid = &(o->fUniqueID); + UInt_t *o_fbits = &(o->fBits); + + if (*o_fuid != kGoldenUUID) { + Error("CheckingDeleteSideEffects", + "fUniqueID is not as expected, we got 0x%.8x instead of 0x%.8x", + *o_fuid, kGoldenUUID); + } + if (*o_fbits != kGoldenbits) { + Error("CheckingDeleteSideEffects", + "fBits is not as expected, we got 0x%.8x instead of 0x%.8x", + *o_fbits, kGoldenbits); + } + if (gDebug >= 9) { + unsigned char *oc = reinterpret_cast(o); // for address calculations + unsigned char references[sizeof(TObject)]; + memcpy(references, oc, sizeof(TObject)); + + // The effective part of this code (the else statement is just that without + // any of the debug statement) + delete o; + + // Not using the error logger, as there routine is meant to be called + // during library initialization/loading. + fprintf(stderr, + "DEBUG: Checking before and after delete the content of a TObject with uniqueID 0x21\n"); + for(size_t i = 0; i < sizeof(TObject); i += 4) { + fprintf(stderr, "DEBUG: 0x%.8x vs 0x%.8x\n", *(int*)(references +i), *(int*)(oc + i)); + } + } else + delete o; // the 'if' part is that surrounded by the debug code. + + // Intentionally accessing the deleted memory to check whether it has been changed as + // a consequence (side effect) of executing operator delete. If there no change, we + // can guess this is always the case and we can rely on the changes to fBits made + // by ~TObject to detect use-after-delete error (and print a message rather than + // stop the program with a segmentation fault) + if ( *o_fbits != 0x01000000 ) { + // operator delete tainted the memory, we can not rely on TestBit(kNotDeleted) + return true; + } + return false; +} + +bool DeleteChangesMemory() +{ + static const bool value = DeleteChangesMemoryImpl(); + if (gDebug >= 9) + DeleteChangesMemoryImpl(); // To allow for printing the debug info + return value; +} + +}} // ROOT::Detail //////////////////////////////////////////////////////////////////////////////// /// Copy this to obj. diff --git a/core/base/src/TQConnection.cxx b/core/base/src/TQConnection.cxx index 448dbb922cbf9..16cd3c5653e39 100644 --- a/core/base/src/TQConnection.cxx +++ b/core/base/src/TQConnection.cxx @@ -284,7 +284,7 @@ CallFunc_t *TQSlot::StartExecuting() { void TQSlot::EndExecuting() { fExecuting--; - if (!TestBit(kNotDeleted) && !fExecuting) + if (ROOT::Detail::HasBeenDeleted(this) && !fExecuting) gCling->CallFunc_Delete(fFunc); } @@ -344,7 +344,7 @@ inline void TQSlot::ExecuteMethod(void *object, Longptr_t *paramArr, Int_t npara fExecuting++; gCling->CallFunc_Exec(fFunc, address); fExecuting--; - if (!TestBit(kNotDeleted) && !fExecuting) + if (ROOT::Detail::HasBeenDeleted(this) && !fExecuting) gCling->CallFunc_Delete(fFunc); } diff --git a/core/base/src/TROOT.cxx b/core/base/src/TROOT.cxx index a09aa34f251a9..d7e218bbd3038 100644 --- a/core/base/src/TROOT.cxx +++ b/core/base/src/TROOT.cxx @@ -1371,7 +1371,7 @@ TObject *TROOT::FindSpecialObject(const char *name, void *&where) } } if (!temp) return nullptr; - if (temp->TestBit(kNotDeleted)) return temp; + if (!ROOT::Detail::HasBeenDeleted(temp)) return temp; return nullptr; } diff --git a/core/cont/src/TClonesArray.cxx b/core/cont/src/TClonesArray.cxx index 5f0620880c2c6..623570b02e353 100644 --- a/core/cont/src/TClonesArray.cxx +++ b/core/cont/src/TClonesArray.cxx @@ -156,7 +156,7 @@ bool R__SetClonesArrayTFormulaUpdater(Updater_t func) { /// Internal Utility routine to correctly release the memory for an object static inline void R__ReleaseMemory(TClass *cl, TObject *obj) { - if (obj && obj->TestBit(TObject::kNotDeleted)) { + if (obj && ! obj->IsDestructed()) { // -- The TObject destructor has not been called. cl->Destructor(obj); } else { @@ -366,7 +366,7 @@ void TClonesArray::Compress() TObject *TClonesArray::ConstructedAt(Int_t idx) { TObject *obj = (*this)[idx]; - if ( obj && obj->TestBit(TObject::kNotDeleted) ) { + if ( obj && ! obj->IsDestructed() ) { return obj; } return (fClass) ? static_cast(fClass->New(obj)) : nullptr; @@ -388,7 +388,7 @@ TObject *TClonesArray::ConstructedAt(Int_t idx) TObject *TClonesArray::ConstructedAt(Int_t idx, Option_t *clear_options) { TObject *obj = (*this)[idx]; - if ( obj && obj->TestBit(TObject::kNotDeleted) ) { + if ( obj && !obj->IsDestructed() ) { obj->Clear(clear_options); return obj; } @@ -444,13 +444,13 @@ void TClonesArray::Delete(Option_t *) // In case of emulated class, we can not use the delete operator // directly, it would use the wrong destructor. for (Int_t i = 0; i < fSize; i++) { - if (fCont[i] && fCont[i]->TestBit(kNotDeleted)) { + if (fCont[i] && ! fCont[i]->IsDestructed()) { fClass->Destructor(fCont[i],kTRUE); } } } else { for (Int_t i = 0; i < fSize; i++) { - if (fCont[i] && fCont[i]->TestBit(kNotDeleted)) { + if (fCont[i] && ! fCont[i]->IsDestructed()) { fCont[i]->~TObject(); } } @@ -516,7 +516,7 @@ void TClonesArray::ExpandCreate(Int_t n) for (i = 0; i < n; i++) { if (!fKeep->fCont[i]) { fKeep->fCont[i] = (TObject*)fClass->New(); - } else if (!fKeep->fCont[i]->TestBit(kNotDeleted)) { + } else if (fKeep->fCont[i]->IsDestructed()) { // The object has been deleted (or never initialized) fClass->New(fKeep->fCont[i]); } @@ -553,7 +553,7 @@ void TClonesArray::ExpandCreateFast(Int_t n) for (i = 0; i < n; i++) { if (i >= oldSize || !fKeep->fCont[i]) { fKeep->fCont[i] = (TObject*)fClass->New(); - } else if (!fKeep->fCont[i]->TestBit(kNotDeleted)) { + } else if (fKeep->fCont[i]->IsDestructed()) { // The object has been deleted (or never initialized) fClass->New(fKeep->fCont[i]); } @@ -575,7 +575,7 @@ TObject *TClonesArray::RemoveAt(Int_t idx) int i = idx-fLowerBound; - if (fCont[i] && fCont[i]->TestBit(kNotDeleted)) { + if (fCont[i] && ! fCont[i]->IsDestructed()) { fCont[i]->~TObject(); } @@ -601,7 +601,7 @@ TObject *TClonesArray::Remove(TObject *obj) if (i == -1) return nullptr; - if (fCont[i] && fCont[i]->TestBit(kNotDeleted)) { + if (fCont[i] && ! fCont[i]->IsDestructed()) { fCont[i]->~TObject(); } @@ -627,7 +627,7 @@ void TClonesArray::RemoveRange(Int_t idx1, Int_t idx2) Bool_t change = kFALSE; for (TObject **obj = fCont+idx1; obj <= fCont+idx2; obj++) { if (!*obj) continue; - if ((*obj)->TestBit(kNotDeleted)) { + if (!(*obj)->IsDestructed()) { (*obj)->~TObject(); } *obj = nullptr; @@ -801,7 +801,7 @@ void TClonesArray::Streamer(TBuffer &b) for (Int_t i = 0; i < nobjects; i++) { if (!fKeep->fCont[i]) { fKeep->fCont[i] = (TObject*)fClass->New(); - } else if (!fKeep->fCont[i]->TestBit(kNotDeleted)) { + } else if (fKeep->fCont[i]->IsDestructed()) { // The object has been deleted (or never initialized) fClass->New(fKeep->fCont[i]); } @@ -836,7 +836,7 @@ void TClonesArray::Streamer(TBuffer &b) if (nch) { if (!fKeep->fCont[i]) fKeep->fCont[i] = (TObject*)fClass->New(); - else if (!fKeep->fCont[i]->TestBit(kNotDeleted)) { + else if (fKeep->fCont[i]->IsDestructed()) { // The object has been deleted (or never initialized) fClass->New(fKeep->fCont[i]); } @@ -922,7 +922,7 @@ TObject *&TClonesArray::operator[](Int_t idx) fKeep->fCont[idx] = (TObject*) TStorage::ObjectAlloc(fClass->Size()); // Reset the bit so that: // obj = myClonesArray[i]; - // obj->TestBit(TObject::kNotDeleted) + // ! obj->IsDestructed() // will behave correctly. // TObject::kNotDeleted is one of the higher bit that is not settable via the public // interface. But luckily we are its friend. diff --git a/core/cont/src/TCollection.cxx b/core/cont/src/TCollection.cxx index 5c5190ef148b6..ea8da3b25ed79 100644 --- a/core/cont/src/TCollection.cxx +++ b/core/cont/src/TCollection.cxx @@ -584,7 +584,7 @@ void TCollection::RecursiveRemove(TObject *obj) TObject *object; while ((object = next())) { - if (object->TestBit(kNotDeleted)) object->RecursiveRemove(obj); + if (!ROOT::Detail::HasBeenDeleted(object)) object->RecursiveRemove(obj); } } diff --git a/core/cont/src/THashList.cxx b/core/cont/src/THashList.cxx index aa2e8688a9b4e..83d139b832ed0 100644 --- a/core/cont/src/THashList.cxx +++ b/core/cont/src/THashList.cxx @@ -227,7 +227,7 @@ void THashList::Delete(Option_t *option) auto obj = tlk->GetObject(); // In case somebody else access it. tlk->SetObject(nullptr); - if (obj && !obj->TestBit(kNotDeleted)) + if (obj && ROOT::Detail::HasBeenDeleted(obj)) Error("Delete", "A list is accessing an object (%p) already deleted (list name = %s)", obj, GetName()); else if (obj && obj->IsOnHeap()) @@ -350,7 +350,7 @@ void THashList::RecursiveRemove(TObject *obj) while (lnk.get()) { next = lnk->NextSP(); TObject *ob = lnk->GetObject(); - if (ob && ob->TestBit(kNotDeleted)) { + if (ob && !ROOT::Detail::HasBeenDeleted(ob)) { ob->RecursiveRemove(obj); } lnk = next; diff --git a/core/cont/src/TList.cxx b/core/cont/src/TList.cxx index 034c7be31adaf..3ce34a2b8bef2 100644 --- a/core/cont/src/TList.cxx +++ b/core/cont/src/TList.cxx @@ -440,12 +440,12 @@ void TList::Clear(Option_t *option) // delete only heap objects marked OK to clear auto obj = tlk->GetObject(); if (!nodel && obj) { - if (!obj->TestBit(kNotDeleted)) { + if (ROOT::Detail::HasBeenDeleted(obj)) { Error("Clear", "A list is accessing an object (%p) already deleted (list name = %s)", obj, GetName()); } else if (obj->IsOnHeap()) { if (obj->TestBit(kCanDelete)) { - if (obj->TestBit(kNotDeleted)) { + if (!ROOT::Detail::HasBeenDeleted(obj)) { TCollection::GarbageCollect(obj); } } @@ -501,7 +501,7 @@ void TList::Delete(Option_t *option) // delete only heap objects auto obj = tlk->GetObject(); - if (obj && !obj->TestBit(kNotDeleted)) + if (obj && ROOT::Detail::HasBeenDeleted(obj)) Error("Delete", "A list is accessing an object (%p) already deleted (list name = %s)", obj, GetName()); else if (obj && obj->IsOnHeap()) @@ -530,7 +530,7 @@ void TList::Delete(Option_t *option) // delete only heap objects auto obj = tlk->GetObject(); tlk->SetObject(nullptr); - if (obj && !obj->TestBit(kNotDeleted)) + if (obj && ROOT::Detail::HasBeenDeleted(obj)) Error("Delete", "A list is accessing an object (%p) already deleted (list name = %s)", obj, GetName()); else if (obj && obj->IsOnHeap()) @@ -643,7 +643,7 @@ TObjLink *TList::FindLink(const TObject *obj, Int_t &idx) const while (lnk) { object = lnk->GetObject(); if (object) { - if (object->TestBit(kNotDeleted)) { + if (!ROOT::Detail::HasBeenDeleted(object)) { if (object->IsEqual(obj)) return lnk; } } @@ -773,7 +773,7 @@ void TList::RecursiveRemove(TObject *obj) auto cached = fCache.lock(); if (cached && cached->fNext.get() == nullptr && cached->fPrev.lock().get() == nullptr) { TObject *ob = cached->GetObject(); - if (ob && ob->TestBit(kNotDeleted)) { + if (ob && !ROOT::Detail::HasBeenDeleted(ob)) { ob->RecursiveRemove(obj); } } @@ -787,7 +787,7 @@ void TList::RecursiveRemove(TObject *obj) while (lnk.get()) { next = lnk->fNext; TObject *ob = lnk->GetObject(); - if (ob && ob->TestBit(kNotDeleted)) { + if (ob && !ROOT::Detail::HasBeenDeleted(ob)) { if (ob->IsEqual(obj)) { lnk->SetObject(nullptr); if (lnk == fFirst) { diff --git a/core/cont/src/TObjArray.cxx b/core/cont/src/TObjArray.cxx index ac02d217fdeca..3b531dbeba36c 100644 --- a/core/cont/src/TObjArray.cxx +++ b/core/cont/src/TObjArray.cxx @@ -675,7 +675,7 @@ void TObjArray::RecursiveRemove(TObject *obj) R__COLLECTION_WRITE_LOCKGUARD(ROOT::gCoreMutex); for (int i = 0; i < fSize; i++) { - if (fCont[i] && fCont[i]->TestBit(kNotDeleted) && fCont[i]->IsEqual(obj)) { + if (fCont[i] && !ROOT::Detail::HasBeenDeleted(fCont[i]) && fCont[i]->IsEqual(obj)) { fCont[i] = nullptr; // recalculate array size if (i == fLast) @@ -683,7 +683,7 @@ void TObjArray::RecursiveRemove(TObject *obj) fLast--; } while (fLast >= 0 && !fCont[fLast]); Changed(); - } else if (fCont[i] && fCont[i]->TestBit(kNotDeleted)) + } else if (fCont[i] && !ROOT::Detail::HasBeenDeleted(fCont[i])) fCont[i]->RecursiveRemove(obj); } } diff --git a/core/cont/src/TObjectTable.cxx b/core/cont/src/TObjectTable.cxx index ed43ccb393e7a..e78f7f339cf98 100644 --- a/core/cont/src/TObjectTable.cxx +++ b/core/cont/src/TObjectTable.cxx @@ -383,7 +383,7 @@ void TObjectTable::UpdateInstCount() const for (int i = 0; i < fSize; i++) if ((op = fTable[i])) { // attention: no == - if (op->TestBit(TObject::kNotDeleted)) + if (!ROOT::Detail::HasBeenDeleted(op)) op->IsA()->AddInstance(op->IsOnHeap()); else Error("UpdateInstCount", "oops 0x%zx\n", (size_t)op); diff --git a/core/gui/src/TContextMenu.cxx b/core/gui/src/TContextMenu.cxx index dc266ee6d2ab0..cf6f17742ced7 100644 --- a/core/gui/src/TContextMenu.cxx +++ b/core/gui/src/TContextMenu.cxx @@ -126,7 +126,7 @@ void TContextMenu::Action(TClassMenuItem *menuitem) if (object) { // If object deleted, remove from popup and return - if (!(object->TestBit(kNotDeleted))) { + if (ROOT::Detail::HasBeenDeleted(object)) { menuitem->SetType(TClassMenuItem::kPopupSeparator); menuitem->SetCall(nullptr, ""); return; diff --git a/etc/valgrind-root.supp b/etc/valgrind-root.supp index 230362de38cc4..01ea89754559d 100644 --- a/etc/valgrind-root.supp +++ b/etc/valgrind-root.supp @@ -1013,6 +1013,14 @@ fun:_ZN11TBufferFile13ReadStdStringERSs } +######## ROOT use-after-delete detection attempts + +{ + Checking if delete taints memory + Memcheck:Addr4 + fun:_ZN4ROOT8Internal*DeleteChangesMemory* +} + ######## ROOT TObject on heap { diff --git a/graf2d/gpad/src/TButton.cxx b/graf2d/gpad/src/TButton.cxx index c768d018c2d26..d724956d1d386 100644 --- a/graf2d/gpad/src/TButton.cxx +++ b/graf2d/gpad/src/TButton.cxx @@ -149,7 +149,7 @@ void TButton::Draw(Option_t *option) void TButton::ExecuteEvent(Int_t event, Int_t px, Int_t py) { //check case where pressing a button deletes itself - if (!TestBit(kNotDeleted)) return; + if (ROOT::Detail::HasBeenDeleted(this)) return; if (IsEditable()) { TPad::ExecuteEvent(event,px,py); @@ -202,7 +202,7 @@ void TButton::ExecuteEvent(Int_t event, Int_t px, Int_t py) gROOT->ProcessLine(GetMethod()); } //check case where pressing a button deletes itself - if (!TestBit(kNotDeleted)) return; + if (ROOT::Detail::HasBeenDeleted(this)) return; SetBorderMode(1); Modified(); Update(); diff --git a/graf2d/gpad/src/TCanvas.cxx b/graf2d/gpad/src/TCanvas.cxx index 4f976b954a4c6..605e23a3926cf 100644 --- a/graf2d/gpad/src/TCanvas.cxx +++ b/graf2d/gpad/src/TCanvas.cxx @@ -687,7 +687,7 @@ void TCanvas::Destructor() if ((*gThreadXAR)("CDEL", 2, arr, 0)) return; } - if (!TestBit(kNotDeleted)) return; + if (ROOT::Detail::HasBeenDeleted(this)) return; SafeDelete(fContextMenu); if (!gPad) return; diff --git a/graf2d/gpad/src/TPad.cxx b/graf2d/gpad/src/TPad.cxx index ab8c1846e8e10..4d4db5996c93e 100644 --- a/graf2d/gpad/src/TPad.cxx +++ b/graf2d/gpad/src/TPad.cxx @@ -379,7 +379,7 @@ TPad::TPad(const char *name, const char *title, Double_t xlow, TPad::~TPad() { - if (!TestBit(kNotDeleted)) return; + if (ROOT::Detail::HasBeenDeleted(this)) return; Close(); CloseToolTip(fTip); DeleteToolTip(fTip); @@ -633,7 +633,7 @@ void TPad::Clear(Option_t *option) SafeDelete(fView); if (fPrimitives) fPrimitives->Clear(option); if (fFrame) { - if (fFrame->TestBit(kNotDeleted)) delete fFrame; + if (! ROOT::Detail::HasBeenDeleted(fFrame)) delete fFrame; fFrame = nullptr; } } @@ -973,18 +973,18 @@ Int_t TPad::ClipPolygon(Int_t n, Double_t *x, Double_t *y, Int_t nn, Double_t *x void TPad::Close(Option_t *) { - if (!TestBit(kNotDeleted)) return; + if (ROOT::Detail::HasBeenDeleted(this)) return; if (!fMother) return; - if (!fMother->TestBit(kNotDeleted)) return; + if (ROOT::Detail::HasBeenDeleted(fMother)) return; if (fPrimitives) fPrimitives->Clear(); if (fView) { - if (fView->TestBit(kNotDeleted)) delete fView; + if (!ROOT::Detail::HasBeenDeleted(fView)) delete fView; fView = nullptr; } if (fFrame) { - if (fFrame->TestBit(kNotDeleted)) delete fFrame; + if (!ROOT::Detail::HasBeenDeleted(fFrame)) delete fFrame; fFrame = nullptr; } @@ -1274,7 +1274,7 @@ void TPad::Draw(Option_t *option) // pad cannot be in itself and it can only be in one other pad at a time if (!fPrimitives) fPrimitives = new TList; if (gPad != this) { - if (fMother && fMother->TestBit(kNotDeleted)) + if (fMother && !ROOT::Detail::HasBeenDeleted(fMother)) if (fMother->GetListOfPrimitives()) fMother->GetListOfPrimitives()->Remove(this); TPad *oldMother = fMother; fCanvas = gPad->GetCanvas(); @@ -4611,7 +4611,7 @@ TPad *TPad::Pick(Int_t px, Int_t py, TObjLink *&pickobj) void TPad::Pop() { if (!fMother) return; - if (!fMother->TestBit(kNotDeleted)) return; + if (ROOT::Detail::HasBeenDeleted(fMother)) return; if (!fPrimitives) fPrimitives = new TList; if (this == fMother->GetListOfPrimitives()->Last()) return; diff --git a/graf2d/gpad/src/TRatioPlot.cxx b/graf2d/gpad/src/TRatioPlot.cxx index 25967b2b3f147..1f6b911cc2e72 100644 --- a/graf2d/gpad/src/TRatioPlot.cxx +++ b/graf2d/gpad/src/TRatioPlot.cxx @@ -111,26 +111,31 @@ TRatioPlot::~TRatioPlot() gROOT->GetListOfCleanups()->Remove(this); - if (fRatioGraph && fRatioGraph->TestBit(kNotDeleted)) delete fRatioGraph; - if (fConfidenceInterval1 && fConfidenceInterval1->TestBit(kNotDeleted)) delete fConfidenceInterval1; - if (fConfidenceInterval2 && fConfidenceInterval2->TestBit(kNotDeleted)) delete fConfidenceInterval2; + auto safeDelete = [](TObject *obj) { + if (obj && !ROOT::Detail::HasBeenDeleted(obj)) + delete obj; + }; + + safeDelete(fRatioGraph); + safeDelete(fConfidenceInterval1); + safeDelete(fConfidenceInterval2); for (unsigned int i=0;iTestBit(kNotDeleted)) delete fSharedXAxis; - if (fUpperGXaxis && fUpperGXaxis->TestBit(kNotDeleted)) delete fUpperGXaxis; - if (fLowerGXaxis && fLowerGXaxis->TestBit(kNotDeleted)) delete fLowerGXaxis; - if (fUpperGYaxis && fUpperGYaxis->TestBit(kNotDeleted)) delete fUpperGYaxis; - if (fLowerGYaxis && fLowerGYaxis->TestBit(kNotDeleted)) delete fLowerGYaxis; - if (fUpperGXaxisMirror && fUpperGXaxisMirror->TestBit(kNotDeleted)) delete fUpperGXaxisMirror; - if (fLowerGXaxisMirror && fLowerGXaxisMirror->TestBit(kNotDeleted)) delete fLowerGXaxisMirror; - if (fUpperGYaxisMirror && fUpperGYaxisMirror->TestBit(kNotDeleted)) delete fUpperGYaxisMirror; - if (fLowerGYaxisMirror && fLowerGYaxisMirror->TestBit(kNotDeleted)) delete fLowerGYaxisMirror; + safeDelete(fSharedXAxis); + safeDelete(fUpperGXaxis); + safeDelete(fLowerGXaxis); + safeDelete(fUpperGYaxis); + safeDelete(fLowerGYaxis); + safeDelete(fUpperGXaxisMirror); + safeDelete(fLowerGXaxisMirror); + safeDelete(fUpperGYaxisMirror); + safeDelete(fLowerGYaxisMirror); - if (fUpYaxis && fUpYaxis->TestBit(kNotDeleted)) delete fUpYaxis; - if (fLowYaxis && fLowYaxis->TestBit(kNotDeleted)) delete fLowYaxis; + safeDelete(fUpYaxis); + safeDelete(fLowYaxis); } //////////////////////////////////////////////////////////////////////////////// diff --git a/graf2d/graf/src/TPaveText.cxx b/graf2d/graf/src/TPaveText.cxx index aa9f24e71d2ba..708a7db84bf7a 100644 --- a/graf2d/graf/src/TPaveText.cxx +++ b/graf2d/graf/src/TPaveText.cxx @@ -115,7 +115,7 @@ TPaveText::TPaveText(Double_t x1, Double_t y1,Double_t x2, Double_t y2, Option_ TPaveText::~TPaveText() { - if (!TestBit(kNotDeleted)) return; + if (ROOT::Detail::HasBeenDeleted(this)) return; if (fLines) fLines->Delete(); delete fLines; fLines = nullptr; diff --git a/graf3d/g3d/src/TGeometry.cxx b/graf3d/g3d/src/TGeometry.cxx index 5355c2b53e76c..b4b02e7adb0e9 100644 --- a/graf3d/g3d/src/TGeometry.cxx +++ b/graf3d/g3d/src/TGeometry.cxx @@ -346,7 +346,7 @@ TNode *TGeometry::GetNode(const char *name) const { TNode *node= (TNode*)GetListOfNodes()->First(); if (!node) return 0; - if (node->TestBit(kNotDeleted)) return node->GetNode(name); + if (!ROOT::Detail::HasBeenDeleted(node)) return node->GetNode(name); return 0; } diff --git a/graf3d/g3d/src/TNode.cxx b/graf3d/g3d/src/TNode.cxx index 92a72dc1f7e91..ace9ebce38aa1 100644 --- a/graf3d/g3d/src/TNode.cxx +++ b/graf3d/g3d/src/TNode.cxx @@ -382,7 +382,7 @@ TNode *TNode::GetNode(const char *name) const TObjLink *lnk = fNodes->FirstLink(); while (lnk) { node = (TNode *)lnk->GetObject(); - if (node->TestBit(kNotDeleted)) { + if (!ROOT::Detail::HasBeenDeleted(node)) { nodefound = node->GetNode(name); if (nodefound) return nodefound; } diff --git a/gui/gui/src/TGFileBrowser.cxx b/gui/gui/src/TGFileBrowser.cxx index 8f049e88e703a..225863f118be4 100644 --- a/gui/gui/src/TGFileBrowser.cxx +++ b/gui/gui/src/TGFileBrowser.cxx @@ -656,7 +656,7 @@ void TGFileBrowser::Update() TGListTreeItem *curr = fListTree->GetSelected(); // GetCurrent() ?? if (curr) { TObject *obj = (TObject *) curr->GetUserData(); - if (obj && !obj->TestBit(kNotDeleted)) { + if (obj && ROOT::Detail::HasBeenDeleted(obj)) { // if the item to be deleted has a filter, // delete its entry in the map if (CheckFiltered(curr)) @@ -665,7 +665,7 @@ void TGFileBrowser::Update() curr = 0; obj = 0; } - else if (obj && obj->TestBit(kNotDeleted) && + else if (obj && !ROOT::Detail::HasBeenDeleted(obj) && obj->InheritsFrom("TObjString") && curr->GetParent()) { fListTree->GetPathnameFromItem(curr->GetParent(), path); if (strlen(path) > 1) { diff --git a/gui/gui/src/TGFrame.cxx b/gui/gui/src/TGFrame.cxx index dadbb6d2bb2b4..1ad1940f3c6b6 100644 --- a/gui/gui/src/TGFrame.cxx +++ b/gui/gui/src/TGFrame.cxx @@ -1731,7 +1731,7 @@ Bool_t TGMainFrame::HandleClientMessage(Event_t *event) if ((event->fFormat == 32) && ((Atom_t)event->fUser[0] == gWM_DELETE_WINDOW) && (event->fHandle != gROOT_MESSAGE)) { Emit("CloseWindow()"); - if (TestBit(kNotDeleted) && !TestBit(kDontCallClose)) + if (!ROOT::Detail::HasBeenDeleted(this) && !TestBit(kDontCallClose)) CloseWindow(); } return kTRUE; diff --git a/gui/gui/src/TGMdiMainFrame.cxx b/gui/gui/src/TGMdiMainFrame.cxx index b6609603f30e4..96a6bc669846e 100644 --- a/gui/gui/src/TGMdiMainFrame.cxx +++ b/gui/gui/src/TGMdiMainFrame.cxx @@ -963,7 +963,7 @@ Int_t TGMdiMainFrame::Close(TGMdiFrame *mdiframe) TGMdiDecorFrame *frame = GetDecorFrame(mdiframe); Restore(mdiframe); mdiframe->Emit("CloseWindow()"); - if (frame && mdiframe->TestBit(kNotDeleted) && !mdiframe->TestBit(TGMdiFrame::kDontCallClose)) + if (frame && !ROOT::Detail::HasBeenDeleted(mdiframe) && !mdiframe->TestBit(TGMdiFrame::kDontCallClose)) return frame->CloseWindow(); return kTRUE; } diff --git a/gui/gui/src/TGTextEntry.cxx b/gui/gui/src/TGTextEntry.cxx index c71f713cd73eb..f51c6cd419575 100644 --- a/gui/gui/src/TGTextEntry.cxx +++ b/gui/gui/src/TGTextEntry.cxx @@ -1229,7 +1229,7 @@ Bool_t TGTextEntry::HandleKey(Event_t* event) if ((EKeySym)keysym == kKey_Enter || (EKeySym)keysym == kKey_Return) { ReturnPressed(); // emit signal - if (!TestBit(kNotDeleted)) return kTRUE; + if (ROOT::Detail::HasBeenDeleted(this)) return kTRUE; fSelectionOn = kFALSE; } else if (event->fState & kKeyShiftMask && (EKeySym)keysym == kKey_Backtab) { diff --git a/hist/hist/src/TEfficiency.cxx b/hist/hist/src/TEfficiency.cxx index e0132c8bab955..2a7002de3d69e 100644 --- a/hist/hist/src/TEfficiency.cxx +++ b/hist/hist/src/TEfficiency.cxx @@ -1033,7 +1033,7 @@ TEfficiency::~TEfficiency() TObject* obj = 0; while ((obj = fFunctions->First())) { while(fFunctions->Remove(obj)) { } - if (!obj->TestBit(kNotDeleted)) { + if (ROOT::Detail::HasBeenDeleted(obj)) { break; } delete obj; diff --git a/hist/hist/src/TH1.cxx b/hist/hist/src/TH1.cxx index 8257ceccd9798..dfa759d744ba2 100644 --- a/hist/hist/src/TH1.cxx +++ b/hist/hist/src/TH1.cxx @@ -623,7 +623,7 @@ TH1::TH1(): TNamed(), TAttLine(), TAttFill(), TAttMarker() TH1::~TH1() { - if (!TestBit(kNotDeleted)) { + if (ROOT::Detail::HasBeenDeleted(this)) { return; } delete[] fIntegral; @@ -644,7 +644,7 @@ TH1::~TH1() //and may have been already deleted. while ((obj = fFunctions->First())) { while(fFunctions->Remove(obj)) { } - if (!obj->TestBit(kNotDeleted)) { + if (ROOT::Detail::HasBeenDeleted(obj)) { break; } delete obj; diff --git a/net/net/src/TApplicationServer.cxx b/net/net/src/TApplicationServer.cxx index ea59b3fc80079..bcd8c9693c105 100644 --- a/net/net/src/TApplicationServer.cxx +++ b/net/net/src/TApplicationServer.cxx @@ -896,7 +896,7 @@ Int_t TApplicationServer::SendCanvases() while (lnk) { TObject *sc = lnk->GetObject(); lnk = lnk->Next(); - if ((sc->TestBit(kNotDeleted)) && sc == o) + if ((!ROOT::Detail::HasBeenDeleted(sc)) && sc == o) sentalready = kTRUE; } if (!sentalready) { diff --git a/tutorials/net/spyserv.C b/tutorials/net/spyserv.C index 53e8a2ea68eb6..587f1f41a4757 100644 --- a/tutorials/net/spyserv.C +++ b/tutorials/net/spyserv.C @@ -133,7 +133,7 @@ SpyServ::SpyServ() TSocket *s; if ((s = fMon->Select(20)) != (TSocket*)-1) HandleSocket(s); - if (!fCanvas->TestBit(TObject::kNotDeleted)) + if (ROOT::Detail::HasBeenDeleted(fCanvas)) break; if (gROOT->IsInterrupted()) break;