From 22a120b00390aadd35871e77afc616f58feaf282 Mon Sep 17 00:00:00 2001
From: Paul Manias <paul@parasol.ws>
Date: Mon, 3 Jul 2023 16:27:36 +0100
Subject: [PATCH] [Core] Improved stability of UnsubscribeAction() by avoiding
 object pointer references

---
 src/core/defs.h          |  9 +++++----
 src/core/lib_objects.cpp | 18 ++++++++++--------
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/src/core/defs.h b/src/core/defs.h
index fa0a476a7..2340c1f9a 100644
--- a/src/core/defs.h
+++ b/src/core/defs.h
@@ -234,12 +234,13 @@ class PrivateAddress {
 //********************************************************************************************************************
 
 struct ActionSubscription {
-   OBJECTPTR Context;
+   OBJECTPTR Subscriber; // The object that initiated the subscription
+   OBJECTID SubscriberID; // Required for sanity checks on subscriber still existing.
    void (*Callback)(OBJECTPTR, ACTIONID, ERROR, APTR);
 
-   ActionSubscription() : Context(NULL), Callback(NULL) { }
-   ActionSubscription(OBJECTPTR pContext, void (*pCallback)(OBJECTPTR, ACTIONID, ERROR, APTR)) : Context(pContext), Callback(pCallback) { }
-   ActionSubscription(OBJECTPTR pContext, APTR pCallback) : Context(pContext), Callback((void (*)(OBJECTPTR, ACTIONID, ERROR, APTR))pCallback) { }
+   ActionSubscription() : Subscriber(NULL), SubscriberID(0), Callback(NULL) { }
+   ActionSubscription(OBJECTPTR pContext, void (*pCallback)(OBJECTPTR, ACTIONID, ERROR, APTR)) : Subscriber(pContext), SubscriberID(pContext->UID), Callback(pCallback) { }
+   ActionSubscription(OBJECTPTR pContext, APTR pCallback) : Subscriber(pContext), SubscriberID(pContext->UID), Callback((void (*)(OBJECTPTR, ACTIONID, ERROR, APTR))pCallback) { }
 };
 
 struct virtual_drive {
diff --git a/src/core/lib_objects.cpp b/src/core/lib_objects.cpp
index af114d0c2..159fc1736 100644
--- a/src/core/lib_objects.cpp
+++ b/src/core/lib_objects.cpp
@@ -436,10 +436,12 @@ ERROR Action(LONG ActionID, OBJECTPTR Object, APTR Parameters)
 
       glSubReadOnly++;
 
-      if ((glSubscriptions.contains(Object->UID)) and (glSubscriptions[Object->UID].contains(ActionID))) {
-         for (auto &list : glSubscriptions[Object->UID][ActionID]) {
-            pf::SwitchContext ctx(list.Context);
-            list.Callback(Object, ActionID, (error IS ERR_NoAction) ? ERR_Okay : error, Parameters);
+      if (auto it = glSubscriptions.find(Object->UID); it != glSubscriptions.end()) {
+         if (it->second.contains(ActionID)) {
+            for (auto &list : it->second[ActionID]) {
+               pf::SwitchContext ctx(list.Subscriber);
+               list.Callback(Object, ActionID, (error IS ERR_NoAction) ? ERR_Okay : error, Parameters);
+            }
          }
       }
 
@@ -1413,8 +1415,8 @@ void NotifySubscribers(OBJECTPTR Object, LONG ActionID, APTR Parameters, ERROR E
    if ((!glSubscriptions[Object->UID].empty()) and (!glSubscriptions[Object->UID][ActionID].empty())) {
       glSubReadOnly++; // Prevents changes to glSubscriptions while we're processing it.
       for (auto &sub : glSubscriptions[Object->UID][ActionID]) {
-         if (sub.Context) {
-            pf::SwitchContext ctx(sub.Context);
+         if (sub.Subscriber) {
+            pf::SwitchContext ctx(sub.Subscriber);
             sub.Callback(Object, ActionID, ErrorCode, Parameters);
          }
       }
@@ -1918,7 +1920,7 @@ ERROR UnsubscribeAction(OBJECTPTR Object, ACTIONID ActionID)
 restart:
          for (auto & [action, list] : glSubscriptions[Object->UID]) {
             for (auto it = list.begin(); it != list.end(); ) {
-               if (it->Context->UID IS subscriber) it = list.erase(it);
+               if (it->SubscriberID IS subscriber) it = list.erase(it);
                else it++;
             }
 
@@ -1942,7 +1944,7 @@ ERROR UnsubscribeAction(OBJECTPTR Object, ACTIONID ActionID)
 
       auto &list = glSubscriptions[Object->UID][ActionID];
       for (auto it = list.begin(); it != list.end(); ) {
-         if (it->Context->UID IS subscriber) it = list.erase(it);
+         if (it->SubscriberID IS subscriber) it = list.erase(it);
          else it++;
       }