Advertisement

asASSERT abort on delegate early destruction after Release()

Started by January 29, 2020 09:56 PM
12 comments, last by robertomurta 4 years, 9 months ago

Hi, I'm having an issue trying to use delegates on script callback functions as the example in https://www.angelcode.com/angelscript/sdk/docs/manual/doc_callbacks.html

Context: I have a C++ Class for networking, that I could mostly wrap for access in angelscript. Then I need some SetOnConnect, SetOnDisconnect, SetOnMessage functions to bind in Angelscript to call some AS class methods to handle the callbacks.

Following the simplest example, passing a function, it works flawless, but when using the delegate example saving the function, object and type, after doing the cb->Release() line, the application crashes.

Debugging showed that the SetOnConnectCallback(asIScriptFunction* cb) runs ok, cb->Release() decrements the reference counter, but to a value of 2, and it is not deleted.
However, somepoint in “vctools\crt\vcstartup\src\rtc\stack.cpp” which is not available, the address of cb parameter vanishes. Then the garbage collector tries do collect it and fails on as_scriptfunction.cpp line 1646, as content of cb is invalid.

If I comment cb->Release() line, everything seems to work, except that when destroying the object, it won't destroy C++ defined counterpart, nor run it's destructor, and every time I load the scene, memory keeps ramping up.

C++ snippet

Class WebsocketClient : ... 
{
...
	void SetOnConnectCallback(asIScriptFunction* cb)
	{

		// Release the previous callback, if any
		if (m_on_connect_callback)
			m_on_connect_callback->Release();
		if (m_on_connect_callbackObject)
			ETHScriptWrapper::m_pASEngine->ReleaseScriptObject(m_on_connect_callbackObject, m_on_connect_callbackObjectType);
		m_on_connect_callback = 0;
		m_on_connect_callbackObject = 0;
		m_on_connect_callbackObjectType = 0;

		if (cb && cb->GetFuncType() == asFUNC_DELEGATE)
		{
			m_on_connect_callbackObject = cb->GetDelegateObject();
			m_on_connect_callbackObjectType = cb->GetDelegateObjectType();
			m_on_connect_callback = cb->GetDelegateFunction();

			// Hold on to the object and method
			ETHScriptWrapper::m_pASEngine->AddRefScriptObject(m_on_connect_callbackObject, m_on_connect_callbackObjectType);
			m_on_connect_callback->AddRef();

			// Release the delegate, since it won't be used anymore
			cb->Release(); //// That's the problematic line
		}
		else
		{
			// Store the received handle for later use
			m_on_connect_callback = cb;

			// Do not release the received script function 
			// until it won't be used any more
		}
	}
...
}

Angelscript snippet:

funcdef void CALLBACK();

class GameScene : Scene
{
	private ...
	...
	private WebsocketClient@ m_room;

	GameScene()
	{
		const string sceneName = "scenes/field.esc";
		super(sceneName);
	}

	void PrintConnected()
	{
		print("AS >>> is connected! ");
	}

	void onCreated()
	{
		SetGravity(vector2(0.0f));

		...

		@m_room = WebsocketClient();

		//CALLBACK @func = CALLBACK(this.PrintConnected);
		//m_room.SetOnConnect(@func);
		//m_room.SetOnConnect(CALLBACK(this.PrintConnected));
		m_room.SetOnConnect(CALLBACK(PrintConnected));

		if(!m_room.IsConnected())
			m_room.Connect("localhost", "1201");
	}
    ...
}

I'll see if I can reproduce this problem.

In the meantime, does the problem also happen if you do not break up the delegate, and instead keep the delegate itself as the callback, just as an ordinary function? I.e.:

Class WebsocketClient : ... 
{
...
	void SetOnConnectCallback(asIScriptFunction* cb)
	{

		// Release the previous callback, if any
		if (m_on_connect_callback)
			m_on_connect_callback->Release();

		// Store the received handle for later use
		m_on_connect_callback = cb;

		// Do not release the received script function 
		// until it won't be used any more
	}
...
}

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game

Advertisement

