YetAnotherForum
Welcome Guest Search | Active Topics | Log In | Register

Assert when releasing a Function in debug mode.
SLC
#1 Posted : Sunday, October 25, 2015 12:30:51 AM(UTC)
Rank: Advanced Member

Groups: Registered
Joined: 7/1/2013(UTC)
Posts: 30
Man
Location: Romania

Thanks: 3 times
Was thanked: 0 time(s) in 0 post(s)
I have an event class that has a Sqrat::Function member variable and exposes that through mutators as a property. However, when I try to release that function, I get an exception somewhere in sqstate.cpp.

Take the following code for example:
Code:
local start_up = BasicEvent(EVENTID.SERVERSTARTUP);

start_up.on_trigger = function()
{
    print("Starting");
}

local shut_down = BasicEvent(EVENTID.SERVERSHUTDOWN);

shut_down.on_trigger = function()
{
    print("Stopping");
}


Just before I close my VM I have an event that is triggered and allows the event instance to release the reference to the function and environment. Same behavior without releasing before closing the VM.

For example. This is where I explicitly release that function:
Code:

// ------------------------------------------------------------------------------------------------
void BasicEvent::VMClose() noexcept
{
    LogInf("[BasicEvent::VMClose() %d %d",
        sq_getrefcount(m_OnTrigger.GetVM(), &m_OnTrigger.GetEnv()),
        sq_getrefcount(m_OnTrigger.GetVM(), &m_OnTrigger.GetFunc())
    );

    // Release the reference to the specified callback
    m_OnTrigger.Release();

    LogInf("]BasicEvent::VMClose() %d %d",
        sq_getrefcount(m_OnTrigger.GetVM(), &m_OnTrigger.GetEnv()),
        sq_getrefcount(m_OnTrigger.GetVM(), &m_OnTrigger.GetFunc())
    );
}


Ignore the code I've added to check the reference count.

And this the code where I close the VM:
Code:
void Core::DestroyVM() noexcept
{
    // See if the Virtual Machine wasn't already destroyed
    if (m_VM != nullptr)
    {
        // Let instances know that they should release links to this VM
        VMClose.Emit();
        // Release the references to the script objects
        m_Scripts.clear();
        // Release the reference to the root table
        m_RootTable.reset();
        // Assertions during close may cause double delete
        HSQUIRRELVM sq_vm = m_VM;
        // Explicitly set the virtual machine to null
        m_VM = nullptr;
        // Close the Virtual Machine
        sq_close(sq_vm);
    }
}


The output would be:
Code:

