fix(lua): CallbackRegistry isolates callback errors and defers mid-dispatch registrations

TriggerEvent called each callback directly, so one error left the frame's
OnEvent script and skipped every callback after it in that dispatch. pfUI
now runs many listeners through one EventRegistry dispatch, so one failing
listener could silence the rest. Each callback now runs through
securecallfunction: the error goes to the engine's script error handler and
the loop goes on, which is retail's secureexecuterange contract. It is
called from TriggerEvent's own loop, not through a vararg helper, because a
vararg Lua function allocates an arg table per call on Lua 5.0.

A callback registered for an event while that event is dispatching now
waits in deferredCallbacks and joins after the outermost dispatch, as on
retail. Before, its key went into the table the pairs loop was walking.
UnregisterCallback clears only owners that are present, and drops pending
deferrals.

The rest now follows retail Blizzard_SharedXMLBase/CallbackRegistry.lua:
UnregisterEvents() clears every event and UnregisterEventsByEventTable
clears the listed ones; GenerateCallbackEvents checks its argument; the
file's own GenerateClosure copy gives way to FunctionUtil's global (one
table per call instead of two, with retail's owner + 4 argument limit).

EventRegistry:OnLoad keeps all instance state, since addons derive their
own registry from the instance (pfUI: CreateFromMixins(EventRegistry)).

Verified in game: an erroring callback reports to the handler and the next
one still runs; a callback registered mid-dispatch first runs on the next
TriggerEvent.

src/embed/AddOns/!!!ClassicAPI/Util/CallbackRegistry.lua

@@ -1,44 +1,25 @@
--- Backport of 3.3.5 FrameXML/CallbackRegistryMixin.lua to vanilla 1.12 / Lua 5.0.
+-- Backport of retail Blizzard_SharedXMLBase/CallbackRegistry.lua to vanilla
+-- 1.12 / Lua 5.0.
 --
--- Implementation differences from the 3.3.5 source:
+-- Implementation differences from the retail source:
 --   - 1.12 has no taint or Secure-Templates system, so the entire
 --     AttributeDelegate/SetForbidden machinery for inserting event keys
---     into the callback tables is unnecessary. Sub-tables are allocated
---     lazily inside RegisterCallback instead.
---   - securecallfunction/secureexecuterange don't exist; they reduce to
---     direct calls / explicit for-loops here.
---   - GenerateClosure doesn't exist in 1.12; provide a local one that
---     captures registration-time varargs via SafePack-style storage.
---   - Lua 5.0 has no select() or `wipe()`; we use the implicit `arg`
---     table and assign-nil to clear tables.
+--     into the callback tables and creating deferred callback tables is
+--     unnecessary. Both are plain assignments here.
+--   - TriggerEvent runs each callback through securecallfunction straight
+--     from its own loop, where retail passes a vararg helper to
+--     secureexecuterange. Same contract (an error goes to the error handler
+--     and the remaining callbacks still run), but a vararg Lua function
+--     allocates an `arg` table on every call in Lua 5.0, which would cost a
+--     table per callback per event.
+
+local securecallfunction = securecallfunction
+local wipe = table.wipe
 
 local generateOwnerID = CreateCounter()
 
 local CallbackType = EnumUtil.MakeEnum("Closure", "Function")
 
--- Local GenerateClosure: binds `func` plus all of `...` so a later call
--- invokes func(boundArgs..., callArgs...). Mirrors 3.3.5's helper of the
--- same name. SafePack/SafeUnpack semantics so embedded nils survive.
-local function GenerateClosure(func, ...)
-    local boundN = arg.n
-    local bound = {}
-    for i = 1, boundN do bound[i] = arg[i] end
-    return function(...)
-        local callN = arg.n
-        -- Forward bound + call args faithfully, embedded/leading nils and all.
-        -- The ONLY thing that makes unpack span holes on this Lua 5.0 build is
-        -- a `.n` field: unpack takes its count from getn (which returns t.n)
-        -- and ignores any explicit i/j, so a `.n`-less table stops dead at the
-        -- first hole (verified: unpack({1, 1, nil, 1}, 1, 4) yields just 1, 1).
-        local all = {}
-        local n = 0
-        for i = 1, boundN do n = n + 1; all[n] = bound[i] end
-        for i = 1, callN do n = n + 1; all[n] = arg[i] end
-        all.n = n
-        return func(unpack(all))
-    end
-end
-
 CallbackRegistryMixin = CallbackRegistryMixin or {}
 
 function CallbackRegistryMixin:OnLoad()
@@ -47,6 +28,9 @@ function CallbackRegistryMixin:OnLoad()
         callbackTables[value] = {}
     end
     self.callbackTables = callbackTables
+
+    self.executingEvents = {}
+    self.deferredCallbacks = {}
 end
 
 function CallbackRegistryMixin:SetUndefinedEventsAllowed(allowed)
@@ -66,6 +50,20 @@ function CallbackRegistryMixin:GetCallbacksByEvent(callbackType, event)
     return callbackTable[event]
 end
 
+-- Returns either the underlying callback table or a temporary table depending
+-- on if the event is currently being dispatched. The callbacks stored in the
+-- temporary table are merged into the underlying callback table after the
+-- dispatch is complete, so a registration never adds a key to a table that
+-- TriggerEvent's pairs loop is walking.
+function CallbackRegistryMixin:GetMutableCallbacksByEvent(callbackType, event)
+    if self.executingEvents[event] == nil then
+        return self:GetCallbacksByEvent(callbackType, event)
+    end
+
+    local deferrals = GetOrCreateTableEntry(self.deferredCallbacks, event)
+    return GetOrCreateTableEntry(deferrals, callbackType)
+end
+
 function CallbackRegistryMixin:HasRegistrantsForEvent(event)
     for _, callbackTable in pairs(self:GetCallbackTables()) do
         local callbacks = callbackTable[event]
@@ -76,7 +74,7 @@ function CallbackRegistryMixin:HasRegistrantsForEvent(event)
     return false
 end
 
--- Lazy allocation of per-event sub-tables. In 3.3.5 this went through a
+-- Lazy allocation of per-event sub-tables. On retail this goes through a
 -- forbidden attribute frame for taint-isolation reasons; that's a no-op
 -- consideration on 1.12, so we just assign here.
 function CallbackRegistryMixin:SecureInsertEvent(event)
@@ -102,16 +100,15 @@ function CallbackRegistryMixin:RegisterCallback(event, func, owner, ...)
 
     self:SecureInsertEvent(event)
 
-    for _, callbackTable in pairs(self:GetCallbackTables()) do
-        local callbacks = callbackTable[event]
-        callbacks[owner] = nil
-    end
+    -- An owner can have a single callback per event. The simplest way to ensure
+    -- this is to remove all callbacks for the owner prior to new registration.
+    self:UnregisterCallback(event, owner)
 
     if arg.n > 0 then
-        local callbacks = self:GetCallbacksByEvent(CallbackType.Closure, event)
+        local callbacks = self:GetMutableCallbacksByEvent(CallbackType.Closure, event)
         callbacks[owner] = GenerateClosure(func, owner, unpack(arg))
     else
-        local callbacks = self:GetCallbacksByEvent(CallbackType.Function, event)
+        local callbacks = self:GetMutableCallbacksByEvent(CallbackType.Function, event)
         callbacks[owner] = func
     end
 
@@ -133,6 +130,28 @@ function CallbackRegistryMixin:RegisterCallbackWithHandle(event, func, owner, ..
     return CreateCallbackHandle(self, event, owner)
 end
 
+function CallbackRegistryMixin:ReconcileDeferredCallbacks(event, closures, funcs)
+    self:CopyDeferredCallbacks(CallbackType.Closure, event, closures)
+    self:CopyDeferredCallbacks(CallbackType.Function, event, funcs)
+    self.deferredCallbacks[event] = nil
+end
+
+function CallbackRegistryMixin:CopyDeferredCallbacks(callbackType, event, target)
+    local deferrals = self.deferredCallbacks[event]
+    if deferrals == nil then
+        return
+    end
+
+    local callbacks = deferrals[callbackType]
+    if callbacks == nil then
+        return
+    end
+
+    for owner, callback in pairs(callbacks) do
+        target[owner] = callback
+    end
+end
+
 function CallbackRegistryMixin:TriggerEvent(event, ...)
     if type(event) ~= "string" then
         error("CallbackRegistryMixin:TriggerEvent 'event' requires string type.")
@@ -140,17 +159,24 @@ function CallbackRegistryMixin:TriggerEvent(event, ...)
         error(string.format("CallbackRegistryMixin:TriggerEvent event '%s' doesn't exist.", event))
     end
 
+    -- TriggerEvent appears to need to support reentrant calls for now.
+    local count = (self.executingEvents[event] or 0) + 1
+
+    -- Set before invoking any callback so a reentrant call does not
+    -- attempt to call ReconcileDeferredCallbacks.
+    self.executingEvents[event] = count
+
     local closures = self:GetCallbacksByEvent(CallbackType.Closure, event)
     if closures then
         for _, closure in pairs(closures) do
-            closure(unpack(arg))
+            securecallfunction(closure, unpack(arg))
         end
     end
 
     local funcs = self:GetCallbacksByEvent(CallbackType.Function, event)
     if funcs then
         for owner, func in pairs(funcs) do
-            -- Stock 3.3.5 passes the owner as the first arg so callers can
+            -- Retail passes the owner as the first arg so callers can
             -- distinguish multiple registrations of the same function. `arg`
             -- carries its own `.n`, so unpack(arg) forwards the payload with
             -- leading/embedded nils intact. Do NOT instead merge into a
@@ -158,9 +184,18 @@ function CallbackRegistryMixin:TriggerEvent(event, ...)
             -- build's unpack counts via getn, so a `.n`-less table collapses at
             -- the first hole (verified in-game: reload's PLAYER_ENTERING_WORLD,
             -- arg1=nil arg2=1, dropped the payload).
-            func(owner, unpack(arg))
+            securecallfunction(func, owner, unpack(arg))
         end
     end
+
+    -- The local count is the only value we care about for the purpose
+    -- of flushing the event key and reconciling the deferred callbacks.
+    -- securecallfunction keeps a failing callback from skipping this.
+    if count == 1 then
+        self.executingEvents[event] = nil
+
+        self:ReconcileDeferredCallbacks(event, closures, funcs)
+    end
 end
 
 function CallbackRegistryMixin:UnregisterCallback(event, owner)
@@ -171,37 +206,52 @@ function CallbackRegistryMixin:UnregisterCallback(event, owner)
     end
 
     for _, callbackTable in pairs(self:GetCallbackTables()) do
+        -- Only assign nil if the owner is present in the table, otherwise
+        -- its insertion could cause a rehash during TriggerEvent's walk.
         local callbacks = callbackTable[event]
-        if callbacks then
+        if callbacks and callbacks[owner] then
+            callbacks[owner] = nil
+        end
+    end
+
+    local deferrals = self.deferredCallbacks[event]
+    if deferrals then
+        for _, callbacks in pairs(deferrals) do
+            -- Freely assign nil because deferredCallbacks are never walked
+            -- by a dispatch.
             callbacks[owner] = nil
         end
     end
 end
 
-function CallbackRegistryMixin:UnregisterEvents(eventTable)
-    if eventTable then
-        for _, callbackTable in pairs(self:GetCallbackTables()) do
-            for event in pairs(eventTable) do
-                if callbackTable[event] then
-                    callbackTable[event] = nil
-                end
-            end
-        end
-    else
-        for _, callbackTable in pairs(self:GetCallbackTables()) do
-            for k in pairs(callbackTable) do
-                callbackTable[k] = nil
-            end
+function CallbackRegistryMixin:UnregisterEvents()
+    for _, callbackTable in pairs(self:GetCallbackTables()) do
+        wipe(callbackTable)
+    end
+end
+
+function CallbackRegistryMixin:UnregisterEventsByEventTable(eventTable)
+    if type(eventTable) ~= "table" then
+        error("CallbackRegistryMixin:UnregisterEventsByEventTable 'eventTable' requires table type.")
+    end
+
+    for _, callbackTable in pairs(self:GetCallbackTables()) do
+        for event in pairs(eventTable) do
+            callbackTable[event] = nil
         end
     end
 end
 
-function CallbackRegistryMixin:GenerateCallbackEvents(events)
+function CallbackRegistryMixin:GenerateCallbackEvents(eventTable)
     if not self.Event then
         self.Event = {}
     end
 
-    for eventIndex, eventName in ipairs(events) do
+    if type(eventTable) ~= "table" then
+        error("CallbackRegistryMixin:GenerateCallbackEvents 'eventTable' requires table type.")
+    end
+
+    for _, eventName in ipairs(eventTable) do
         if self.Event[eventName] then
             error(string.format("CallbackRegistryMixin:GenerateCallbackEvents: event '%s' already exists.", eventName))
         end

src/embed/AddOns/!!!ClassicAPI/Util/GlobalCallbackRegistry.lua

@@ -13,6 +13,9 @@
 
 EventRegistry = CreateFromMixins(CallbackRegistryMixin);
 
+-- Keep every instance field in OnLoad. Addons derive their own registry from
+-- this instance (pfUI: CreateFromMixins(EventRegistry) then :OnLoad()), and a
+-- field set anywhere else would be shared with the global registry.
 function EventRegistry:OnLoad()
 	CallbackRegistryMixin.OnLoad(self);
 	self:SetUndefinedEventsAllowed(true);