Fix marker crash on logout and add module lifecycle table

The marker cleanup hook on CleanupWorldAndEntities was never installed —
markers.installHooks() was missing from install() in main.zig. Entities
created via WorldMarker were never cleaned up before the game's atexit
handler iterated the hash table over freed heap memory.

Key changes:
- Add markers.installHooks() call (the actual crash fix)
- Replace manual install/uninstall/shutdown lists with a single modules
  table that drives all three phases — prevents this class of bug
- Gate marker Lua functions, addon, and keybindings behind isActive()
  so they're skipped when another DLL owns the hooks
- Add world_cleanup_hook.detach() to removeHooks() (was missing)
- Migrate from vendored libs/hook to external zhook dependency
- Unify installHooks return types to void across all modules
- Add diagnostic logging to marker cleanup (temporary, for testing)
This commit is contained in:
MarcelineVQ
2026-03-02 07:34:18 -08:00
parent 43313f439d
commit 475070e910
25 changed files with 1793 additions and 1749 deletions
File diff suppressed because it is too large Load Diff
+7
View File
@@ -14,6 +14,13 @@ frame:SetScript("OnEvent", function()
end
end)
-- Per-frame animation driver: queues Hold after Stand finishes on new
-- markers and re-queues it periodically so it never falls back to Stand.
local animFrame = CreateFrame("Frame")
animFrame:SetScript("OnUpdate", function()
ProcessMarkerAnimations()
end)
SLASH_MARKERS1 = "/markers"
SLASH_MARKERS2 = "/mark"
SlashCmdList["MARKERS"] = function(msg)
+71 -8
View File
@@ -28,6 +28,11 @@ const ERROR_ALREADY_EXISTS: u32 = 183;
var g_mutex: ?*anyopaque = null;
var g_is_hook_owner: bool = false;
/// True if this DLL instance owns the markers hooks and Lua API is safe to use.
pub fn isActive() bool {
return g_is_hook_owner;
}
// =============================================================================
// Constants
// =============================================================================
@@ -83,6 +88,7 @@ var despawning: [MAX_DESPAWNING]?DespawningEntity = .{null} ** MAX_DESPAWNING;
// =============================================================================
const fc = std.builtin.CallingConvention{ .x86_fastcall = .{} };
const sc = std.builtin.CallingConvention{ .x86_stdcall = .{} };
const lapi = struct {
fn gettop(L: u32) i32 {
@@ -461,6 +467,58 @@ pub fn luaGetPlayerPosition(L: u32) callconv(.c) u32 {
return 3;
}
// =============================================================================
// World teardown hook
// =============================================================================
var world_cleanup_hook: hook.Detour(fn () callconv(sc) void) = .{};
/// Pre-hook on CleanupWorldAndEntities (0x66fc40).
/// Destroys all our entities via CleanupEntity_ProcessAttachments before the
/// game's teardown runs — the same pattern every native caller uses (e.g.
/// processCinematicExit, DestroyPathObjectIfPresent). This unlinks them from
/// the WDOODADDEF hash table so the atexit handler never touches freed memory.
fn worldCleanupDetour() callconv(sc) void {
con.print("[markers] >>> worldCleanupDetour FIRING <<<\n");
destroyAllEntities();
world_cleanup_hook.callOriginal(.{});
}
/// Destroy all tracked entities (active markers + despawning).
/// Idempotent — safe to call multiple times.
fn destroyAllEntities() void {
var count: u32 = 0;
for (&marker_entities, 0..) |*slot, i| {
if (slot.*) |existing| {
const addr = @intFromPtr(existing);
const flags = hook.readMem(u32, addr + 0x8);
const refcount = hook.readMem(u16, addr + 0x0E);
con.fmt("[markers] destroying marker[{d}] @0x{x} flags=0x{x} refcount={d}\n", .{ i, addr, flags, refcount });
cleanupEntity(existing);
slot.* = null;
count += 1;
}
}
for (&hold_queued) |*h| h.* = false;
for (&marker_created_tick) |*t| t.* = 0;
for (&despawning, 0..) |*slot, i| {
if (slot.*) |d| {
const addr = @intFromPtr(d.entity);
const flags = hook.readMem(u32, addr + 0x8);
const refcount = hook.readMem(u16, addr + 0x0E);
con.fmt("[markers] destroying despawn[{d}] @0x{x} flags=0x{x} refcount={d}\n", .{ i, addr, flags, refcount });
cleanupEntity(d.entity);
slot.* = null;
count += 1;
}
}
if (count > 0) {
con.fmt("[markers] world cleanup: destroyed {d} entities\n", .{count});
}
}
// =============================================================================
// Install hooks
// =============================================================================
@@ -483,18 +541,23 @@ pub fn installHooks() void {
return;
}
g_is_hook_owner = true;
// Hook CleanupWorldAndEntities to destroy our entities before world teardown.
// This fires on map change, logout, AND exit — before heaps are destroyed.
const hook_result = world_cleanup_hook.attach(o.FN_CLEANUP_WORLD_AND_ENTITIES, &worldCleanupDetour);
if (hook_result != .ok) {
con.print("[markers] FAILED to hook CleanupWorldAndEntities!\n");
} else {
con.print("[markers] hooked CleanupWorldAndEntities OK\n");
}
}
pub fn removeHooks() void {
if (g_is_hook_owner) {
// Force-cleanup: no time for animations during shutdown
for (&marker_entities) |*slot| {
if (slot.*) |existing| {
cleanupEntity(existing);
slot.* = null;
}
}
forceCleanupDespawning();
// destroyAllEntities is idempotent — if worldCleanupDetour already ran,
// all slots are null and this is a no-op.
destroyAllEntities();
world_cleanup_hook.detach();
}
if (g_is_hook_owner) {
+11
View File
@@ -33,6 +33,17 @@ pub const MOVEMENT_POS_Z: usize = 0x18;
/// Increments refcount at entity+0x0E.
pub const FN_CREATE_ENTITY_INSTANCE: usize = 0x006707c0;
// =============================================================================
// World teardown (map unload / logout / exit)
// =============================================================================
/// CleanupWorldAndEntities — void(), no params, __stdcall.
/// Top-level world teardown: calls CleanupEntityList_ProcessAll, then
/// CleanupWorldAndReleaseResources (which iterates heaps and force-frees).
/// Called from InitializeWorldScene (map change) and ShutdownClientSystems (exit).
/// Hook this to destroy custom entities BEFORE the game's teardown begins.
pub const FN_CLEANUP_WORLD_AND_ENTITIES: usize = 0x0066fc40;
// =============================================================================
// World object lifecycle
// =============================================================================