[MSG] Stopping
[INF] [BasicEvent::VMClose() 1 1
Assertion failed!

Program: [Drive]\Game\server.exe
File: [Drive]\SqMod\external\Squirrel\sqstate.cpp, Line 475

Expression: 0
[INF] [BasicEvent::VMClose() 1 1
Assertion failed!

Program: [Drive]\Game\server.exe
File: [Drive]\SqMod\external\Squirrel\sqstate.cpp, Line 475

Expression: 0


So, the reference for the environment and the function is both 1. Anyway, I'm really sorry for the lack of details. But I'm not really sure how to explain this. For example, what's the purpose of that assert:

Code:
SQBool RefTable::Release(SQObject &obj)
{
    SQHash mainpos;
    RefNode *prev;
    RefNode *ref = Get(obj,mainpos,&prev,false);
    if(ref) {
        if(--ref->refs == 0) {
            SQObjectPtr o = ref->obj;
            if(prev) {
                prev->next = ref->next;
            }
            else {
                _buckets[mainpos] = ref->next;
            }
            ref->next = _freelist;
            _freelist = ref;
            _slotused--;
            ref->obj.Null();
            //<<FIXME>>test for shrink?
            return SQTrue;
        }
    }
    else {
        assert(0);
    }
    return SQFalse;
}


And why do I get it when the reference count indicates that the object should be alive. I'm a little confused here and I've been stuck for the past few days.
absence
#2 Posted : Monday, October 26, 2015 5:03:22 PM(UTC)
Rank: Advanced Member

Groups: Registered
Joined: 8/23/2014(UTC)
Posts: 109
Man
Location: Northern Germany & Lincolnshire, U.K.

Thanks: 1 times
Was thanked: 10 time(s) in 10 post(s)
After having a quick glance at some sources I came to the conclusion:
You must NOT call Release() on m_OnTrigger, because m_OnTrigger is a member of your BasicEvent C++ instance and its destructor will call Release(), too. (see sqratFunction.h)

So when ~BasicEvent is called, also ~Function() is called on m_OnTrigger, trying to release the closure AGAIN. And that's why the assertion in sqstate bails out. You do m_OnTrigger.Release(), effectively releasing the squirrel closure (setting it to NULL). Now, on the second call through the C++ destructors the GC does NOT have any reference to it anymore (because it is already null) and asserts.

(I might be wrong, maybe I didn't dig deep enough what exactly Release() does. But for sure, Release() gets called twice on m_OnTrigger, which does not sound right to me)

On a sidenote: Giving this combination of m_OnTrigger being a member, it is really, really necessary that all BasicEvent destructors get called BEFORE sq_close (and not BY sq_close).
(From what I see you do that by clearing the root table first. Not the best way to do things, but well... should do for now. Nevertheless, with this approach you must never ever store a BasicEvent OUTSIDE the roottable, like for example in the const table OR ON THE STACK or whatever comes to your mind I can't imagine right now. Also be very careful where to store additional references in C++).



SLC
#3 Posted : Wednesday, October 28, 2015 1:05:57 AM(UTC)
Rank: Advanced Member

Groups: Registered
Joined: 7/1/2013(UTC)
Posts: 30
Man
Location: Romania

Thanks: 3 times
Was thanked: 0 time(s) in 0 post(s)
Unfortunately 'Release()' checks if the environment and function object are null:
Quote:
void Release() {
if(!IsNull()) {
sq_release(vm, &env);
sq_release(vm, &obj);
sq_resetobject(&env);
sq_resetobject(&obj);
}
}


I actually relied on the class destructor to release the function first. But the same assert happens. I only implemented that manual release trying to see if that was the cause. But it wasn't.

I will look into your second suggestion which is to release my instances before closing the VM. But that would mean to disable my constructors and to rely on a function that creates instances of that type and stores them in a pool somewhere.

I thought that since Sqrat constructs the instances it will also destruct them when the VM is closed and their reference counter reaches 0 since I don't store any reference to them other then the scripts. Either way, I am still nto sure why this happens and I'll continue to look into it.
absence
#4 Posted : Thursday, October 29, 2015 9:11:57 PM(UTC)
Rank: Advanced Member

Groups: Registered
Joined: 8/23/2014(UTC)
Posts: 109
Man
Location: Northern Germany & Lincolnshire, U.K.

Thanks: 1 times
Was thanked: 10 time(s) in 10 post(s)
SLC wrote:

I thought that since Sqrat constructs the instances it will also destruct them when the VM is closed and their reference counter reaches 0 since I don't store any reference to them other then the scripts.


That might be it, I already stated that.
Note that when objects get released through sq_close, the VM is already invalid that very moment. It cannot be used anymore, not even to do a sq_release()!
Check if those objects still exist right before your call of sq_close(), they must not.
When sq_close causes your BasicEvent destructors that's likely to fail (as the Function member will call sq_release then - with an invalid VM)

I'm sorry I currently don't have the time to dig through your code and sqrat.
If i'd be you, I'd set a breakpoint on destructors and see the stacktrace to understand when they're called (and what exactly calls them).

SLC
#5 Posted : Friday, October 30, 2015 12:15:56 PM(UTC)
Rank: Advanced Member

Groups: Registered
Joined: 7/1/2013(UTC)
Posts: 30
Man
Location: Romania

Thanks: 3 times
Was thanked: 0 time(s) in 0 post(s)
Turns out this happens when the reference counter on a script function is 1 and you call sq_release() on it. Need to look further though. Not sure why this happens. Are script functions not supposed to be released?
absence
#6 Posted : Monday, November 2, 2015 12:30:47 PM(UTC)
Rank: Advanced Member

Groups: Registered
Joined: 8/23/2014(UTC)
Posts: 109
Man
Location: Northern Germany & Lincolnshire, U.K.

Thanks: 1 times
Was thanked: 10 time(s) in 10 post(s)
Script functions (closures) are referenced objects like all others, too , otherwise they wouldn't have a refcount at all.
In your case I suspect release gets called _twice_, maybe from a release hook. You really should set breakpoints to see what's happening, like...set a breakpoint to your destructor. When it hits, set another on sq_release, continue and see if and who calls it.
SLC
#7 Posted : Tuesday, November 3, 2015 1:05:43 AM(UTC)
Rank: Advanced Member

Groups: Registered
Joined: 7/1/2013(UTC)
Posts: 30
Man
Location: Romania

Thanks: 3 times
Was thanked: 0 time(s) in 0 post(s)
Unfortunately I can't debug like that since this is a DLL loaded by another application which I cannot control. However, I've managed to solve this by not releasing the reference to the squirrel function if the reference counter is 1.

Pretty much I've added another dummy member function to Sqrat's Function type:
Code:
    void Release2() {
        if(!IsNull()) {
            sq_release(vm, &env);
            if (sq_getrefcount(vm, &obj) > 1)
            {
                sq_release(vm, &obj);
            }
            sq_resetobject(&env);
            sq_resetobject(&obj);
        }
    }


And call that instead when the VMClose() function is called before the VM is closed. If I try this in the destructor I still get crashes. I've probably just introduced a memory leak but I've seen some code in squirrel which lead me to believe the last reference should not be released.

Code:
SQBool sq_release(HSQUIRRELVM v,HSQOBJECT *po)
{
    if(!ISREFCOUNTED(type(*po))) return SQTrue;
#ifdef NO_GARBAGE_COLLECTOR
    bool ret = (po->_unVal.pRefCounted->_uiRef <= 1) ? SQTrue : SQFalse;
    __Release(po->_type,po->_unVal);
    return ret; //the ret val doesn't work(and cannot be fixed)
#else
    return _ss(v)->_refs_table.Release(*po);
#endif
}


More specifically this part:
Code:
bool ret = (po->_unVal.pRefCounted->_uiRef <= 1) ? SQTrue : SQFalse;


Which means that if I assign the function anonymously like this:
Code:
start_up.on_trigger = function()
{
    print("Starting");
}


Then there's only one reference to it. Which is the one in my object and when my object is deleted then the last reference to that function is released.

However, if I save it first and then assign it:
Code:
local my_weird_startup_function_name = function()
{
    print("Starting");
}

start_up.on_trigger = my_weird_startup_function_name ;


Now even if my object is deleted, the script keeps one more reference to the function preventing me from getting a crash.

But I believe these are two different issues because I still get an assert in debug mode. And I must say that this is turning to be a real pain in the a$$.

Either way, I'll see if I can live with this issue when I compile for all platforms and see if at least the release build can live without any weird segmentation fault errors on linux.
Users browsing this topic
Guest
Forum Jump  
You cannot post new topics in this forum.
You cannot reply to topics in this forum.
You cannot delete your posts in this forum.
You cannot edit your posts in this forum.
You cannot create polls in this forum.
You cannot vote in polls in this forum.

Clean Slate theme by Jaben Cargman (Tiny Gecko)
Powered by YAF 1.9.4 | YAF © 2003-2010, Yet Another Forum.NET
This page was generated in 0.094 seconds.