Skip to content

Commit

Permalink
[fixed] fix pitfall when using == and != with MangedWeakReference
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
fallchildren2 committed Aug 25, 2022
1 parent e5ea329 commit 32449f4
Showing 1 changed file with 33 additions and 3 deletions.
36 changes: 33 additions & 3 deletions MMOEngine/src/engine/core/ManagedWeakReference.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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() {
Expand Down

0 comments on commit 32449f4

Please sign in to comment.