From 9d00c99cd0c636aa5a6c25677d334d455cd9195f Mon Sep 17 00:00:00 2001 From: Freeyorp Date: Mon, 29 Apr 2024 10:15:51 +0000 Subject: npc_destroy: Defer NPC destruction via timer 055-1 _nodes.txt will call `destroy;` from within OnInit, that is during an iteration of the global `ev_db`. Previously, concurrent modification invalidated this iteration, resulting in a crash. This still nullifes `oid`, dequeues all timers, and effectively calls `builtin_end`. `npc_data::deletion_pending` is extended to include a third state. In addition to no deletion happening, and indicating when `npc_free` is currently on the stack, it now also tracks whether the NPC is about to be deleted by a timer. This means that an NPC which is about to be deleted is still blocked from triggering new events, much like an NPC actively being deleted would. Starting or continuing an NPC dialog with an NPC that does not, or is about to no longer exist, is now also blocked. --- src/map/map.hpp | 6 +++++- src/map/npc-parse.cpp | 8 ++++---- src/map/npc.cpp | 19 ++++++++++++++----- src/map/script-fun.cpp | 15 +++++++++++++-- 4 files changed, 36 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/map/map.hpp b/src/map/map.hpp index 7cf43d5..56d07a5 100644 --- a/src/map/map.hpp +++ b/src/map/map.hpp @@ -360,7 +360,11 @@ struct npc_data : block_list Opt0 option; short flag; - bool deletion_pending; + enum { + NOT_DELETING = 0, + DELETION_QUEUED = 1, + DELETION_ACTIVE = 2 + } deletion_pending; Array eventtimer; diff --git a/src/map/npc-parse.cpp b/src/map/npc-parse.cpp index 47b851c..df1a09a 100644 --- a/src/map/npc-parse.cpp +++ b/src/map/npc-parse.cpp @@ -164,7 +164,7 @@ bool npc_load_warp(ast::npc::Warp& warp) nd->warp.xs = xs; nd->warp.ys = ys; - nd->deletion_pending = false; + nd->deletion_pending = npc_data::NOT_DELETING; npc_warp++; nd->bl_type = BL::NPC; @@ -228,7 +228,7 @@ bool npc_load_shop(ast::npc::Shop& shop) nd->opt2 = Opt2::ZERO; nd->opt3 = Opt3::ZERO; - nd->deletion_pending = false; + nd->deletion_pending = npc_data::NOT_DELETING; npc_shop++; nd->bl_type = BL::NPC; @@ -458,7 +458,7 @@ bool npc_load_script_none(ast::script::ScriptBody& body, ast::npc::ScriptNone& s nd->opt2 = Opt2::ZERO; nd->opt3 = Opt3::ZERO; - nd->deletion_pending = false; + nd->deletion_pending = npc_data::NOT_DELETING; npc_script++; nd->bl_type = BL::NPC; @@ -568,7 +568,7 @@ bool npc_load_script_map(ast::script::ScriptBody& body, ast::npc::ScriptMap& scr nd->opt2 = Opt2::ZERO; nd->opt3 = Opt3::ZERO; - nd->deletion_pending = false; + nd->deletion_pending = npc_data::NOT_DELETING; npc_script++; nd->bl_type = BL::NPC; diff --git a/src/map/npc.cpp b/src/map/npc.cpp index e8d6d4b..8a6bead 100644 --- a/src/map/npc.cpp +++ b/src/map/npc.cpp @@ -357,7 +357,7 @@ void npc_eventtimer(TimerData *, tick_t, BlockId, NpcEvent data) data); return; }); - if ((nd = ev->nd) == nullptr || nd->deletion_pending == true) + if ((nd = ev->nd) == nullptr || nd->deletion_pending != npc_data::NOT_DELETING) { if (battle_config.error_log) PRINTF("npc_event: event not found [%s]\n"_fmt, @@ -591,7 +591,7 @@ int npc_event(dumb_ptr sd, NpcEvent eventname, ev.pos = ev2->pos; } - if ((nd = ev.nd) == nullptr || nd->deletion_pending == true) + if ((nd = ev.nd) == nullptr || nd->deletion_pending != npc_data::NOT_DELETING) { if (!mob_kill && battle_config.error_log) PRINTF("npc_event: event not found [%s]\n"_fmt, @@ -774,6 +774,14 @@ int npc_click(dumb_ptr sd, BlockId id) nd = map_id_is_npc(id); + // If someone clicked on an NPC that is about to no longer exist, then + // release them + if (nd->deletion_pending != npc_data::NOT_DELETING) + { + clif_scriptclose(sd, id); + return 1; + } + if (nd->flag & 1) // 無効化されている return 1; @@ -818,7 +826,8 @@ int npc_scriptcont(dumb_ptr sd, BlockId id) nd = map_id_is_npc(id); - if (!nd /* NPC was disposed? */) + // If the NPC is about to be deleted, release the PC + if (nd->deletion_pending != npc_data::NOT_DELETING) { clif_scriptclose(sd, id); npc_event_dequeue(sd); @@ -1091,10 +1100,10 @@ void npc_propagate_update(dumb_ptr nd) void npc_free(dumb_ptr nd) { - if (nd == nullptr || nd->deletion_pending == true) + if (nd == nullptr || nd->deletion_pending == npc_data::DELETION_ACTIVE) return; - nd->deletion_pending = true; + nd->deletion_pending = npc_data::DELETION_ACTIVE; nd->flag |= 1; clif_clearchar(nd, BeingRemoveWhy::GONE); npc_propagate_update(nd); diff --git a/src/map/script-fun.cpp b/src/map/script-fun.cpp index 0014805..13b4bc8 100644 --- a/src/map/script-fun.cpp +++ b/src/map/script-fun.cpp @@ -1283,8 +1283,19 @@ void builtin_destroy(ScriptState *st) npc_event_dequeue(sd); } + // Cancel all existing timers on the NPC. + // They "would" never fire, and we don't want race conditions here. + for (int i = 0; i < MAX_EVENTTIMER; i++) + { + nd->eventtimer[i].cancel(); + } + // Schedule the NPC to be freed on the next available tick. + // Scripts can be invoked under iteration of the ev_db global event + // database, and we don't want to invalidate active iterators. + nd->deletion_pending = npc_data::DELETION_QUEUED; + nd->eventtimer[0] = Timer(gettick(), std::bind(npc_free, nd)); + nd = nd->is_script(); - npc_free(nd); st->oid = BlockId(); if (!HARG(0)) @@ -1350,7 +1361,7 @@ void builtin_puppet(ScriptState *st) nd->npc_subtype = NpcSubtype::SCRIPT; npc_script++; - nd->deletion_pending = false; + nd->deletion_pending = npc_data::NOT_DELETING; nd->n = map_addnpc(nd->bl_m, nd); -- cgit v1.2.3-70-g09d2