From 32449f4c7ce7430029ef189084694be18e46e3ee Mon Sep 17 00:00:00 2001
From: Sebastian Fieber <fallchildren@disroot.org>
Date: Wed, 24 Aug 2022 12:55:08 +0200
Subject: [PATCH] [fixed] fix pitfall when using == and != with
 MangedWeakReference

The following steps should illustrate the problem:

1. Object A is freshly constructed and gets an _objectID of 0.
2. ManagedWeakReference of A is created -> savedObjectID will be also 0
3. ObjectManager::persist is called and A gets a validi _objectID
4. ManagedWeakReference of A will still have the savedObjectID of 0
   and will result in wrong behaviour of == and !=

To circumvent these this commit changes the case when one of the
compared ManagedWeakReference instances has a savedObjectID of 0 and
checks the actual reference object for the real _objectID.
---
 .../src/engine/core/ManagedWeakReference.h    | 36 +++++++++++++++++--
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/MMOEngine/src/engine/core/ManagedWeakReference.h b/MMOEngine/src/engine/core/ManagedWeakReference.h
index 93ca478b..9555dba3 100644
--- a/MMOEngine/src/engine/core/ManagedWeakReference.h
+++ b/MMOEngine/src/engine/core/ManagedWeakReference.h
@@ -102,6 +102,12 @@ namespace engine {
 			return ref;
 		}
 
+		inline O getReferenceUnsafeNoReload() const {
+			O ref = WeakReference<O>::getReferenceUnsafeStaticCast();
+
+			return ref;
+		}
+
 		inline O getReferenceUnsafe() {
 			O ref = WeakReference<O>::getReferenceUnsafeStaticCast();
 
@@ -143,14 +149,38 @@ namespace engine {
 
 		inline bool operator!=(const ManagedWeakReference<O>& r) const {
 			auto savedObjectID = this->savedObjectID.load(std::memory_order_relaxed);
-
-			return savedObjectID != r.savedObjectID.load(std::memory_order_relaxed);
+			auto rSavedObjectID = r.savedObjectID.load(std::memory_order_relaxed);
+
+			// the savedObjectID may be incorrectly be 0
+			// if object O was given an objectID after
+			// creating the ManagedWeakReference - so we
+			// have to check for the actual objectID of O
+			// here
+                        auto objectID = savedObjectID == 0 ?
+                            getReferenceUnsafeNoReload() == nullptr ? 0 : getReferenceUnsafeNoReload()->getObjectID()
+                            : savedObjectID;
+
+                        auto rObjectID = rSavedObjectID == 0 ?
+                            r.getReferenceUnsafeNoReload() == nullptr ? 0 : r.getReferenceUnsafeNoReload()->getObjectID()
+                            : rSavedObjectID;
+
+                        return objectID != rObjectID;
 		}
 
 		inline bool operator==(const ManagedWeakReference<O>& r) const {
 			auto savedObjectID = this->savedObjectID.load(std::memory_order_relaxed);
+			auto rSavedObjectID = r.savedObjectID.load(std::memory_order_relaxed);
+
+                        // see comment on same logic in the operator!= code
+                        auto objectID = savedObjectID == 0 ?
+                            getReferenceUnsafeNoReload() == nullptr ? 0 : getReferenceUnsafeNoReload()->getObjectID()
+                            : savedObjectID;
+
+                        auto rObjectID = rSavedObjectID == 0 ?
+                            r.getReferenceUnsafeNoReload() == nullptr ? 0 : r.getReferenceUnsafeNoReload()->getObjectID()
+                            : rSavedObjectID;
 
-			return savedObjectID == r.savedObjectID.load(std::memory_order_relaxed);;
+                        return objectID == rObjectID;
 		}
 
 		inline ManagedReference<O> get() {