From 3f2e56be67f580a181bc18bb26897dd1595b35a0 Mon Sep 17 00:00:00 2001 From: JonKazama-Hellion Date: Sun, 3 May 2026 22:02:46 +0200 Subject: [PATCH] fix: tighten resource-leak and null-deref hot spots Three pre-existing upstream defects flagged by CodeRabbit, fixed in the v1.0.0 standalone cut where we own the codebase: - Ipc/ExtraChat.cs Dispose now unsubscribes all three IPC subscriptions (OverrideChannelGate, ChannelCommandColoursGate, ChannelNamesGate) instead of only the first; previously the latter two leaked their subscriptions on every plugin reload - GameFunctions/Types/TellTarget.cs FromTarget guards against a zero IPlayerCharacter.Address before dereferencing the unsafe Character* cast; previously a missing/destroyed target object would crash the game on /tell construction - GameFunctions/GameFunctions.cs ResolveTextCommandPlaceholderDetour null-checks the Hook reference before calling .Original instead of using the null-forgiving operator; defensive guard for teardown races --- HellionChat/GameFunctions/GameFunctions.cs | 8 +++++++- HellionChat/GameFunctions/Types/TellTarget.cs | 3 +++ HellionChat/Ipc/ExtraChat.cs | 2 ++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/HellionChat/GameFunctions/GameFunctions.cs b/HellionChat/GameFunctions/GameFunctions.cs index 5e6de44..31b8970 100755 --- a/HellionChat/GameFunctions/GameFunctions.cs +++ b/HellionChat/GameFunctions/GameFunctions.cs @@ -249,9 +249,15 @@ internal unsafe class GameFunctions : IDisposable private nint ResolveTextCommandPlaceholderDetour(nint a1, byte* placeholderText, byte a3, byte a4) { + // The detour is only invoked through the hook, so the hook should + // never be null here, but the nullable field declaration forces us + // to handle the theoretical race during teardown. + if (ResolveTextCommandPlaceholderHook is null) + return nint.Zero; + var placeholder = MemoryHelper.ReadStringNullTerminated((nint) placeholderText); if (ReplacementName == null || placeholder != Placeholder) - return ResolveTextCommandPlaceholderHook!.Original(a1, placeholderText, a3, a4); + return ResolveTextCommandPlaceholderHook.Original(a1, placeholderText, a3, a4); MemoryHelper.WriteString(PlaceholderNamePtr, ReplacementName); ReplacementName = null; diff --git a/HellionChat/GameFunctions/Types/TellTarget.cs b/HellionChat/GameFunctions/Types/TellTarget.cs index e1c9469..b6151c2 100755 --- a/HellionChat/GameFunctions/Types/TellTarget.cs +++ b/HellionChat/GameFunctions/Types/TellTarget.cs @@ -30,6 +30,9 @@ public class TellTarget public unsafe void FromTarget(IPlayerCharacter target) { + if (target.Address == nint.Zero) + return; + Name = target.Name.TextValue; World = target.HomeWorld.RowId; ContentId = ((Character*)target.Address)->ContentId; diff --git a/HellionChat/Ipc/ExtraChat.cs b/HellionChat/Ipc/ExtraChat.cs index 178cb96..b04521e 100644 --- a/HellionChat/Ipc/ExtraChat.cs +++ b/HellionChat/Ipc/ExtraChat.cs @@ -49,6 +49,8 @@ public sealed class ExtraChat : IDisposable public void Dispose() { OverrideChannelGate.Unsubscribe(OnOverrideChannel); + ChannelCommandColoursGate.Unsubscribe(OnChannelCommandColours); + ChannelNamesGate.Unsubscribe(OnChannelNames); } private void OnOverrideChannel(OverrideInfo info)