#5623: The MergeOperation should hold strong references on the two ro…

…ot nodes, such that they don't get released before the operation reaches end of life
codereader · May 24, 2021 · 338d4df · 338d4df
1 parent a95f159
commit 338d4df
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 8 deletions.
diff --git a/libs/scene/merge/ComparisonResult.h b/libs/scene/merge/ComparisonResult.h
@@ -96,6 +96,11 @@ class ComparisonResult
     {
         return _baseRoot;
     }
+
+    const scene::IMapRootNodePtr& getSourceRootNode() const
+    {
+        return _sourceRoot;
+    }
 };
 
 }

diff --git a/libs/scene/merge/MergeOperation.cpp b/libs/scene/merge/MergeOperation.cpp
@@ -209,7 +209,7 @@ void MergeOperation::createActionsForEntity(const ComparisonResult::EntityDiffer
         break;
 
     case ComparisonResult::EntityDifference::Type::EntityMissingInBase:
-        addAction(std::make_shared<AddEntityAction>(difference.sourceNode, _targetRoot));
+        addAction(std::make_shared<AddEntityAction>(difference.sourceNode, _baseRoot));
         break;
 
     case ComparisonResult::EntityDifference::Type::EntityPresentButDifferent:
@@ -228,11 +228,11 @@ void MergeOperation::createActionsForEntity(const ComparisonResult::EntityDiffer
     };
 }
 
-MergeOperation::Ptr MergeOperation::CreateFromComparisonResult(const ComparisonResult& comparisonResult)
+MergeOperation::Ptr MergeOperation::CreateFromComparisonResult(const ComparisonResult& result)
 {
-    auto operation = std::make_shared<MergeOperation>(comparisonResult.getBaseRootNode());
+    auto operation = std::make_shared<MergeOperation>(result.getSourceRootNode(), result.getBaseRootNode());
 
-    for (const auto& difference : comparisonResult.differingEntities)
+    for (const auto& difference : result.differingEntities)
     {
         operation->createActionsForEntity(difference);
     }

diff --git a/libs/scene/merge/MergeOperation.h b/libs/scene/merge/MergeOperation.h
@@ -12,19 +12,21 @@ namespace merge
 {
 
 // A MergeOperation groups one or more merge actions
-// together in order to apply a set of changes to an existing map
+// together in order to apply a set of changes from source => base
 class MergeOperation
 {
 private:
-    scene::IMapRootNodePtr _targetRoot;
+    scene::IMapRootNodePtr _sourceRoot;
+    scene::IMapRootNodePtr _baseRoot;
 
     std::list<MergeAction::Ptr> _actions;
 
 public:
     using Ptr = std::shared_ptr<MergeOperation>;
 
-    MergeOperation(scene::IMapRootNodePtr targetRoot) :
-        _targetRoot(targetRoot)
+    MergeOperation(scene::IMapRootNodePtr sourceRoot, scene::IMapRootNodePtr baseRoot) :
+        _sourceRoot(sourceRoot),
+        _baseRoot(baseRoot)
     {}
 
     // Creates the merge operation from the given comparison result.