Skip to content

fix vscript memory leaks #1963

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bottiger1 wants to merge 1 commit into
base: master
Choose a base branch
from
+72 −11
Open

fix vscript memory leaks #1963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions src/game/server/baseentity.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1737,12 +1737,18 @@ int CBaseEntity::TakeDamage( const CTakeDamageInfo &inputInfo )

if ( RunScriptHook( "OnTakeDamage", varTable ) )
{
info.SetInflictor( ToEnt( pVM->Get<HSCRIPT>( varTable, "inflictor" ) ) );
info.SetWeapon( ToEnt( pVM->Get<HSCRIPT>( varTable, "weapon" ) ) );
info.SetAttacker( ToEnt( pVM->Get<HSCRIPT>( varTable, "attacker" ) ) );
// HSCRIPT results use GetValue + ReleaseValue: the Get<HSCRIPT> specialization
// returns a handle that aliases the variant, so it intentionally does not release.
// Get<Vector>/Get<float>/Get<int> use the generic template, which releases for us.
ScriptVariant_t vr;
if ( pVM->GetValue( varTable, "inflictor", &vr ) ) info.SetInflictor( ToEnt( (HSCRIPT)vr ) ); pVM->ReleaseValue( vr );
if ( pVM->GetValue( varTable, "weapon", &vr ) ) info.SetWeapon( ToEnt( (HSCRIPT)vr ) ); pVM->ReleaseValue( vr );
if ( pVM->GetValue( varTable, "attacker", &vr ) ) info.SetAttacker( ToEnt( (HSCRIPT)vr ) ); pVM->ReleaseValue( vr );
info.SetDamage( pVM->Get<float>( varTable, "damage" ) );
info.SetMaxDamage( pVM->Get<float>( varTable, "max_damage" ) );
info.SetDamageBonus( pVM->Get<float>( varTable, "damage_bonus" ), ToEnt( pVM->Get<HSCRIPT>( varTable, "damage_bonus_provider" ) ) );
CBaseEntity *pBonusProvider = NULL;
if ( pVM->GetValue( varTable, "damage_bonus_provider", &vr ) ) pBonusProvider = ToEnt( (HSCRIPT)vr ); pVM->ReleaseValue( vr );
info.SetDamageBonus( pVM->Get<float>( varTable, "damage_bonus" ), pBonusProvider );
info.SetDamageForce( pVM->Get<Vector>( varTable, "damage_force" ) );
info.SetDamageForForceCalc( pVM->Get<float>( varTable, "damage_for_force_calc" ) );
info.SetDamagePosition( pVM->Get<Vector>( varTable, "damage_position" ) );
Expand All @@ -1755,9 +1761,15 @@ int CBaseEntity::TakeDamage( const CTakeDamageInfo &inputInfo )
info.SetPlayerPenetrationCount( pVM->Get<int>( varTable, "player_penetration_count" ) );
info.SetDamagedOtherPlayers( pVM->Get<int>( varTable, "damaged_other_players" ) );
info.SetCritType( (CTakeDamageInfo::ECritType) pVM->Get<int>( varTable, "crit_type" ) );

if ( pVM->Get<bool>( varTable, "early_out" ) )
{
pVM->ReleaseValue( varTable );
return info.GetDamage();
}
}

pVM->ReleaseValue( varTable );
}

return OnTakeDamage( info );
Expand Down
44 changes: 38 additions & 6 deletions src/game/server/vscript_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,13 @@ void CVScriptGameEventListener::FireGameEvent( IGameEvent *event )
// Pass all keyvales as a table of parameters
HSCRIPT paramsTable = ScriptTableFromKeyValues( g_pScriptVM, event->GetDataKeys() );
RunGameEventCallbacks( event->GetName(), paramsTable );

if ( paramsTable )
{
ScriptVariant_t varTable;
varTable = paramsTable;
g_pScriptVM->ReleaseValue( varTable );
}
}

void CVScriptGameEventListener::ListenForScriptHook( const char* szName )
Expand Down Expand Up @@ -1159,6 +1166,8 @@ bool ScriptSendGlobalGameEvent( const char *szName, HSCRIPT params )
if ( vKey.GetType() != FIELD_CSTRING )
{
Log_Msg( LOG_VScript, "VSCRIPT: ScriptSendRealGameEvent: Key must be a FIELD_CSTRING" );
g_pScriptVM->ReleaseValue( vKey );
g_pScriptVM->ReleaseValue( vValue );
continue;
}
const char *pszKeyName = (const char *)vKey;
Expand All @@ -1175,6 +1184,8 @@ bool ScriptSendGlobalGameEvent( const char *szName, HSCRIPT params )
break;
}
}
g_pScriptVM->ReleaseValue( vKey );
g_pScriptVM->ReleaseValue( vValue );
}
}