No, it breaks at line 424 of as_context.cpp:

 // Make sure the function is from the same engine as the context to avoid mixups
 if( m_engine != func->GetEngine() )
 {

Exception thrown: read access violation.

func-> was 0xDDDDDDDD.

____
Observation: I didn't mentioned before that we are using asCALL_GENERIC, of course with the wrappers, hope that's ok.

asDECLARE_METHOD_WRAPPERPR(WebsocketClient__SetOnConnectCallback, WebsocketClient, SetOnConnectCallback, (asIScriptFunction* cb), void);
...

void RegisterWebSocketClient(asIScriptEngine* pASEngine)
{
	int r;
	...
	r = pASEngine->RegisterObjectMethod("WebsocketClient", "void SetOnConnect(CALLBACK_void @)", asFUNCTION(WebsocketClient__SetOnConnectCallback), asCALL_GENERIC); assert(r >= 0);
}

It shouldn't matter if you use the generic calling convention. But just to confirm, can you show me the asDECLARE _METHOD _WRAPPERPR macro?

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game

I wrote a short test, and unfortunately I wasn't able to reproduce the problem. Perhaps the problem is within the code generated by the asDECLARE _METHOD _WRAPPERPR that you use.

Here's the code that I tested:

asIScriptFunction *cb = 0;
void *cbo = 0;
asITypeInfo *cbot = 0;
void SetCB(asIScriptGeneric *gen)
{
	if( cb )
		cb->Release();
	if( cbo )
	{
		gen->GetEngine()->ReleaseScriptObject(cbo, cbot);
		cbo = cbot = 0;
	}
	asIScriptFunction *f = (asIScriptFunction*)gen->GetArgAddress(0);
	if( f && f->GetFuncType() == asFUNC_DELEGATE )
	{
		cbo = f->GetDelegateObject();
		cbot = f->GetDelegateObjectType();
		cb = f->GetDelegateFunction();
		
		// Hold on to the objects
		gen->GetEngine()->AddRefScriptObject(cbo, cbot);
		cb->AddRef();
		
		// Release the delegate, which we don't need
		f->Release();
	}
	else
		cb = f;
}

bool Test()
{
	RET_ON_MAX_PORT

	bool fail = false;
	int r;
	COutStream out;
	asIScriptEngine *engine;
	asIScriptModule *mod;
	asIScriptContext *ctx;
	CBufferedOutStream bout;
	
	// Test delegates as callbacks
	// https://www.gamedev.net/forums/topic/705573-asassert-abort-on-delegate-early-destruction-after-release/5420521/
	{
		engine = asCreateScriptEngine();
		engine->SetMessageCallback(asMETHOD(COutStream, Callback), &out, asCALL_THISCALL);
		
		engine->RegisterFuncdef("void CB()");
		engine->RegisterGlobalFunction("void SetCB(CB @)", asFUNCTION(SetCB), asCALL_GENERIC);
		
		mod = engine->GetModule("test", asGM_ALWAYS_CREATE);
		mod->AddScriptSection("test",
		"class Test { \n"
		"  void cb() { \n"
		"  } \n"
		"  int cbCount = 0; \n"
		"  Test() { \n"
		"    SetCB(CB(cb)); \n"
		"  } \n"
		"} \n"
		"Test t; \n");
		r = mod->Build();
		if( r < 0 )
			TEST_FAILED;
		
		if( cb == 0 || cbo == 0 || cbot == 0 )
			TEST_FAILED;

		ctx = engine->CreateContext();
		ctx->Prepare(cb);
		ctx->SetObject(cbo);
		r = ctx->Execute();
		if( r != asEXECUTION_FINISHED )
			TEST_FAILED;
		ctx->Release();

		if( cb )
		{
			cb->Release();
			cb = 0;
		}
		
		if( cbo )
		{
			engine->ReleaseScriptObject(cbo, cbot);
			cbo = cbot = 0;
		}

		engine->ShutDownAndRelease();
	}

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game

the macro definition:

#define asDECLARE_METHOD_WRAPPERPR(wrapper_name,cl,func,params,rettype) \
    static void wrapper_name(asIScriptGeneric *gen)\
    { \
        asCallWrappedFunc((rettype (cl::*)params)(&amp;cl::func),gen);\
    }

should I be using ->GetArgAddress(0); or receiving (asIScriptGeneric *gen) as parameter instead of (asIScriptFunction* cb)?

other, maybe relevant or not, information:
I'm calling the method in a nested context, (which works when I don't use delegate, or simply don't release it).
The class where SetCallback is defined uses std::enable_shared_from_this
AngelScript v2.32.0 DEBUG options: AS_MAX_PORTABILITY AS_NO_THREADS AS_WIN AS_X86

Advertisement

I presume this asCallWrappedFunc that you have is a template? Can you show the implementation of it? I cannot evaluate if it is working properly if I cannot see the code.

robertomurta said:
should I be using ->GetArgAddress(0); or receiving (asIScriptGeneric *gen) as parameter instead of (asIScriptFunction* cb)?

This is probably what is done already inside your asCallWrappedFunc.

robertomurta said:
v2.32.0

This is relevant. A lot has changed since Dec, 2017, including a lot of bug fixes. For example, the generic calling convention has been modified to make it behave exactly like the native calling conventions with regards to reference counting.

With the old version you use the generic calling convention will automatically call Release on the argument after returning, so your SetOnConnectCallback method must not call Release() on cb for the delegate, and when it is not a delegate it must call AddRef() to take ownership of the handle.

	void SetOnConnectCallback(asIScriptFunction* cb)
	{

		// Release the previous callback, if any
		if (m_on_connect_callback)
			m_on_connect_callback->Release();
		if (m_on_connect_callbackObject)
			ETHScriptWrapper::m_pASEngine->ReleaseScriptObject(m_on_connect_callbackObject, m_on_connect_callbackObjectType);
		m_on_connect_callback = 0;
		m_on_connect_callbackObject = 0;
		m_on_connect_callbackObjectType = 0;

		if (cb && cb->GetFuncType() == asFUNC_DELEGATE)
		{
			m_on_connect_callbackObject = cb->GetDelegateObject();
			m_on_connect_callbackObjectType = cb->GetDelegateObjectType();
			m_on_connect_callback = cb->GetDelegateFunction();

			// Hold on to the object and method
			ETHScriptWrapper::m_pASEngine->AddRefScriptObject(m_on_connect_callbackObject, m_on_connect_callbackObjectType);
			m_on_connect_callback->AddRef();

			// Release the delegate, since it won't be used anymore
			// cb->Release(); //// <- for 2.32.2 we must not release here, since the generic calling convention will do that for us
		}
		else
		{
			// Store the received handle for later use
			m_on_connect_callback = cb;

			// Do not release the received script function 
			// until it won't be used any more
			cb->AddRef(); // <- for 2.32.2 it is necessary to increment the ref count since the generic calling convention will release the handle received in the argument
		}
	}

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game

I recommend you upgrade to version 2.34.0 to take advantage of the last 2 years of bug fixes.

If you use generic calling convention extensively and don't want to spend time going through all of the wrappers to update to the new behaviour, you can make angelscript 2.34.0 behave as 2.32.0 by setting the engine property asEP_GENERIC_CALL_MODE to 0.

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game

it is on aswrappedcall.h (https://pastebin.com/MGCthFbP) from addons of that version, a long list of templates.

static void asWrapNative_p0_void(void (*func)(),asIScriptGeneric *)
{
    func( );
}

Upgrading is desirable, I think I'll try doing it, can be heculeous, then I'll update this thread

Ah, you're using the previous implementation of the auto-wrappers, replaced by the current implementation in 2012. I thought it was familiar, but I had completely forgotten about this old version.

It should still work though. For your SetOnConnectCallback it would evaluate to the following implementation:

template<typename C,typename T1>
static void asWrapNative_p1_void_this(void (C::*func)(T1),asIScriptGeneric *gen)
{
    ((*((C*)gen->GetObject())).*func)( ((as_wrapNative_helper<T1> *)gen->GetAddressOfArg(0))->d );
}

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game

This topic is closed to new replies.

Advertisement