Calling "external" functions from a destructor causes a crash

[hexagonrecursion]

To reproduce run the following CBot code:

public class BugBug
{
	void ~BugBug()
	{
		message("Crash Me!");
	}
}

extern void Bug()
{
	BugBug bug = new BugBug();
	bug = null;
}

Traceback:

Thread 1 "colobot" received signal SIGSEGV, Segmentation fault.
0x00005555557a57de in CScriptFunctions::rMessage (var=0x0, result=0x55555a226db0, exception=@0x7fffffffb504: 0, user=0x0) at /home/cdda/git/colobot/colobot-base/src/script
/scriptfunc.cpp:2906

#0  0x00005555557a57de in CScriptFunctions::rMessage (var=0x0, result=0x55555a226db0, exception=@0x7fffffffb504: 0, user=0x0)
    at /home/cdda/git/colobot/colobot-base/src/script/scriptfunc.cpp:2906
#1  0x00007ffff7f178f1 in CBot::CBotExternalCallDefault::Run (this=0x55555689e660, thisVar=0x0, pStack=0x55555a2fad40)
    at /home/cdda/git/colobot/CBot/src/CBot/CBotExternalCall.cpp:160
#2  0x00007ffff7f17556 in CBot::CBotExternalCallList::DoCall (this=0x555556895de0, token=0x55555a0a8ba8, thisVar=0x0, ppVar=0x7fffffffb700, pStack=0x55555a2fad40, 
    rettype=...) at /home/cdda/git/colobot/CBot/src/CBot/CBotExternalCall.cpp:104
#3  0x00007ffff7f537ff in CBot::CBotStack::ExecuteCall (this=0x55555a2fad40, nIdent=@0x55555a0a8c60: 0, token=0x55555a0a8ba8, ppVar=0x7fffffffb700, rettype=...)
    at /home/cdda/git/colobot/CBot/src/CBot/CBotStack.cpp:608
#4  0x00007ffff7f3eb84 in CBot::CBotInstrCall::Execute (this=0x55555a0a8ba0, pj=@0x7fffffffd688: 0x55555a2fac08)
    at /home/cdda/git/colobot/CBot/src/CBot/CBotInstr/CBotInstrCall.cpp:156
#5  0x00007ffff7f4441c in CBot::CBotListInstr::Execute (this=0x55555a0a1210, pj=@0x7fffffffd6f8: 0x55555a2faba0)
    at /home/cdda/git/colobot/CBot/src/CBot/CBotInstr/CBotListInstr.cpp:91
#6  0x00007ffff7f37269 in CBot::CBotFunction::DoCall (nIdent=@0x7fffffffd8b0: 10080, name="~BugBug", pThis=0x55555a223510, ppVars=0x7fffffffd8e8, pStack=0x55555a2faad0, 
    pToken=0x7fffffffd930, pClass=0x55555a22af30) at /home/cdda/git/colobot/CBot/src/CBot/CBotInstr/CBotFunction.cpp:1085
#7  0x00007ffff7f0501e in CBot::CBotClass::ExecuteMethode (this=0x55555a22af30, nIdent=@0x7fffffffd8b0: 10080, pThis=0x55555a223510, ppParams=0x7fffffffd8e8, 
    pResultType=..., pStack=@0x7fffffffd8a8: 0x55555a2faad0, pToken=0x7fffffffd930) at /home/cdda/git/colobot/CBot/src/CBot/CBotClass.cpp:335
#8  0x00007ffff7f6ae63 in CBot::CBotVarClass::DecrementUse (this=0x555556bb33f0) at /home/cdda/git/colobot/CBot/src/CBot/CBotVar/CBotVarClass.cpp:395
#9  0x00007ffff7f6d2d5 in CBot::CBotVarPointer::~CBotVarPointer (this=0x5555569d0f20, __in_chrg=<optimized out>)
    at /home/cdda/git/colobot/CBot/src/CBot/CBotVar/CBotVarPointer.cpp:58
#10 0x00007ffff7f6d300 in CBot::CBotVarPointer::~CBotVarPointer (this=0x5555569d0f20, __in_chrg=<optimized out>)
    at /home/cdda/git/colobot/CBot/src/CBot/CBotVar/CBotVarPointer.cpp:59
#11 0x00007ffff7f52562 in CBot::CBotStack::Delete (this=0x55555a2e15b8) at /home/cdda/git/colobot/CBot/src/CBot/CBotStack.cpp:105
#12 0x00007ffff7f528b8 in CBot::CBotStack::Return (this=0x55555a2e1550, pfils=0x55555a2e1688) at /home/cdda/git/colobot/CBot/src/CBot/CBotStack.cpp:207
#13 0x00007ffff7f303a1 in CBot::CBotExpression::Execute (this=0x555556c3e990, pj=@0x7fffffffdb68: 0x55555a2e1550)
    at /home/cdda/git/colobot/CBot/src/CBot/CBotInstr/CBotExpression.cpp:276
