From 44d1b964bf1c87324b82b1793218b5f79a9be9eb Mon Sep 17 00:00:00 2001 From: Rubidium Date: Thu, 15 Apr 2021 19:44:43 +0200 Subject: Fix #7513: recursive array/class/table release caused stack overflow --- src/3rdparty/squirrel/squirrel/sqarray.h | 12 ++++++++---- src/3rdparty/squirrel/squirrel/sqclass.h | 16 +++++++++------- src/3rdparty/squirrel/squirrel/sqobject.h | 22 +++++++++++++++------- src/3rdparty/squirrel/squirrel/sqstate.cpp | 29 +++++++++++++++++++++++++++++ src/3rdparty/squirrel/squirrel/sqstate.h | 5 +++++ src/3rdparty/squirrel/squirrel/sqtable.h | 10 +++++++--- 6 files changed, 73 insertions(+), 21 deletions(-) (limited to 'src/3rdparty/squirrel') diff --git a/src/3rdparty/squirrel/squirrel/sqarray.h b/src/3rdparty/squirrel/squirrel/sqarray.h index 37d91bc4e..13ae11619 100644 --- a/src/3rdparty/squirrel/squirrel/sqarray.h +++ b/src/3rdparty/squirrel/squirrel/sqarray.h @@ -17,9 +17,9 @@ public: return newarray; } #ifndef NO_GARBAGE_COLLECTOR - void EnqueueMarkObjectForChildren(SQGCMarkerQueue &queue); + void EnqueueMarkObjectForChildren(SQGCMarkerQueue &queue) override; #endif - void Finalize(){ + void Finalize() override { _values.resize(0); } bool Get(const SQInteger nidx,SQObjectPtr &val) @@ -78,9 +78,13 @@ public: ShrinkIfNeeded(); return true; } - void Release() + void Release() override { - sq_delete(this,SQArray); + this->_sharedstate->DelayFinalFree(this); + } + void FinalFree() override + { + sq_delete(this, SQArray); } SQObjectPtrVec _values; }; diff --git a/src/3rdparty/squirrel/squirrel/sqclass.h b/src/3rdparty/squirrel/squirrel/sqclass.h index cb0973bbe..4fb6ecbd9 100644 --- a/src/3rdparty/squirrel/squirrel/sqclass.h +++ b/src/3rdparty/squirrel/squirrel/sqclass.h @@ -126,31 +126,33 @@ public: } return false; } - void Release() { + void Release() override { _uiRef++; try { if (_hook) { _hook(_userpointer,0);} } catch (...) { _uiRef--; if (_uiRef == 0) { - SQInteger size = _memsize; - this->~SQInstance(); - SQ_FREE(this, size); + this->_sharedstate->DelayFinalFree(this); } throw; } _uiRef--; if(_uiRef > 0) return; + this->_sharedstate->DelayFinalFree(this); + } + void FinalFree() override + { SQInteger size = _memsize; this->~SQInstance(); SQ_FREE(this, size); } - void Finalize(); + void Finalize() override; #ifndef NO_GARBAGE_COLLECTOR - void EnqueueMarkObjectForChildren(SQGCMarkerQueue &queue); + void EnqueueMarkObjectForChildren(SQGCMarkerQueue &queue) override; #endif bool InstanceOf(SQClass *trg); - bool GetMetaMethod(SQVM *v,SQMetaMethod mm,SQObjectPtr &res); + bool GetMetaMethod(SQVM *v,SQMetaMethod mm,SQObjectPtr &res) override; SQClass *_class; SQUserPointer _userpointer; diff --git a/src/3rdparty/squirrel/squirrel/sqobject.h b/src/3rdparty/squirrel/squirrel/sqobject.h index ed4f31fcb..9212766ee 100644 --- a/src/3rdparty/squirrel/squirrel/sqobject.h +++ b/src/3rdparty/squirrel/squirrel/sqobject.h @@ -2,7 +2,7 @@ #ifndef _SQOBJECT_H_ #define _SQOBJECT_H_ -#include +#include #include "squtils.h" #define SQ_CLOSURESTREAM_HEAD (('S'<<24)|('Q'<<16)|('I'<<8)|('R')) @@ -350,6 +350,14 @@ struct SQCollectable : public SQRefCounted { virtual void Finalize()=0; static void AddToChain(SQCollectable **chain,SQCollectable *c); static void RemoveFromChain(SQCollectable **chain,SQCollectable *c); + + /** + * Helper to perform the final memory freeing of this instance. Since the destructor might + * release more objects, this can cause a very deep recursion. As such, the calls to this + * are to be done via _sharedstate->DelayFinalFree which ensures the calls to this method + * are done in an iterative instead of recursive approach. + */ + virtual void FinalFree() {} }; /** @@ -357,19 +365,19 @@ struct SQCollectable : public SQRefCounted { * The iterative approach provides effectively a depth first search approach. */ class SQGCMarkerQueue { - std::forward_list queue; ///< The queue of elements to still process. + std::vector stack; ///< The elements to still process, with the most recent elements at the back. public: /** Whether there are any elements left to process. */ - bool IsEmpty() { return this->queue.empty(); } + bool IsEmpty() { return this->stack.empty(); } /** - * Remove the first element from the queue. + * Remove the most recently added element from the queue. * Removal when the queue is empty results in undefined behaviour. */ SQCollectable *Pop() { - SQCollectable *collectable = this->queue.front(); - this->queue.pop_front(); + SQCollectable *collectable = this->stack.back(); + this->stack.pop_back(); return collectable; } @@ -382,7 +390,7 @@ public: { if ((collectable->_uiRef & MARK_FLAG) == 0) { collectable->_uiRef |= MARK_FLAG; - this->queue.push_front(collectable); + this->stack.push_back(collectable); } } }; diff --git a/src/3rdparty/squirrel/squirrel/sqstate.cpp b/src/3rdparty/squirrel/squirrel/sqstate.cpp index 9d91c7899..31345d664 100644 --- a/src/3rdparty/squirrel/squirrel/sqstate.cpp +++ b/src/3rdparty/squirrel/squirrel/sqstate.cpp @@ -99,6 +99,7 @@ SQSharedState::SQSharedState() _notifyallexceptions = false; _scratchpad=NULL; _scratchpadsize=0; + _collectable_free_processing = false; #ifndef NO_GARBAGE_COLLECTOR _gc_chain=NULL; #endif @@ -226,6 +227,34 @@ SQInteger SQSharedState::GetMetaMethodIdxByName(const SQObjectPtr &name) return -1; } +/** + * Helper function that is to be used instead of calling FinalFree directly on the instance, + * so the frees can happen iteratively. This as in the FinalFree the references to any other + * objects are released, which can cause those object to be freed yielding a potentially + * very deep stack in case of for example a link list. + * + * This is done internally by a vector onto which the to be freed instances are pushed. When + * this is called when not already processing, this method will actually call the FinalFree + * function which might cause more elements to end up in the queue which this method then + * picks up continueing until it has processed all instances in that queue. + * @param collectable The collectable to (eventually) free. + */ +void SQSharedState::DelayFinalFree(SQCollectable *collectable) +{ + this->_collectable_free_queue.push_back(collectable); + + if (!this->_collectable_free_processing) { + this->_collectable_free_processing = true; + while (!this->_collectable_free_queue.empty()) { + SQCollectable *collectable = this->_collectable_free_queue.back(); + this->_collectable_free_queue.pop_back(); + collectable->FinalFree(); + } + this->_collectable_free_processing = false; + } +} + + #ifndef NO_GARBAGE_COLLECTOR void SQSharedState::EnqueueMarkObject(SQObjectPtr &o,SQGCMarkerQueue &queue) diff --git a/src/3rdparty/squirrel/squirrel/sqstate.h b/src/3rdparty/squirrel/squirrel/sqstate.h index d1a1f8dd6..547e6de47 100644 --- a/src/3rdparty/squirrel/squirrel/sqstate.h +++ b/src/3rdparty/squirrel/squirrel/sqstate.h @@ -61,6 +61,7 @@ struct SQSharedState public: SQChar* GetScratchPad(SQInteger size); SQInteger GetMetaMethodIdxByName(const SQObjectPtr &name); + void DelayFinalFree(SQCollectable *collectable); #ifndef NO_GARBAGE_COLLECTOR SQInteger CollectGarbage(SQVM *vm); static void EnqueueMarkObject(SQObjectPtr &o,SQGCMarkerQueue &queue); @@ -74,6 +75,10 @@ public: SQObjectPtr _registry; SQObjectPtr _consts; SQObjectPtr _constructoridx; + /** Queue to make freeing of collectables iterative. */ + std::vector _collectable_free_queue; + /** Whether someone is already processing the _collectable_free_queue. */ + bool _collectable_free_processing; #ifndef NO_GARBAGE_COLLECTOR SQCollectable *_gc_chain; #endif diff --git a/src/3rdparty/squirrel/squirrel/sqtable.h b/src/3rdparty/squirrel/squirrel/sqtable.h index 0083a90a9..fad2fdc60 100644 --- a/src/3rdparty/squirrel/squirrel/sqtable.h +++ b/src/3rdparty/squirrel/squirrel/sqtable.h @@ -50,7 +50,7 @@ public: newtable->_delegate = NULL; return newtable; } - void Finalize(); + void Finalize() override; SQTable *Clone(); ~SQTable() { @@ -60,7 +60,7 @@ public: SQ_FREE(_nodes, _numofnodes * sizeof(_HashNode)); } #ifndef NO_GARBAGE_COLLECTOR - void EnqueueMarkObjectForChildren(SQGCMarkerQueue &queue); + void EnqueueMarkObjectForChildren(SQGCMarkerQueue &queue) override; #endif inline _HashNode *_Get(const SQObjectPtr &key,SQHash hash) { @@ -81,7 +81,11 @@ public: SQInteger CountUsed(){ return _usednodes;} void Clear(); - void Release() + void Release() override + { + this->_sharedstate->DelayFinalFree(this); + } + void FinalFree() override { sq_delete(this, SQTable); } -- cgit v1.2.3-70-g09d2