Duplicated call of releasehook was fixed.

[baldale]

It seems that GetRefCount method breaks read semantics. Since it will add reference to object in case it missing.
I have recived this problem when called this method after my native class was destructed .
Thi method added reference into RefTable and relase hoook was called second time => I'v got dupliucated delition of my object.
So it seems it shouldn't add reference during call

[mingodad]

Hello Alexey !
Could you provide a code example that shows the problem ?
Cheers !

[baldale]

Hi.
Sorry for delay.
I had to exctract code from my project to reproduce the problem.
I know that it quite ugly and calling sq_getrefcount during vm close operation, but it cause program crash. But it depends on hash of object and has no 100% chance to fail.

The adding of redundant referece occures at line below //FIXME comment:

#include <squirrel.h>
#include <sqstdblob.h>
#include <sqstdsystem.h>
#include <sqstdio.h>
#include <sqstdmath.h>
#include <sqstdstring.h>
#include <sqstdaux.h>
#include <stdio.h>

#ifdef SQUNICODE
#define scfprintf fwprintf
#define scvprintf vfwprintf
#else
#define scfprintf fprintf
#define scvprintf vfprintf
#endif

SQInteger quit(HSQUIRRELVM v)
{
int done;
sq_getuserpointer(v,-1,(SQUserPointer)&done);
*done=1;
return 0;
}

void printfunc(HSQUIRRELVM SQ_UNUSED_ARG(v),const SQChar *s,...)
{
va_list vl;
va_start(vl, s);
scvprintf(stdout, s, vl);
va_end(vl);
}

void errorfunc(HSQUIRRELVM SQ_UNUSED_ARG(v),const SQChar *s,...)
{
va_list vl;
va_start(vl, s);
scvprintf(stderr, s, vl);
va_end(vl);
}

HSQUIRRELVM global_vm;
HSQOBJECT callback_cls;
HSQOBJECT test_mock_cls;
HSQOBJECT callback;

class Callback
{
public:
Callback() {printf("Callback()\n");}
~Callback() {printf("~Callback()\n");}
};

class Mock
{
public:
Mock() {printf("Mock()\n");}
~Mock() {printf("~Mock()\n");}
};

int test_mock_destructor(SQUserPointer pointer, SQInteger size)
{
Mock* m = reinterpret_cast<Mock*>(pointer);
delete m;
return SQFalse;
}

int callback_destructor(SQUserPointer pointer, SQInteger size)
{
Callback* c = reinterpret_cast<Callback*>(pointer);
delete c;

// FIXME
printf("%d\n", sq_getrefcount(global_vm, &callback));
return SQFalse;

}

int test_mock_constructor(HSQUIRRELVM vm)
{
Mock* m = new Mock();

sq_setinstanceup(vm, 1, m);
sq_setreleasehook(vm, 1, &test_mock_destructor);
sq_pushroottable(vm);
sq_pushstring(vm, "c", -1);
if (SQ_FAILED(sq_get(vm, -2))) {return sq_throwerror(vm, "Unable to get");}
if (SQ_FAILED(sq_getstackobj(vm, -1, &callback))) {return sq_throwerror(vm, "Unable to get");}
sq_addref(vm, &callback);
sq_pop(vm, 1);
return SQFalse;

}

int callback_constructor(HSQUIRRELVM vm)
{
Callback* c = new Callback();
sq_setinstanceup(vm, 1, c);
sq_setreleasehook(vm, 1, &callback_destructor);
return SQFalse;
}

int test_mock_get_name(HSQUIRRELVM vm)
{
SQUserPointer self_ptr;
if (SQ_FAILED(sq_getinstanceup(vm, 1, &self_ptr, NULL))) { return sq_throwerror(vm, "Error");}
sq_pushstring(vm, "my name is Mock!\n", -1);
return SQTrue;
}

void register_class_test_mock(HSQUIRRELVM vm)
{
sq_pushconsttable(vm);
sq_pushstring(vm, "test_mock", -1);
if (SQ_FAILED(sq_newclass(vm, SQFalse))) {return;}
sq_resetobject(&test_mock_cls);
sq_getstackobj(vm, -1, &test_mock_cls);
sq_addref(vm, &test_mock_cls);
// constructor
sq_pushstring(vm, "constructor", -1);
sq_newclosure(vm, &test_mock_constructor, 0);
sq_setparamscheck(vm, 0, NULL);
sq_setnativeclosurename(vm, -1, "constructor");
if (SQ_FAILED(sq_newslot(vm, -3, SQFalse))) {return;}
// get_name
sq_pushstring(vm, "get_name", -1);
sq_newclosure(vm, &test_mock_constructor, 0);
sq_setparamscheck(vm, 0, NULL);
sq_setnativeclosurename(vm, -1, "get_name");
if (SQ_FAILED(sq_newslot(vm, -3, SQFalse))) {return;}
// class
if (SQ_FAILED(sq_newslot(vm, -3, SQFalse))) {return;}

sq_pop(vm, 1);

}

void register_class_callback(HSQUIRRELVM vm)
{
sq_pushconsttable(vm);
sq_pushstring(vm, "callback", -1);
if (SQ_FAILED(sq_newclass(vm, SQFalse))) {return;}
sq_resetobject(&test_mock_cls);
sq_getstackobj(vm, -1, &test_mock_cls);
sq_addref(vm, &test_mock_cls);
// constructor
sq_pushstring(vm, "constructor", -1);
sq_newclosure(vm, &callback_constructor, 0);
sq_setparamscheck(vm, 0, NULL);
sq_setnativeclosurename(vm, -1, "constructor");
if (SQ_FAILED(sq_newslot(vm, -3, SQFalse))) {return;}
// class
if (SQ_FAILED(sq_newslot(vm, -3, SQFalse))) {return;}

sq_pop(vm, 1);

}

void test_case(HSQUIRRELVM vm)
{
global_vm = vm;
register_class_test_mock(vm);
register_class_callback(vm);

const char* src = 
    "counter <- 0\n"
    "c<-callback()\n"
    "class test2 extends test_mock {\n"
    "  function get_name() { return \"my name is Child Mock!\\n\"}\n"
    "}\n"
    "t <- test2()\n"
    "print(t.get_name());";

sq_compilebuffer(vm, src, strlen(src), "src", SQTrue);
sq_pushroottable(vm);
sq_call(vm, 1, SQTrue, SQTrue);

}

int main(int argc, char* argv[])
{
HSQUIRRELVM v;
v=sq_open(1024);
sq_setprintfunc(v,printfunc,errorfunc);
sq_pushroottable(v);

sqstd_register_bloblib(v);
sqstd_register_iolib(v);
sqstd_register_systemlib(v);
sqstd_register_mathlib(v);
sqstd_register_stringlib(v);

test_case(v);

sq_close(v);

}

[baldale]

Any concernes about this fix?

[mingodad]

I followed the references for SQUnsignedInteger RefTable::GetRefCount(SQObject &obj) and found it only used by SQUnsignedInteger sq_getrefcount(HSQUIRRELVM v,HSQOBJECT *po) and looking at the changes made by you it seems that you are right and the actual code in Squirrel is a mistake due to a copy and paste without checking it's correctness (zero test/usage).