python: fix meshtools.filter_*_attributes() to reference the owner.

These don't copy the data but rather reference the original with
different metadata. That caused use-after-free originally (if the input
variable got deleted or overwritten), the previous commit made it
assert (which was the intention there). Now it works properly.

src/python/magnum/meshtools.cpp

@@ -40,6 +40,9 @@
 #include <Magnum/MeshTools/Transform.h>
 #include <Magnum/Trade/MeshData.h>
 
+#include "Corrade/PythonBindings.h"
+#include "Magnum/Trade/PythonBindings.h"
+
 #include "corrade/EnumOperators.h"
 #include "magnum/bootstrap.h"
 
@@ -129,10 +132,16 @@ void meshtools(py::module_& m) {
             return MeshTools::duplicate(mesh);
         }, "Duplicate indexed mesh data", py::arg("mesh"))
         .def("filter_except_attributes", [](const Trade::MeshData& mesh, const std::vector<Trade::MeshAttribute> attributes) {
-            return MeshTools::filterExceptAttributes(mesh, attributes);
+            /* If the mesh already has an owner, use that instead to avoid
+               long reference chains */
+            py::object meshOwner = pyObjectHolderFor<Trade::PyDataHolder>(mesh).owner;
+            return Trade::pyDataHolder(MeshTools::filterExceptAttributes(mesh, attributes), meshOwner.is_none() ? py::cast(mesh) : std::move(meshOwner));
         }, "Filter a mesh to contain everything except the selected subset of named attributes", py::arg("mesh"), py::arg("attributes"))
         .def("filter_only_attributes", [](const Trade::MeshData& mesh, const std::vector<Trade::MeshAttribute> attributes) {
-            return MeshTools::filterOnlyAttributes(mesh, attributes);
+            /* If the mesh already has an owner, use that instead to avoid
+               long reference chains */
+            py::object meshOwner = pyObjectHolderFor<Trade::PyDataHolder>(mesh).owner;
+            return Trade::pyDataHolder(MeshTools::filterOnlyAttributes(mesh, attributes), meshOwner.is_none() ? py::cast(mesh) : std::move(meshOwner));
         }, "Filter a mesh to contain only the selected subset of named attributes", py::arg("mesh"), py::arg("attributes"))
         .def("generate_indices", [](const Trade::MeshData& mesh) {
             if(mesh.primitive() != MeshPrimitive::LineStrip &&

src/python/magnum/test/test_meshtools.py

@@ -24,6 +24,7 @@
 #
 
 import os
+import sys
 import unittest
 
 from magnum import *
@@ -106,23 +107,61 @@ class GenerateIndices(unittest.TestCase):
 class FilterAttributes(unittest.TestCase):
     def test_only(self):
         mesh = primitives.cube_solid()
+        mesh_refcount = sys.getrefcount(mesh)
         self.assertEqual(mesh.attribute_count(), 2)
         self.assertTrue(mesh.has_attribute(trade.MeshAttribute.NORMAL))
 
         # Currently it doesn't blow up if unknown attributes are listed
         filtered = meshtools.filter_only_attributes(mesh, [trade.MeshAttribute.TEXTURE_COORDINATES, trade.MeshAttribute.NORMAL])
+        filtered_refcount = sys.getrefcount(filtered)
         self.assertEqual(filtered.attribute_count(), 1)
         self.assertTrue(filtered.has_attribute(trade.MeshAttribute.NORMAL))
+        self.assertEqual(sys.getrefcount(mesh), mesh_refcount + 1)
+        self.assertIs(filtered.owner, mesh)
+
+        # Subsequent filtering will still reference the original mesh, not the
+        # intermediates
+        filtered2 = meshtools.filter_only_attributes(filtered, [trade.MeshAttribute.NORMAL])
+        self.assertEqual(filtered.attribute_count(), 1)
+        self.assertTrue(filtered.has_attribute(trade.MeshAttribute.NORMAL))
+        self.assertEqual(sys.getrefcount(filtered), filtered_refcount)
+        self.assertEqual(sys.getrefcount(mesh), mesh_refcount + 2)
+        self.assertIs(filtered2.owner, mesh)
+
+        del filtered
+        self.assertEqual(sys.getrefcount(mesh), mesh_refcount + 1)
+
+        del filtered2
+        self.assertEqual(sys.getrefcount(mesh), mesh_refcount)
 
     def test_except(self):
         mesh = primitives.cube_solid()
+        mesh_refcount = sys.getrefcount(mesh)
         self.assertEqual(mesh.attribute_count(), 2)
         self.assertTrue(mesh.has_attribute(trade.MeshAttribute.NORMAL))
 
         # Currently it doesn't blow up if unknown attributes are listed
         filtered = meshtools.filter_except_attributes(mesh, [trade.MeshAttribute.TEXTURE_COORDINATES, trade.MeshAttribute.NORMAL])
+        filtered_refcount = sys.getrefcount(filtered)
         self.assertEqual(filtered.attribute_count(), 1)
-        self.assertFalse(filtered.has_attribute(trade.MeshAttribute.NORMAL))
+        self.assertTrue(filtered.has_attribute(trade.MeshAttribute.POSITION))
+        self.assertEqual(sys.getrefcount(mesh), mesh_refcount + 1)
+        self.assertIs(filtered.owner, mesh)
+
+        # Subsequent filtering will still reference the original mesh, not the
+        # intermediates
+        filtered2 = meshtools.filter_except_attributes(filtered, [trade.MeshAttribute.NORMAL])
+        self.assertEqual(filtered.attribute_count(), 1)
+        self.assertTrue(filtered.has_attribute(trade.MeshAttribute.POSITION))
+        self.assertEqual(sys.getrefcount(filtered), filtered_refcount)
+        self.assertEqual(sys.getrefcount(mesh), mesh_refcount + 2)
+        self.assertIs(filtered2.owner, mesh)
+
+        del filtered
+        self.assertEqual(sys.getrefcount(mesh), mesh_refcount + 1)
+
+        del filtered2
+        self.assertEqual(sys.getrefcount(mesh), mesh_refcount)
 
 class Interleave(unittest.TestCase):
     def test(self):