[VSCRIPT] Crash on self remove while thinking

[samisalreadytaken]

Describe the bug

Entity killing itself while (script) thinking will cause segfault.

Steps to reproduce

local ent = Entities.CreateByClassname( "info_null" )
ent.SetContextThink("", function(self) { self.Destroy() }, 0.0)

A real use example

local ent = Entities.CreateByClassname( "gib" )
ent.SetLocalOrigin( player.EyePosition() + player.GetEyeForward() * 16 )
ent.SetModel("models/gibs/agibs.mdl")
ent.SetRenderMode( RenderMode.Color )
ent.SetContextThink("FadeOut", function(self)
{
	local a = self.GetRenderAlpha()
	if ( a > 4 )
	{
		printl(a)
		self.SetRenderAlpha( a - 4 )
		return 0.0
	}
	else
	{
		printl("rem " + self)
		UTIL_Remove(self)
	}
}, 0.0)

---

Additional context

UTIL_Remove can be considered a direct callback to CBaseEntity::UpdateOnRemove. When UTIL_Remove is called during script think func iteration in CBaseEntity::ScriptContextThink, these think funcs will be freed inside UpdateOnRemove, then accessed and deleted again in the rest of ScriptContextThink. This was why the deferred deletion added for stopping while thinking.

Fixing this without coming up with an alternative allocation method for scriptthinkfunc_t, or integrating PhysCallbackRemove is easy, but I am not sure if there are any side effects of doing this in ~CBaseEntity.

--- a/sp/src/game/server/baseentity.cpp
+++ b/sp/src/game/server/baseentity.cpp
@@ -451,6 +451,18 @@ extern bool g_bDisableEhandleAccess;
 //-----------------------------------------------------------------------------
 CBaseEntity::~CBaseEntity( )
 {
+#ifdef MAPBASE_VSCRIPT
+       if ( g_pScriptVM )
+       {
+               FOR_EACH_VEC( m_ScriptThinkFuncs, i )
+               {
+                       HSCRIPT h = m_ScriptThinkFuncs[i]->m_hfnThink;
+                       if ( h ) g_pScriptVM->ReleaseScript( h );
+               }
+       }
+       m_ScriptThinkFuncs.PurgeAndDeleteElements();
+#endif
+
        // FIXME: This can't be called from UpdateOnRemove! There's at least one
        // case where friction sounds are added between the call to UpdateOnRemove + ~CBaseEntity
        PhysCleanupFrictionSounds( this );
@@ -2592,15 +2604,6 @@ void CBaseEntity::UpdateOnRemove( void )

                g_pScriptVM->RemoveInstance( m_hScriptInstance );
                m_hScriptInstance = NULL;
-
-#ifdef MAPBASE_VSCRIPT
-               FOR_EACH_VEC( m_ScriptThinkFuncs, i )
-               {
-                       HSCRIPT h = m_ScriptThinkFuncs[i]->m_hfnThink;
-                       if ( h ) g_pScriptVM->ReleaseScript( h );
-               }
-               m_ScriptThinkFuncs.PurgeAndDeleteElements();
-#endif // MAPBASE_VSCRIPT
        }
 }