Skip to content

Commit

Permalink
Fix memory leaks and heap size (#1004)
Browse files Browse the repository at this point in the history
Fix memory leaks and heap sizes
Fix missing HTTP event
  • Loading branch information
nickdnk authored Mar 20, 2023
1 parent ae973cf commit 10763a8
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 25 deletions.
19 changes: 14 additions & 5 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,18 @@ Whenever you update your Get5 plugin, remember to **always** update the `transla
Please see the [installation instructions](https://splewis.github.io/get5/latest/installation/#installation) for
details.

# 0.13.1

#### 2023-03-20

## New Features / Changes 🎉

1. Fix critical memory leaks and heap size issues.
2. Fix missing HTTP event for `Get5_OnTeamReadyStatusChanged` if a team goes from ready to unready.

# 0.13.0

#### 2022-02-18
#### 2023-02-18

### Breaking Changes 🛠

Expand Down Expand Up @@ -95,7 +104,7 @@ details.
# 0.12.1
#### 2022-01-16
#### 2023-01-16
## What's Changed

Expand Down Expand Up @@ -142,10 +151,10 @@ to [the documentation](https://splewis.github.io/get5/latest/translations/) for
separately from [`get5_time_to_start`](https://splewis.github.io/get5/latest/configuration/#get5_time_to_start). If
you don't set this variable, players will have infinite time to ready for veto.
4. `maplist` is now required in match configurations. There is now no "default map list" in Get5.
6. The `filename` property of the `demo_finished` and `demo_upload_ended` events now includes the folder, i.e. the full
5. The `filename` property of the `demo_finished` and `demo_upload_ended` events now includes the folder, i.e. the full
path to the file, if [`get5_demo_path`](https://splewis.github.io/get5/latest/configuration/#get5_demo_path) is set.
7. `Get5_OnPreLoadMatchConfig()` no longer fires when loading a backup file.
8. If you use `fromfile`, make sure to always have JSON files end with `.json`, or Get5 will assume they are KeyValues,
6. `Get5_OnPreLoadMatchConfig()` no longer fires when loading a backup file.
7. If you use `fromfile`, make sure to always have JSON files end with `.json`, or Get5 will assume they are KeyValues,
regardless of the format of the match config.
### New Features 🎉
Expand Down
26 changes: 14 additions & 12 deletions scripting/get5.sp
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,12 @@
#pragma semicolon 1
#pragma newdecls required
/**
* Increases stack space to 32000 cells (or 128KB, a cell is 4 bytes)
* This is to prevent "Not enough space on the heap" error when dumping match stats
* Default heap size is 4KB
* Increases stack space to 131072 cells (or 512KB; a cell is 4 bytes).
* This is to prevent "Not enough space on the heap" error when dumping match stats while also
* allowing a 64KB static buffer for JSON encode output for HTTP events.
* Default heap size is 1024 cells (4KB).
*/
#pragma dynamic 32000
#pragma dynamic 131072

/** ConVar handles **/
ConVar g_AllowPauseCancellationCvar;
Expand Down Expand Up @@ -1064,6 +1065,10 @@ static Action Command_EndMatch(int client, int args) {
return Plugin_Handled;
}

// Clear ongoing grenade containers, if any. Requires live state or the events will be lost,
// so we do this before changing the game state below.
Stats_ResetGrenadeContainers();

Get5Team winningTeam = Get5Team_None; // defaults to tie
if (args >= 1) {
char forcedWinningTeam[8];
Expand Down Expand Up @@ -1723,15 +1728,12 @@ void ResetMatchCvarsAndHostnameAndKickPlayers(bool kickPlayers) {

static Action Event_RoundPreStart(Event event, const char[] name, bool dontBroadcast) {
LogDebug("Event_RoundPreStart");
Stats_ResetGrenadeContainers(); // Always do this. If not in live, the events are simply
// discarded and this just prevents memory leaks.
if (g_GameState == Get5State_None) {
return;
}

if (g_GameState == Get5State_Live) {
// End lingering grenade trackers from previous round.
Stats_ResetGrenadeContainers();
}

if (g_PendingSideSwap) {
SwapSides();
}
Expand Down Expand Up @@ -2218,9 +2220,9 @@ void EventLogger_LogAndDeleteEvent(Get5Event event) {

// We could use json_encode_size here from sm-json, but since we fire events *all the time*
// and the function to calculate the buffer size is a lot of code, we just statically allocate
// a 16k buffer here and reuse that.
static char buffer[16384];
event.Encode(buffer, 16384, options);
// a 64K buffer here and reuse that.
static char buffer[65536];
event.Encode(buffer, 65536, options);

char logPath[PLATFORM_MAX_PATH];
if (FormatCvarString(g_EventLogFormatCvar, logPath, sizeof(logPath))) {
Expand Down
3 changes: 2 additions & 1 deletion scripting/get5/debug.sp
Original file line number Diff line number Diff line change
Expand Up @@ -236,11 +236,12 @@ static void AddLogLines(File f, const char[] pattern, int maxLines) {
}

delete logFile;
delete lines;
} else {
f.WriteLine("Couldn't read log file %s", filename);
}
}

delete logFilenames;
delete dir;
}

Expand Down
2 changes: 2 additions & 0 deletions scripting/get5/readysystem.sp
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,8 @@ void UnreadyTeam(Get5Team team) {
Call_PushCell(readyEvent);
Call_Finish();

EventLogger_LogAndDeleteEvent(readyEvent);

SetMatchTeamCvars();
Get5_MessageToAll("%t", "TeamIsNoLongerReady", g_FormattedTeamNames[team]);
}
Expand Down
16 changes: 11 additions & 5 deletions scripting/get5/stats.sp
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,8 @@ static void EndMolotovEvent(const char[] molotovKey) {
Get5MolotovDetonatedEvent molotovObject;
if (g_MolotovContainer.GetValue(molotovKey, molotovObject)) {
if (g_GameState != Get5State_Live || IsDoingRestoreOrMapChange()) {
delete molotovObject;
json_cleanup_and_delete(molotovObject);
LogDebug("Deleting molotov event with key %s as match is no longer live.", molotovKey);
} else {
molotovObject.EndTime = GetRoundTime();
LogDebug("Calling Get5_OnMolotovDetonated()");
Expand All @@ -380,7 +381,8 @@ static void EndHEEvent(const char[] grenadeKey) {
Get5HEDetonatedEvent heObject;
if (g_HEGrenadeContainer.GetValue(grenadeKey, heObject)) {
if (g_GameState != Get5State_Live || IsDoingRestoreOrMapChange()) {
delete heObject;
json_cleanup_and_delete(heObject);
LogDebug("Deleting HE event with key %s as match is no longer live.", grenadeKey);
} else {
LogDebug("Calling Get5_OnHEGrenadeDetonated()");
Call_StartForward(g_OnHEGrenadeDetonated);
Expand All @@ -396,7 +398,8 @@ static void EndFlashbangEvent(const char[] flashKey) {
Get5FlashbangDetonatedEvent flashEvent;
if (g_FlashbangContainer.GetValue(flashKey, flashEvent)) {
if (g_GameState != Get5State_Live || IsDoingRestoreOrMapChange()) {
delete flashEvent;
json_cleanup_and_delete(flashEvent);
LogDebug("Deleting flash event with key %s as match is no longer live.", flashKey);
} else {
LogDebug("Calling Get5_OnFlashbangDetonated()");
Call_StartForward(g_OnFlashbangDetonated);
Expand Down Expand Up @@ -625,7 +628,6 @@ static Action Stats_PlayerDeathEvent(Event event, const char[] name, bool dontBr
if (!IsValidClient(victim)) {
return Plugin_Continue; // Not sure how this would happen, but it's not something we care about.
}
Get5Player victimPlayer = GetPlayerObject(victim);

int attacker = GetClientOfUserId(event.GetInt("attacker"));
Get5Player attackerPlayer = IsValidClient(attacker) ? GetPlayerObject(attacker) : null;
Expand All @@ -634,9 +636,12 @@ static Action Stats_PlayerDeathEvent(Event event, const char[] name, bool dontBr
// HandleReadyCommand checks for game state, so we don't need to do that here as well.
HandleReadyCommand(attacker, true);
}
json_cleanup_and_delete(attackerPlayer);
return Plugin_Continue;
}

Get5Player victimPlayer = GetPlayerObject(victim);

Get5Side victimSide = victimPlayer.Side;
Get5Side attackerSide = attackerPlayer != null ? attackerPlayer.Side : Get5Side_None;

Expand Down Expand Up @@ -1060,10 +1065,11 @@ static bool DumpToJSONFile(const char[] path) {
File stats_file = OpenFile(path, "w");
if (stats_file == null) {
LogError("Failed to open stats file");
json_cleanup_and_delete(stats);
return false;
}

// Mark the JSON buffer static to avoid running into limited haep/stack space, see
// Mark the JSON buffer static to avoid running into limited heap/stack space, see
// https://forums.alliedmods.net/showpost.php?p=2620835&postcount=6
static char jsonBuffer[65536]; // 64 KiB
stats.Encode(jsonBuffer, sizeof(jsonBuffer));
Expand Down
2 changes: 1 addition & 1 deletion scripting/get5/version.sp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#tryinclude "manual_version.sp"
#if !defined PLUGIN_VERSION
#define PLUGIN_VERSION "0.13.0-dev"
#define PLUGIN_VERSION "0.13.1-dev"
#endif

// This MUST be the latest version in x.y.z semver format followed by -dev.
Expand Down

0 comments on commit 10763a8

Please sign in to comment.