From dfcee625db09c9a03d630a36e8c9d04d2104afb3 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Thu, 28 Jan 2016 11:56:47 +0100 Subject: [PATCH] - fixed: The ACS String garbage collector only looked at the active script's stack but never considered a recursive call from another script. Fixing this required adding an external list of active stack objects that the garbage collector can access. A nice side effect: It's no longer necessary to pass around the stack info to various functions that might end up triggering a garbage collection. --- src/p_acs.cpp | 150 +++++++++++++++++++++++++++++++++----------------- src/p_acs.h | 12 ++-- 2 files changed, 105 insertions(+), 57 deletions(-) diff --git a/src/p_acs.cpp b/src/p_acs.cpp index 3aab261f4..da76408ed 100644 --- a/src/p_acs.cpp +++ b/src/p_acs.cpp @@ -217,6 +217,52 @@ FWorldGlobalArray ACS_WorldArrays[NUM_WORLDVARS]; SDWORD ACS_GlobalVars[NUM_GLOBALVARS]; FWorldGlobalArray ACS_GlobalArrays[NUM_GLOBALVARS]; +//---------------------------------------------------------------------------- +// +// ACS stack management +// +// This is needed so that the garbage collector has access to all active +// script stacks +// +//---------------------------------------------------------------------------- + +struct FACSStack +{ + SDWORD buffer[STACK_SIZE]; + int sp; + FACSStack *next; + FACSStack *prev; + static FACSStack *head; + + FACSStack(); + ~FACSStack(); +}; + +FACSStack *FACSStack::head; + +FACSStack::FACSStack() +{ + sp = 0; + next = head; + prev = NULL; + head = this; + + Printf("Adding stack %08d\n", this); +} + +FACSStack::~FACSStack() +{ + if (next != NULL) next->prev = prev; + if (prev == NULL) + { + head = next; + } + else + { + prev->next = next; + } + Printf("Removing stack %08d\n", this); +} //---------------------------------------------------------------------------- // @@ -290,7 +336,7 @@ void ACSStringPool::Clear() // //============================================================================ -int ACSStringPool::AddString(const char *str, const SDWORD *stack, int stackdepth) +int ACSStringPool::AddString(const char *str) { size_t len = strlen(str); unsigned int h = SuperFastHash(str, len); @@ -301,10 +347,10 @@ int ACSStringPool::AddString(const char *str, const SDWORD *stack, int stackdept return i | STRPOOL_LIBRARYID_OR; } FString fstr(str); - return InsertString(fstr, h, bucketnum, stack, stackdepth); + return InsertString(fstr, h, bucketnum); } -int ACSStringPool::AddString(FString &str, const SDWORD *stack, int stackdepth) +int ACSStringPool::AddString(FString &str) { unsigned int h = SuperFastHash(str.GetChars(), str.Len()); unsigned int bucketnum = h % NUM_BUCKETS; @@ -313,7 +359,7 @@ int ACSStringPool::AddString(FString &str, const SDWORD *stack, int stackdepth) { return i | STRPOOL_LIBRARYID_OR; } - return InsertString(str, h, bucketnum, stack, stackdepth); + return InsertString(str, h, bucketnum); } //============================================================================ @@ -583,12 +629,12 @@ int ACSStringPool::FindString(const char *str, size_t len, unsigned int h, unsig // //============================================================================ -int ACSStringPool::InsertString(FString &str, unsigned int h, unsigned int bucketnum, const SDWORD *stack, int stackdepth) +int ACSStringPool::InsertString(FString &str, unsigned int h, unsigned int bucketnum) { unsigned int index = FirstFreeEntry; if (index >= MIN_GC_SIZE && index == Pool.Max()) { // We will need to grow the array. Try a garbage collection first. - P_CollectACSGlobalStrings(stack, stackdepth); + P_CollectACSGlobalStrings(); index = FirstFreeEntry; } if (FirstFreeEntry >= STRPOOL_LIBRARYID_OR) @@ -772,11 +818,11 @@ void P_MarkGlobalVarStrings() // //============================================================================ -void P_CollectACSGlobalStrings(const SDWORD *stack, int stackdepth) +void P_CollectACSGlobalStrings() { - if (stack != NULL && stackdepth != 0) + for (FACSStack *stack = FACSStack::head; stack != NULL; stack = stack->next) { - GlobalACSStrings.MarkStringArray(stack, stackdepth); + GlobalACSStrings.MarkStringArray(stack->buffer, stack->sp); } FBehavior::StaticMarkLevelVarStrings(); P_MarkWorldVarStrings(); @@ -787,7 +833,7 @@ void P_CollectACSGlobalStrings(const SDWORD *stack, int stackdepth) #ifdef _DEBUG CCMD(acsgc) { - P_CollectACSGlobalStrings(NULL, 0); + P_CollectACSGlobalStrings(); } CCMD(globstr) { @@ -1995,7 +2041,7 @@ bool FBehavior::Init(int lumpnum, FileReader * fr, int len) const char *str = LookupString(MapVarStore[LittleLong(chunk[i+2])]); if (str != NULL) { - MapVarStore[LittleLong(chunk[i+2])] = GlobalACSStrings.AddString(str, NULL, 0); + MapVarStore[LittleLong(chunk[i+2])] = GlobalACSStrings.AddString(str); } } } @@ -2015,7 +2061,7 @@ bool FBehavior::Init(int lumpnum, FileReader * fr, int len) const char *str = LookupString(*elems); if (str != NULL) { - *elems = GlobalACSStrings.AddString(str, NULL, 0); + *elems = GlobalACSStrings.AddString(str); } } } @@ -2049,7 +2095,7 @@ bool FBehavior::Init(int lumpnum, FileReader * fr, int len) const char *str = LookupString(*elems); if (str != NULL) { - *elems = GlobalACSStrings.AddString(str, NULL, 0); + *elems = GlobalACSStrings.AddString(str); } } } @@ -3949,7 +3995,7 @@ void DLevelScript::DoSetActorProperty (AActor *actor, int property, int value) } } -int DLevelScript::GetActorProperty (int tid, int property, const SDWORD *stack, int stackdepth) +int DLevelScript::GetActorProperty (int tid, int property) { AActor *actor = SingleActorFromTID (tid, activator); @@ -4034,13 +4080,13 @@ int DLevelScript::GetActorProperty (int tid, int property, const SDWORD *stack, return 0; } - case APROP_SeeSound: return GlobalACSStrings.AddString(actor->SeeSound, stack, stackdepth); - case APROP_AttackSound: return GlobalACSStrings.AddString(actor->AttackSound, stack, stackdepth); - case APROP_PainSound: return GlobalACSStrings.AddString(actor->PainSound, stack, stackdepth); - case APROP_DeathSound: return GlobalACSStrings.AddString(actor->DeathSound, stack, stackdepth); - case APROP_ActiveSound: return GlobalACSStrings.AddString(actor->ActiveSound, stack, stackdepth); - case APROP_Species: return GlobalACSStrings.AddString(actor->GetSpecies(), stack, stackdepth); - case APROP_NameTag: return GlobalACSStrings.AddString(actor->GetTag(), stack, stackdepth); + case APROP_SeeSound: return GlobalACSStrings.AddString(actor->SeeSound); + case APROP_AttackSound: return GlobalACSStrings.AddString(actor->AttackSound); + case APROP_PainSound: return GlobalACSStrings.AddString(actor->PainSound); + case APROP_DeathSound: return GlobalACSStrings.AddString(actor->DeathSound); + case APROP_ActiveSound: return GlobalACSStrings.AddString(actor->ActiveSound); + case APROP_Species: return GlobalACSStrings.AddString(actor->GetSpecies()); + case APROP_NameTag: return GlobalACSStrings.AddString(actor->GetTag()); case APROP_StencilColor:return actor->fillcolor; case APROP_Friction: return actor->Friction; @@ -4090,7 +4136,7 @@ int DLevelScript::CheckActorProperty (int tid, int property, int value) case APROP_ViewHeight: case APROP_AttackZOffset: case APROP_StencilColor: - return (GetActorProperty(tid, property, NULL, 0) == value); + return (GetActorProperty(tid, property) == value); // Boolean values need to compare to a binary version of value case APROP_Ambush: @@ -4102,7 +4148,7 @@ int DLevelScript::CheckActorProperty (int tid, int property, int value) case APROP_Notarget: case APROP_Notrigger: case APROP_Dormant: - return (GetActorProperty(tid, property, NULL, 0) == (!!value)); + return (GetActorProperty(tid, property) == (!!value)); // Strings are covered by GetActorProperty, but they're fairly // heavy-duty, so make the check here. @@ -4614,14 +4660,14 @@ static void DoSetCVar(FBaseCVar *cvar, int value, bool is_string, bool force=fal } // Converts floating- to fixed-point as required. -static int DoGetCVar(FBaseCVar *cvar, bool is_string, const SDWORD *stack, int stackdepth) +static int DoGetCVar(FBaseCVar *cvar, bool is_string) { UCVarValue val; if (is_string) { val = cvar->GetGenericRep(CVAR_String); - return GlobalACSStrings.AddString(val.String, stack, stackdepth); + return GlobalACSStrings.AddString(val.String); } else if (cvar->GetRealType() == CVAR_Float) { @@ -4635,7 +4681,7 @@ static int DoGetCVar(FBaseCVar *cvar, bool is_string, const SDWORD *stack, int s } } -static int GetUserCVar(int playernum, const char *cvarname, bool is_string, const SDWORD *stack, int stackdepth) +static int GetUserCVar(int playernum, const char *cvarname, bool is_string) { if ((unsigned)playernum >= MAXPLAYERS || !playeringame[playernum]) { @@ -4647,10 +4693,10 @@ static int GetUserCVar(int playernum, const char *cvarname, bool is_string, cons { return 0; } - return DoGetCVar(cvar, is_string, stack, stackdepth); + return DoGetCVar(cvar, is_string); } -static int GetCVar(AActor *activator, const char *cvarname, bool is_string, const SDWORD *stack, int stackdepth) +static int GetCVar(AActor *activator, const char *cvarname, bool is_string) { FBaseCVar *cvar = FindCVar(cvarname, NULL); // Either the cvar doesn't exist, or it's for a mod that isn't loaded, so return 0. @@ -4667,9 +4713,9 @@ static int GetCVar(AActor *activator, const char *cvarname, bool is_string, cons { return 0; } - return GetUserCVar(int(activator->player - players), cvarname, is_string, stack, stackdepth); + return GetUserCVar(int(activator->player - players), cvarname, is_string); } - return DoGetCVar(cvar, is_string, stack, stackdepth); + return DoGetCVar(cvar, is_string); } } @@ -4860,7 +4906,7 @@ static int SwapActorTeleFog(AActor *activator, int tid) -int DLevelScript::CallFunction(int argCount, int funcIndex, SDWORD *args, const SDWORD *stack, int stackdepth) +int DLevelScript::CallFunction(int argCount, int funcIndex, SDWORD *args) { AActor *actor; switch(funcIndex) @@ -5031,7 +5077,7 @@ int DLevelScript::CallFunction(int argCount, int funcIndex, SDWORD *args, const switch(args[0]) { case ARMORINFO_CLASSNAME: - return GlobalACSStrings.AddString(equippedarmor->ArmorType.GetChars(), stack, stackdepth); + return GlobalACSStrings.AddString(equippedarmor->ArmorType.GetChars()); case ARMORINFO_SAVEAMOUNT: return equippedarmor->MaxAmount; @@ -5052,7 +5098,7 @@ int DLevelScript::CallFunction(int argCount, int funcIndex, SDWORD *args, const return 0; } } - return args[0] == ARMORINFO_CLASSNAME ? GlobalACSStrings.AddString("None", stack, stackdepth) : 0; + return args[0] == ARMORINFO_CLASSNAME ? GlobalACSStrings.AddString("None") : 0; } case ACSF_SpawnSpotForced: @@ -5171,7 +5217,7 @@ int DLevelScript::CallFunction(int argCount, int funcIndex, SDWORD *args, const case ACSF_GetActorClass: { AActor *a = SingleActorFromTID(args[0], activator); - return GlobalACSStrings.AddString(a == NULL ? "None" : a->GetClass()->TypeName.GetChars(), stack, stackdepth); + return GlobalACSStrings.AddString(a == NULL ? "None" : a->GetClass()->TypeName.GetChars()); } case ACSF_SoundSequenceOnActor: @@ -5349,7 +5395,7 @@ int DLevelScript::CallFunction(int argCount, int funcIndex, SDWORD *args, const case ACSF_GetCVarString: if (argCount == 1) { - return GetCVar(activator, FBehavior::StaticLookupString(args[0]), true, stack, stackdepth); + return GetCVar(activator, FBehavior::StaticLookupString(args[0]), true); } break; @@ -5370,14 +5416,14 @@ int DLevelScript::CallFunction(int argCount, int funcIndex, SDWORD *args, const case ACSF_GetUserCVar: if (argCount == 2) { - return GetUserCVar(args[0], FBehavior::StaticLookupString(args[1]), false, stack, stackdepth); + return GetUserCVar(args[0], FBehavior::StaticLookupString(args[1]), false); } break; case ACSF_GetUserCVarString: if (argCount == 2) { - return GetUserCVar(args[0], FBehavior::StaticLookupString(args[1]), true, stack, stackdepth); + return GetUserCVar(args[0], FBehavior::StaticLookupString(args[1]), true); } break; @@ -5564,7 +5610,7 @@ doplaysound: if (funcIndex == ACSF_PlayActorSound) const char *oldstr = FBehavior::StaticLookupString(args[0]); if (oldstr == NULL || *oldstr == '\0') { - return GlobalACSStrings.AddString("", stack, stackdepth); + return GlobalACSStrings.AddString(""); } size_t oldlen = strlen(oldstr); size_t newlen = args[1]; @@ -5574,7 +5620,7 @@ doplaysound: if (funcIndex == ACSF_PlayActorSound) newlen = oldlen; } FString newstr(funcIndex == ACSF_StrLeft ? oldstr : oldstr + oldlen - newlen, newlen); - return GlobalACSStrings.AddString(newstr, stack, stackdepth); + return GlobalACSStrings.AddString(newstr); } break; @@ -5584,7 +5630,7 @@ doplaysound: if (funcIndex == ACSF_PlayActorSound) const char *oldstr = FBehavior::StaticLookupString(args[0]); if (oldstr == NULL || *oldstr == '\0') { - return GlobalACSStrings.AddString("", stack, stackdepth); + return GlobalACSStrings.AddString(""); } size_t oldlen = strlen(oldstr); size_t pos = args[1]; @@ -5592,13 +5638,13 @@ doplaysound: if (funcIndex == ACSF_PlayActorSound) if (pos >= oldlen) { - return GlobalACSStrings.AddString("", stack, stackdepth); + return GlobalACSStrings.AddString(""); } if (pos + newlen > oldlen || pos + newlen < pos) { newlen = oldlen - pos; } - return GlobalACSStrings.AddString(FString(oldstr + pos, newlen), stack, stackdepth); + return GlobalACSStrings.AddString(FString(oldstr + pos, newlen)); } break; @@ -5606,11 +5652,11 @@ doplaysound: if (funcIndex == ACSF_PlayActorSound) if (activator == NULL || activator->player == NULL || // Non-players do not have weapons activator->player->ReadyWeapon == NULL) { - return GlobalACSStrings.AddString("None", stack, stackdepth); + return GlobalACSStrings.AddString("None"); } else { - return GlobalACSStrings.AddString(activator->player->ReadyWeapon->GetClass()->TypeName.GetChars(), stack, stackdepth); + return GlobalACSStrings.AddString(activator->player->ReadyWeapon->GetClass()->TypeName.GetChars()); } case ACSF_SpawnDecal: @@ -6157,8 +6203,10 @@ int DLevelScript::RunScript () break; } - SDWORD Stack[STACK_SIZE]; - int sp = 0; + FACSStack stackobj; + SDWORD *Stack = stackobj.buffer; + int &sp = stackobj.sp; + int *pc = this->pc; ACSFormat fmt = activeBehavior->GetFormat(); FBehavior* const savedActiveBehavior = activeBehavior; @@ -6211,7 +6259,7 @@ int DLevelScript::RunScript () case PCD_TAGSTRING: //Stack[sp-1] |= activeBehavior->GetLibraryID(); - Stack[sp-1] = GlobalACSStrings.AddString(activeBehavior->LookupString(Stack[sp-1]), Stack, sp); + Stack[sp-1] = GlobalACSStrings.AddString(activeBehavior->LookupString(Stack[sp-1])); break; case PCD_PUSHNUMBER: @@ -6409,7 +6457,7 @@ int DLevelScript::RunScript () int argCount = NEXTBYTE; int funcIndex = NEXTSHORT; - int retval = CallFunction(argCount, funcIndex, &STACK(argCount), Stack, sp); + int retval = CallFunction(argCount, funcIndex, &STACK(argCount)); sp -= argCount-1; STACK(1) = retval; } @@ -8933,7 +8981,7 @@ scriptwait: break; case PCD_GETACTORPROPERTY: - STACK(2) = GetActorProperty (STACK(2), STACK(1), Stack, sp); + STACK(2) = GetActorProperty (STACK(2), STACK(1)); sp -= 1; break; @@ -9030,7 +9078,7 @@ scriptwait: break; case PCD_GETCVAR: - STACK(1) = GetCVar(activator, FBehavior::StaticLookupString(STACK(1)), false, Stack, sp); + STACK(1) = GetCVar(activator, FBehavior::StaticLookupString(STACK(1)), false); break; case PCD_SETHUDSIZE: @@ -9384,7 +9432,7 @@ scriptwait: case PCD_SAVESTRING: // Saves the string { - const int str = GlobalACSStrings.AddString(work, Stack, sp); + const int str = GlobalACSStrings.AddString(work); PushToStack(str); STRINGBUILDER_FINISH(work); } diff --git a/src/p_acs.h b/src/p_acs.h index 3188e46aa..858364e2f 100644 --- a/src/p_acs.h +++ b/src/p_acs.h @@ -80,8 +80,8 @@ class ACSStringPool { public: ACSStringPool(); - int AddString(const char *str, const SDWORD *stack, int stackdepth); - int AddString(FString &str, const SDWORD *stack, int stackdepth); + int AddString(const char *str); + int AddString(FString &str); const char *GetString(int strnum); void LockString(int strnum); void UnlockString(int strnum); @@ -99,7 +99,7 @@ public: private: int FindString(const char *str, size_t len, unsigned int h, unsigned int bucketnum); - int InsertString(FString &str, unsigned int h, unsigned int bucketnum, const SDWORD *stack, int stackdepth); + int InsertString(FString &str, unsigned int h, unsigned int bucketnum); void FindFirstFreeEntry(unsigned int base); enum { NUM_BUCKETS = 251 }; @@ -119,7 +119,7 @@ private: }; extern ACSStringPool GlobalACSStrings; -void P_CollectACSGlobalStrings(const SDWORD *stack, int stackdepth); +void P_CollectACSGlobalStrings(); void P_ReadACSVars(PNGHandle *); void P_WriteACSVars(FILE*); void P_ClearACSVars(bool); @@ -910,7 +910,7 @@ protected: int DoSpawnSpot (int type, int spot, int tid, int angle, bool forced); int DoSpawnSpotFacing (int type, int spot, int tid, bool forced); int DoClassifyActor (int tid); - int CallFunction(int argCount, int funcIndex, SDWORD *args, const SDWORD *stack, int stackdepth); + int CallFunction(int argCount, int funcIndex, SDWORD *args); void DoFadeTo (int r, int g, int b, int a, fixed_t time); void DoFadeRange (int r1, int g1, int b1, int a1, @@ -918,7 +918,7 @@ protected: void DoSetFont (int fontnum); void SetActorProperty (int tid, int property, int value); void DoSetActorProperty (AActor *actor, int property, int value); - int GetActorProperty (int tid, int property, const SDWORD *stack, int stackdepth); + int GetActorProperty (int tid, int property); int CheckActorProperty (int tid, int property, int value); int GetPlayerInput (int playernum, int inputnum);