Expand Down Expand Up @@ -1479,6 +1490,9 @@ static void Script_EmitSoundEx( HSCRIPT params )

EmitSound_t soundParams;

// IfHas<T> releases its temporary ScriptVariant_t via Get<T>, so the value-type reads are
// leak-free. The HSCRIPT lookups use GetValue + ReleaseValue because the Get<HSCRIPT>
// specialization returns an aliasing handle and intentionally does not release.
IScriptVM *pVM = g_pScriptVM;
pVM->IfHas<int> ( params, "channel", [&](int value) { soundParams.m_nChannel = value; } );
pVM->IfHas<CUtlString> ( params, "sound_name", [&](CUtlString value) { szSoundName = std::move( value ); soundParams.m_pSoundName = szSoundName.Get(); });
Expand All @@ -1490,10 +1504,11 @@ static void Script_EmitSoundEx( HSCRIPT params )
pVM->IfHas<Vector> ( params, "origin", [&](Vector value) { vecOriginCache = value; bSetOrigin = true; soundParams.m_pOrigin = &vecOriginCache; } );
pVM->IfHas<float> ( params, "delay", [&](float value) { soundParams.m_flSoundTime = gpGlobals->curtime + value; } );
pVM->IfHas<float> ( params, "sound_time", [&](float value) { soundParams.m_flSoundTime = value; } );
pVM->IfHas<HSCRIPT> ( params, "speaker_entity", [&](HSCRIPT value) { CBaseEntity *pEntity = ToEnt( value ); if ( pEntity ) soundParams.m_nSpeakerEntity = pEntity->entindex(); });
pVM->IfHas<HSCRIPT> ( params, "entity", [&](HSCRIPT value) { pOutputEntity = ToEnt( value ); });
pVM->IfHas<int> ( params, "filter_type", [&](int value) { eFilter = EScriptRecipientFilter( value ); });
pVM->IfHas<int> ( params, "filter_param", [&](int value) { nFilterParam = value; });
ScriptVariant_t v;
if ( pVM->GetValue( params, "speaker_entity", &v ) ) { CBaseEntity *pEntity = ToEnt( (HSCRIPT)v ); if ( pEntity ) soundParams.m_nSpeakerEntity = pEntity->entindex(); } pVM->ReleaseValue( v );
if ( pVM->GetValue( params, "entity", &v ) ) pOutputEntity = ToEnt( (HSCRIPT)v ); pVM->ReleaseValue( v );
soundParams.m_bWarnOnMissingCloseCaption = false;
soundParams.m_bWarnOnDirectWaveReference = false;

