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 24, 2022
1 parent e5ea329 commit 896d66a
Showing 1 changed file with 42 additions and 3 deletions.
45 changes: 42 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,47 @@ 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
if (savedObjectID == 0 || rSavedObjectID == 0) {
O ref = getReferenceUnsafeNoReload();
O rRef = r.getReferenceUnsafeNoReload();

// if both are nullptr only the savedObjectIDs matter
if (ref == nullptr && rRef == nullptr)
return savedObjectID != rSavedObjectID;
// if one of them is null they can't be equal
else if (ref == nullptr || rRef == nullptr)
return true;
// if both are not null we can check the real object id
else
return ref->_getObjectID() != rRef->getObjectID();
} else
return savedObjectID != rSavedObjectID;
}

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
if (savedObjectID == 0 || rSavedObjectID == 0) {
O ref = getReferenceUnsafeNoReload();
O rRef = r.getReferenceUnsafeNoReload();

return savedObjectID == r.savedObjectID.load(std::memory_order_relaxed);;
if (ref == nullptr && rRef == nullptr)
return savedObjectID == rSavedObjectID;
else if (ref == nullptr || rRef == nullptr)
return false;
else
return ref->_getObjectID() == rRef->getObjectID();
} else
return savedObjectID == rSavedObjectID;
}

inline ManagedReference<O> get() {
Expand Down

0 comments on commit 896d66a

Please sign in to comment.