npc: Deregister events with `ev_db` on free

In serverdata@c3b7fe59a, `magic-knuckles` made use of temporary copies of the
spell NPC via `puppet`/`destroy` to provide additional functionality. This
implementation was correct with respect to tmwa behaviour as documented.

However, `puppet` and `destroy` doesn't quite return the server to a clean
prior state. In `builtin_puppet`, events with the fresh new copy of NPC are
registered into the global `ev_db` map to track named events, such as
`OnDischarge`, which might be triggered later. However, these registrations
are not reversed with `destroy`, leaving `ev_db` to retain references to now
invalid and destroyed NPCs.

serverdata@c3b7fe59a revealed this oversight through an intersection of rare
conditions:
 - An NPC that is routinely cloned and destroyed
 - Having a variety of events
 - Including one that can be triggered during a search for the event over ALL
   NPCs when `#discharge` is cast.

To reproduce, compile with `-fsanitize=address -fsanitize=undefined` to more
reliably catch use of invalid memory, cast `magic-knuckles`, log out the
character which cast `magic-knuckles` to destroy the NPC, log back in, then
cast `#discharge`. The server will then crash.

Special thanks to SystemError for testing out this theory. Currently lacking
a test environment of my own, this would not have been possible without his
patience and diligence.

1 files changed, 12 insertions, 0 deletions

diff --git a/src/map/npc.cpp b/src/map/npc.cpp
index ee2f30c..efb2ba3 100644
--- a/src/map/npc.cpp
+++ b/src/map/npc.cpp
@@ -1050,6 +1050,18 @@ void npc_free_internal(dumb_ptr<npc_data> nd_)
         nd_->bl_m->npc[nd_->n] = nullptr;
     }
 
+    // Also clean up any events we registered to the global ev_db
+    if (auto nd = nd_->is_script())
+    {
+        for (auto& pair : ev_db)
+        {
+            if (pair.second.nd == nd)
+            {
+                ev_db.erase(pair.first);
+            }
+        }
+    }
+
     nd_.delete_();
 }