Expand All @@ -1517,6 +1532,7 @@ static bool Script_PrecacheItemFromTable( HSCRIPT hSpawnTable )
}
// CBaseEntity *pEntity = VScript_ParseEntity( pszClassname, spawn_table );
CBaseEntity *pEntity = VScript_ParseEntity( vClassname, hSpawnTable );
g_pScriptVM->ReleaseValue( vClassname );
if ( pEntity )
{
DispatchSpawn( pEntity, false );
Expand Down Expand Up @@ -1778,9 +1794,12 @@ static const char *Script_FileToString( const char *pszFileName )

void TraceToScriptVM( HSCRIPT hTable, Vector vStart, Vector vEnd, trace_t& tr )
{
ScriptVariant_t v;

g_pScriptVM->SetValue( hTable, "fraction", tr.fraction );
g_pScriptVM->SetValue( hTable, "hit", (tr.fraction != 1.0 || tr.startsolid ));
g_pScriptVM->SetValue( hTable, "plane_normal", tr.plane.normal );
v = tr.plane.normal;
g_pScriptVM->SetValue( hTable, "plane_normal", v );
g_pScriptVM->SetValue( hTable, "plane_dist", tr.plane.dist );
g_pScriptVM->SetValue( hTable, "contents", tr.contents );
if (tr.allsolid)
Expand All @@ -1790,9 +1809,12 @@ void TraceToScriptVM( HSCRIPT hTable, Vector vStart, Vector vEnd, trace_t& tr )
if (tr.startsolid)
g_pScriptVM->SetValue( hTable, "startsolid", tr.startsolid);
Vector vPos = vStart + ( tr.fraction * (vEnd - vStart));
g_pScriptVM->SetValue( hTable, "pos", vPos );
g_pScriptVM->SetValue( hTable, "startpos", tr.startpos );
g_pScriptVM->SetValue( hTable, "endpos", tr.endpos );
v = vPos;
g_pScriptVM->SetValue( hTable, "pos", v );
v = tr.startpos;
g_pScriptVM->SetValue( hTable, "startpos", v );
v = tr.endpos;
g_pScriptVM->SetValue( hTable, "endpos", v );
g_pScriptVM->SetValue( hTable, "surface_name", tr.surface.name ? tr.surface.name : "" );
g_pScriptVM->SetValue( hTable, "surface_flags", tr.surface.flags );
g_pScriptVM->SetValue( hTable, "surface_props", tr.surface.surfaceProps );
Expand All @@ -1812,12 +1834,15 @@ static bool Script_TraceLineEx( HSCRIPT hTable )
vStart = rval;
else
bNoParams = true;
g_pScriptVM->ReleaseValue( rval );
if (g_pScriptVM->GetValue( hTable, "end", &rval ) )
vEnd = rval;
else
bNoParams = true;
g_pScriptVM->ReleaseValue( rval );
if (g_pScriptVM->GetValue( hTable, "mask", &rval ))
mask = rval;
g_pScriptVM->ReleaseValue( rval );
if (bNoParams)
{
Warning("Didnt supply start and end to Script TraceLine call, failing, setting called to false\n");
Expand All @@ -1828,6 +1853,7 @@ static bool Script_TraceLineEx( HSCRIPT hTable )
{
pIgnoreEnt = ToEnt((HSCRIPT)rval);
}
g_pScriptVM->ReleaseValue( rval );
trace_t tr;
UTIL_TraceLine( vStart, vEnd, mask, pIgnoreEnt, coll, &tr );

Expand All @@ -1851,21 +1877,25 @@ static bool Script_TraceHull( HSCRIPT hTable )
vStart = rval;
else
bNoParams = true;
g_pScriptVM->ReleaseValue( rval );

if (g_pScriptVM->GetValue( hTable, "end", &rval ) )
vEnd = rval;
else
bNoParams = true;
g_pScriptVM->ReleaseValue( rval );

if (g_pScriptVM->GetValue( hTable, "hullmin", &rval ) )
vHullMin = rval;
else
bNoParams = true;
g_pScriptVM->ReleaseValue( rval );

if (g_pScriptVM->GetValue( hTable, "hullmax", &rval ) )
vHullMax = rval;
else
bNoParams = true;
g_pScriptVM->ReleaseValue( rval );

if (bNoParams)
{
Expand All @@ -1878,11 +1908,13 @@ static bool Script_TraceHull( HSCRIPT hTable )
{
mask = rval;
}
g_pScriptVM->ReleaseValue( rval );

if (g_pScriptVM->GetValue( hTable, "ignore", &rval ))
{
pIgnoreEnt = ToEnt((HSCRIPT)rval);
}
g_pScriptVM->ReleaseValue( rval );

trace_t tr;
UTIL_TraceHull(vStart, vEnd, vHullMin, vHullMax, mask, pIgnoreEnt, coll, &tr);
Expand Down
19 changes: 18 additions & 1 deletion src/public/vscript/ivscript.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,12 @@ class IScriptVM
{
ScriptVariant_t variant;
GetValue( hScope, pszKey, &variant );
return variant.Get<T>();
T result = variant.Get<T>();
// Get<T>() copies the value out; release the source or GetValue's SV_FREE
// buffer (FIELD_VECTOR/FIELD_CSTRING) leaks. No-op for primitives. The
// const char*/HSCRIPT specializations must NOT release (result aliases it).
ReleaseValue( variant );
return result;
}

template <typename T>
Expand Down Expand Up @@ -1402,6 +1407,18 @@ inline HSCRIPT IScriptVM::Get<HSCRIPT>( HSCRIPT hScope, const char *pszKey )
return variant.Get<HSCRIPT>();
}

// const char* aliases the variant's string buffer, so (unlike the generic template)
// this must NOT ReleaseValue. The returned pointer would dangle; use Get<CUtlString> to own a copy.
template <>
inline const char *IScriptVM::Get<const char *>( HSCRIPT hScope, const char *pszKey )
{
ScriptVariant_t variant;
GetValue( hScope, pszKey, &variant );
if ( variant.GetType() == FIELD_VOID )
return NULL;
return variant.Get<const char *>();
}

#include "tier0/memdbgoff.h"

#endif // IVSCRIPT_H