HPCC-25527 Fix potential crash when re-adding ptree

Signed-off-by: Jake Smith <jake.smith@lexisnexisrisk.com>

system/jlib/jptree.cpp

@@ -282,6 +282,23 @@ IPropertyTreeIterator *ChildMap::getIterator(bool sort)
     return createSortedIterator(*baseIter);
 }
 
+void ChildMap::onRemove(void *e)
+{
+    PTree &elem = static_cast<PTree &>(*(IPropertyTree *)e);
+    IPTArrayValue *value = elem.queryValue();
+    if (value && value->isArray())
+    {
+        IPropertyTree **elems = value->getRawArray();
+        IPropertyTree **elemsEnd = elems+value->elements();
+        while (elems != elemsEnd)
+        {
+            PTree *pElem = static_cast<PTree *>(*elems++);
+            pElem->setOwner(nullptr);
+        }
+    }
+    elem.Release();
+}
+
 ///////////
 
 bool validateXMLTag(const char *name)
@@ -1203,11 +1220,10 @@ void CPTArray::setElement(unsigned idx, IPropertyTree *tree)
 void CPTArray::removeElement(unsigned idx)
 {
     CQualifierMap *map = queryMap();
+    IPropertyTree *existing = &((IPropertyTree &)item(idx));
     if (map)
-    {
-        IPropertyTree *existing = &((IPropertyTree &)item(idx));
         map->removeMatchingValues(existing);
-    }
+    ((PTree *)existing)->setOwner(nullptr);
     remove(idx);
 }
 

system/jlib/jptree.ipp

@@ -49,11 +49,7 @@ class jlib_decl ChildMap : protected SuperHashTableOf<IPropertyTree, constcharpt
 protected:
 // SuperHashTable definitions
     virtual void onAdd(void *) override {}
-    virtual void onRemove(void *e) override
-    {
-        IPropertyTree &elem = *(IPropertyTree *)e;
-        elem.Release();
-    }
+    virtual void onRemove(void *e) override;
     virtual const void *getFindParam(const void *e) const override
     {
         const IPropertyTree &elem = *(const IPropertyTree *)e;

testing/unittests/jlibtests.cpp

@@ -1228,6 +1228,7 @@ class JlibIPTTest : public CppUnit::TestFixture
         CPPUNIT_TEST(testRootArrayMarkup);
         CPPUNIT_TEST(testArrayMarkup);
         CPPUNIT_TEST(testMergeConfig);
+        CPPUNIT_TEST(testRemoveReuse);
     CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -1931,6 +1932,23 @@ subX:
             ++match;
         }
     }
+    void testRemoveReuse()
+    {
+        Owned<IPropertyTree> t = createPTree();
+        t->addPropInt("a", 1);
+        t->addPropInt("a", 2);
+        Owned<IPropertyTree> a1 = t->getPropTree("a[1]");
+        Owned<IPropertyTree> a2 = t->getPropTree("a[2]");
+        CPPUNIT_ASSERT(t->removeProp("a[2]"));
+        CPPUNIT_ASSERT(t->removeProp("a"));
+        CPPUNIT_ASSERT(!a1->isArray(nullptr));
+        CPPUNIT_ASSERT(!a2->isArray(nullptr));
+        IPropertyTree *na2 = t->addPropTree("na", a2.getClear());
+        CPPUNIT_ASSERT(!na2->isArray(nullptr));
+        IPropertyTree *na1 = t->addPropTree("na", a1.getClear());
+        CPPUNIT_ASSERT(na1->isArray(nullptr));
+        CPPUNIT_ASSERT(na2->isArray(nullptr));
+    }
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(JlibIPTTest);