Skip to content

Commit

Permalink
Fix #7513: recursive array/class/table release caused stack overflow
Browse files Browse the repository at this point in the history
  • Loading branch information
rubidium42 committed Apr 16, 2021
1 parent 8c9a154 commit 86eb17b
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 21 deletions.
12 changes: 8 additions & 4 deletions src/3rdparty/squirrel/squirrel/sqarray.h
Expand Up @@ -17,9 +17,9 @@ struct SQArray : public CHAINABLE_OBJ
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)
Expand Down Expand Up @@ -78,9 +78,13 @@ struct SQArray : public CHAINABLE_OBJ
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;
};
Expand Down
16 changes: 9 additions & 7 deletions src/3rdparty/squirrel/squirrel/sqclass.h
Expand Up @@ -126,31 +126,33 @@ struct SQInstance : public SQDelegable
}
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;
Expand Down
22 changes: 15 additions & 7 deletions src/3rdparty/squirrel/squirrel/sqobject.h
Expand Up @@ -2,7 +2,7 @@
#ifndef _SQOBJECT_H_
#define _SQOBJECT_H_

#include <forward_list>
#include <vector>
#include "squtils.h"

#define SQ_CLOSURESTREAM_HEAD (('S'<<24)|('Q'<<16)|('I'<<8)|('R'))
Expand Down Expand Up @@ -350,26 +350,34 @@ 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() {}
};

/**
* Helper container for state to change the garbage collection from a recursive to an iterative approach.
* The iterative approach provides effectively a depth first search approach.
*/
class SQGCMarkerQueue {
std::forward_list<SQCollectable*> queue; ///< The queue of elements to still process.
std::vector<SQCollectable*> 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;
}

Expand All @@ -382,7 +390,7 @@ class SQGCMarkerQueue {
{
if ((collectable->_uiRef & MARK_FLAG) == 0) {
collectable->_uiRef |= MARK_FLAG;
this->queue.push_front(collectable);
this->stack.push_back(collectable);
}
}
};
Expand Down
29 changes: 29 additions & 0 deletions src/3rdparty/squirrel/squirrel/sqstate.cpp
Expand Up @@ -99,6 +99,7 @@ SQSharedState::SQSharedState()
_notifyallexceptions = false;
_scratchpad=NULL;
_scratchpadsize=0;
_collectable_free_processing = false;
#ifndef NO_GARBAGE_COLLECTOR
_gc_chain=NULL;
#endif
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions src/3rdparty/squirrel/squirrel/sqstate.h
Expand Up @@ -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);
Expand All @@ -74,6 +75,10 @@ struct SQSharedState
SQObjectPtr _registry;
SQObjectPtr _consts;
SQObjectPtr _constructoridx;
/** Queue to make freeing of collectables iterative. */
std::vector<SQCollectable *> _collectable_free_queue;
/** Whether someone is already processing the _collectable_free_queue. */
bool _collectable_free_processing;
#ifndef NO_GARBAGE_COLLECTOR
SQCollectable *_gc_chain;
#endif
Expand Down
10 changes: 7 additions & 3 deletions src/3rdparty/squirrel/squirrel/sqtable.h
Expand Up @@ -50,7 +50,7 @@ struct SQTable : public SQDelegable
newtable->_delegate = NULL;
return newtable;
}
void Finalize();
void Finalize() override;
SQTable *Clone();
~SQTable()
{
Expand All @@ -60,7 +60,7 @@ struct SQTable : public SQDelegable
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)
{
Expand All @@ -81,7 +81,11 @@ struct SQTable : public SQDelegable

SQInteger CountUsed(){ return _usednodes;}
void Clear();
void Release()
void Release() override
{
this->_sharedstate->DelayFinalFree(this);
}
void FinalFree() override
{
sq_delete(this, SQTable);
}
Expand Down

0 comments on commit 86eb17b

Please sign in to comment.