#14 0x00007ffff7f4441c in CBot::CBotListInstr::Execute (this=0x555556c30860, pj=@0x7fffffffdbb8: 0x55555a2e14e8)
    at /home/cdda/git/colobot/CBot/src/CBot/CBotInstr/CBotListInstr.cpp:91
#15 0x00007ffff7f34553 in CBot::CBotFunction::Execute (this=0x55555a22a940, ppVars=0x0, pj=@0x55555a225f38: 0x55555a2e1480, pInstance=0x55555a0a6f70)
    at /home/cdda/git/colobot/CBot/src/CBot/CBotInstr/CBotFunction.cpp:448
#16 0x00007ffff7f4f90c in CBot::CBotProgram::Run (this=0x55555a225f00, pUser=0x55555a0a9660, timer=100) at /home/cdda/git/colobot/CBot/src/CBot/CBotProgram.cpp:198
#17 0x0000555555958bb7 in CScript::Continue (this=0x55555a0a9660) at /home/cdda/git/colobot/colobot-base/src/script/script.cpp:362
#18 0x00005555558cd9dc in CProgrammableObjectImpl::EventProcess (this=0x55555a0aa788, event=...)
    at /home/cdda/git/colobot/colobot-base/src/object/implementation/programmable_impl.cpp:81
#19 0x000055555576cd23 in COldObject::EventProcess (this=0x55555a0aa690, event=...) at /home/cdda/git/colobot/colobot-base/src/object/old_object.cpp:2329
#20 0x00005555556e711b in CRobotMain::EventFrame (this=0x55555688e5e0, event=...) at /home/cdda/git/colobot/colobot-base/src/level/robotmain.cpp:2412
#21 0x00005555556dfb80 in CRobotMain::ProcessEvent (this=0x55555688e5e0, event=...) at /home/cdda/git/colobot/colobot-base/src/level/robotmain.cpp:689
#22 0x00005555555b7caa in CController::ProcessEvent (this=0x5555565701a0, event=...) at /home/cdda/git/colobot/colobot-base/src/app/controller.cpp:53
#23 0x0000555555595ee9 in CApplication::Run (this=0x7fffffffe5d0) at /home/cdda/git/colobot/colobot-base/src/app/app.cpp:1193
#24 0x000055555558b360 in main (argc=3, argv=0x7fffffffe978) at /home/cdda/git/colobot/colobot-app/src/main.cpp:173

script is null here:

colobot/colobot-base/src/script/scriptfunc.cpp

Line 2906 in e00f99f

script->m_main->GetDisplayText()->DisplayText(p.c_str(), script->m_object, 10.0f, type);

This appears to be because CBotVarClass::DecrementUse() creates a new stack with CBotStack::AllocateStack(), but does not call SetUserPtr():

colobot/CBot/src/CBot/CBotVar/CBotVarClass.cpp

Lines 372 to 404 in e00f99f

	void CBotVarClass::DecrementUse()
	{
	m_CptUse--;
	if ( m_CptUse == 0 )
	{
	// if there is one, call the destructor
	// but only if a constructor had been called.
	if ( m_bConstructor )
	{
	m_CptUse++; // does not return to the destructor
	CBotStack* pile = CBotStack::AllocateStack();
	CBotVar* ppVars[1];
	ppVars[0] = nullptr;
	CBotVar* pThis = CBotVar::Create("this", CBotTypNullPointer);
	pThis->SetPointer(this);
	std::string nom = std::string("~") + m_pClass->GetName();
	long ident = 0;
	CBotToken token(nom); // TODO
	while ( pile->IsOk() && !m_pClass->ExecuteMethode(ident, pThis, ppVars, CBotTypResult(CBotTypVoid), pile, &token)) ; // waits for the end
	pile->Delete();
	delete pThis;
	m_CptUse--;
	}
	delete this; // self-destructs!
	}
	}

game version: e00f99f

Edit:

I am wandering: what is the purpose of destructors in CBot? We do not need destructors for memory management because memory is managed by reference counting. Other use-cases for destructors are niche and can be served by try-finally. Why do we bend over backwards to support this feature? Can we just delete it?

[hexagonrecursion]

Destructors are a strange wild beast that breeds bugs and edge cases. Here is one tricky edge case: a destructor can be executed when no CBot script is running:

1. Add this script to a bot

   public class MyClass
   {
   	static public int counter = 0; // instance counter
   	
   	void  MyClass( )
   	{
   		counter++;  // one instance more
   	}
   	void ~MyClass( )
   	{
   		counter--;  // one instance less
   	}
   }
   extern void PrintCount()
   {
   	MyClass c();
   	message(c.counter - 1);
   }

2. Add this script to the same bot and run it once

   public class HoldMyBear
   {
   	static public MyClass c;
   }
   extern void Use()
   {
   	HoldMyBear h();
   	h.c = new MyClass();
   }

3. Run the first script. It should print 1 because one instance of MyClass is statically stored by the second script
4. Delete the second script. Note that neither script is running at this point
5. Run the first script again. It should print 0 because the instance have been destroyed and the destructor decremented the counter