Repository: robotpy/mostrobotpy (language: python)

Issue:
cscore cv::Mat converter segfault
Original CD post: https://www.chiefdelphi.com/t/segmentation-fault-from-cvsink/453849/6

The core problem is that the original numpy array is being destroyed, and the new numpy array is referring to the old data. To prevent the crash from happening, you just need to keep a reference to the original data.

Here's a reproducer:

```
def test_nonempty_img():
    img = np.zeros(shape=(480, 640, 3), dtype=np.uint8)
    sink = cs.CvSink("something")
    _, img = sink.grabFrame(img)
    _, img = sink.grabFrame(img) # crashes here because img is a new numpy object, not the original one
```


Discussion / hints:

Fix patch:
diff --git a/subprojects/robotpy-cscore/cscore/cvnp/cvnp.cpp b/subprojects/robotpy-cscore/cscore/cvnp/cvnp.cpp
index 18a1c7dc..5950218b 100644
--- a/subprojects/robotpy-cscore/cscore/cvnp/cvnp.cpp
+++ b/subprojects/robotpy-cscore/cscore/cvnp/cvnp.cpp
@@ -8,8 +8,88 @@ namespace cvnp
 
     namespace detail
     {
+        // #define DEBUG_ALLOCATOR
+
+        #ifdef DEBUG_ALLOCATOR
+        int nbAllocations = 0;
+        #endif
+
         namespace py = pybind11;
-        
+
+        // Translated from cv2_numpy.cpp in OpenCV source code
+        class CvnpAllocator : public cv::MatAllocator
+        {
+        public:
+            CvnpAllocator() = default;
+            ~CvnpAllocator() = default;
+
+            // Attaches a numpy array object to a cv::Mat
+            static void attach_nparray(cv::Mat &m, pybind11::array& a)
+            {
+                static CvnpAllocator instance;
+
+                cv::UMatData* u = new cv::UMatData(&instance);
+                u->data = u->origdata = (uchar*)a.mutable_data(0);
+                u->size = a.size();
+                u->userdata = a.inc_ref().ptr();
+                u->refcount = 1;
+
+                #ifdef DEBUG_ALLOCATOR
+                ++nbAllocations;
+                printf("CvnpAllocator::attach_nparray(py::array) nbAllocations=%d\n", nbAllocations);
+                #endif
+
+                m.u = u;
+                m.allocator = &instance;
+            }
+
+            cv::UMatData* allocate(int dims0, const int* sizes, int type, void* data, size_t* step, cv::AccessFlag flags, cv::UMatUsageFlags usageFlags) const override
+            {
+                throw py::value_error("CvnpAllocator::allocate \"standard\" should never happen");
+                // return stdAllocator->allocate(dims0, sizes, type, data, step, flags, usageFlags);
+            }
+
+            bool allocate(cv::UMatData* u, cv::AccessFlag accessFlags, cv::UMatUsageFlags usageFlags) const override
+            {
+                throw py::value_error("CvnpAllocator::allocate \"copy\" should never happen");
+                // return stdAllocator->allocate(u, accessFlags, usageFlags);
+            }
+
+            void deallocate(cv::UMatData* u) const override
+            {
+                if(!u)
+                {
+                    #ifdef DEBUG_ALLOCATOR
+                    printf("CvnpAllocator::deallocate() with null ptr!!! nbAllocations=%d\n", nbAllocations);
+                    #endif
+                    return;
+                }
+
+                // This function can be called from anywhere, so need the GIL
+                py::gil_scoped_acquire gil;
+                assert(u->urefcount >= 0);
+                assert(u->refcount >= 0);
+                if(u->refcount == 0)
+                {
+                    PyObject* o = (PyObject*)u->userdata;
+                    Py_XDECREF(o);
+                    delete u;
+                    #ifdef DEBUG_ALLOCATOR
+                    --nbAllocations;
+                    printf("CvnpAllocator::deallocate() nbAllocations=%d\n", nbAllocations);
+                    #endif
+                }
+                else
+                {
+                    #ifdef DEBUG_ALLOCATOR
+                    printf("CvnpAllocator::deallocate() - not doing anything since urefcount=%d nbAllocations=%d\n",
+                            u->urefcount,
+                           nbAllocations);
+                    #endif
+                }
+            }
+        };
+
         py::dtype determine_np_dtype(int cv_depth)
         {
             for (auto format_synonym : cvnp::sTypeSynonyms)
@@ -127,10 +207,15 @@ namespace cvnp
         int type = detail::determine_cv_type(a, depth);
         cv::Size size = detail::determine_cv_size(a);
         cv::Mat m(size, type, is_not_empty ? a.mutable_data(0) : nullptr);
+
+        if (is_not_empty) {
+            detail::CvnpAllocator::attach_nparray(m, a); //, ndims, size, type, step);
+        }
+
         return m;
     }
 
-    // this version tries to handles strides and submatrices
+    // this version tries to handle strides and sub-matrices
     // this is WIP, currently broken, and not used
     cv::Mat nparray_to_mat_with_strides_broken(pybind11::array& a)
     {
diff --git a/subprojects/robotpy-cscore/cscore/cvnp/cvnp_shared_mat.h b/subprojects/robotpy-cscore/cscore/cvnp/cvnp_shared_mat.h
deleted file mode 100644
index f22d0871..00000000
--- a/subprojects/robotpy-cscore/cscore/cvnp/cvnp_shared_mat.h
+++ /dev/null
@@ -1,101 +0,0 @@
-#pragma once
-
-#include <opencv2/core/core.hpp>
-
-
-namespace cvnp
-{
-    //
-    // Wrappers for cv::Matx, cv::Mat and cv::Vec when you explicitly intend to share memory
-    //
-
-    struct Mat_shared
-    {
-        Mat_shared(const cv::Mat value = cv::Mat()): Value(value) {};
-        cv::Mat Value;
-    };
-
-
-    template<typename Tp, int m, int n>
-    struct Matx_shared
-    {
-        Matx_shared(const cv::Matx<Tp, m, n>& value = cv::Matx<Tp, m, n>()): Value(value) {};
-        cv::Matx<Tp, m, n> Value;
-    };
-
-
-    template<typename Tp, int m>
-    struct Vec_shared : public cv::Vec<Tp, m>
-    {
-        Vec_shared(const cv::Vec<Tp, m>& value): Value(value) {};
-        cv::Vec<Tp, m> Value;
-    };
-
-
-    typedef Matx_shared<float, 1, 2> Matx_shared12f;
-    typedef Matx_shared<double, 1, 2> Matx_shared12d;
-    typedef Matx_shared<float, 1, 3> Matx_shared13f;
-    typedef Matx_shared<double, 1, 3> Matx_shared13d;
-    typedef Matx_shared<float, 1, 4> Matx_shared14f;
-    typedef Matx_shared<double, 1, 4> Matx_shared14d;
-    typedef Matx_shared<float, 1, 6> Matx_shared16f;
-    typedef Matx_shared<double, 1, 6> Matx_shared16d;
-
-    typedef Matx_shared<float, 2, 1> Matx_shared21f;
-    typedef Matx_shared<double, 2, 1> Matx_shared21d;
-    typedef Matx_shared<float, 3, 1> Matx_shared31f;
-    typedef Matx_shared<double, 3, 1> Matx_shared31d;
-    typedef Matx_shared<float, 4, 1> Matx_shared41f;
-    typedef Matx_shared<double, 4, 1> Matx_shared41d;
-    typedef Matx_shared<float, 6, 1> Matx_shared61f;
-    typedef Matx_shared<double, 6, 1> Matx_shared61d;
-
-    typedef Matx_shared<float, 2, 2> Matx_shared22f;
-    typedef Matx_shared<double, 2, 2> Matx_shared22d;
-    typedef Matx_shared<float, 2, 3> Matx_shared23f;
-    typedef Matx_shared<double, 2, 3> Matx_shared23d;
-    typedef Matx_shared<float, 3, 2> Matx_shared32f;
-    typedef Matx_shared<double, 3, 2> Matx_shared32d;
-
-    typedef Matx_shared<float, 3, 3> Matx_shared33f;
-    typedef Matx_shared<double, 3, 3> Matx_shared33d;
-
-    typedef Matx_shared<float, 3, 4> Matx_shared34f;
-    typedef Matx_shared<double, 3, 4> Matx_shared34d;
-    typedef Matx_shared<float, 4, 3> Matx_shared43f;
-    typedef Matx_shared<double, 4, 3> Matx_shared43d;
-
-    typedef Matx_shared<float, 4, 4> Matx_shared44f;
-    typedef Matx_shared<double, 4, 4> Matx_shared44d;
-    typedef Matx_shared<float, 6, 6> Matx_shared66f;
-    typedef Matx_shared<double, 6, 6> Matx_shared66d;
-
-
-    typedef Vec_shared<uchar, 2> Vec_shared2b;
-    typedef Vec_shared<uchar, 3> Vec_shared3b;
-    typedef Vec_shared<uchar, 4> Vec_shared4b;
-
-    typedef Vec_shared<short, 2> Vec_shared2s;
-    typedef Vec_shared<short, 3> Vec_shared3s;
-    typedef Vec_shared<short, 4> Vec_shared4s;
-
-    typedef Vec_shared<ushort, 2> Vec_shared2w;
-    typedef Vec_shared<ushort, 3> Vec_shared3w;
-    typedef Vec_shared<ushort, 4> Vec_shared4w;
-
-    typedef Vec_shared<int, 2> Vec_shared2i;
-    typedef Vec_shared<int, 3> Vec_shared3i;
-    typedef Vec_shared<int, 4> Vec_shared4i;
-    typedef Vec_shared<int, 6> Vec_shared6i;
-    typedef Vec_shared<int, 8> Vec_shared8i;
-
-    typedef Vec_shared<float, 2> Vec_shared2f;
-    typedef Vec_shared<float, 3> Vec_shared3f;
-    typedef Vec_shared<float, 4> Vec_shared4f;
-    typedef Vec_shared<float, 6> Vec_shared6f;
-
-    typedef Vec_shared<double, 2> Vec_shared2d;
-    typedef Vec_shared<double, 3> Vec_shared3d;
-    typedef Vec_shared<double, 4> Vec_shared4d;
-    typedef Vec_shared<double, 6> Vec_shared6d;
-} // namespace cvnp


Test patch:
diff --git a/subprojects/robotpy-cscore/tests/test_cv.py b/subprojects/robotpy-cscore/tests/test_cv.py
index 3e780545..d63d0cba 100644
--- a/subprojects/robotpy-cscore/tests/test_cv.py
+++ b/subprojects/robotpy-cscore/tests/test_cv.py
@@ -7,3 +7,12 @@ def test_empty_img():
     sink = cs.CvSink("something")
     _, rimg = sink.grabFrame(img)
     assert (rimg == img).all()
+
+
+def test_nonempty_img():
+    img = np.zeros(shape=(480, 640, 3), dtype=np.uint8)
+    img2 = np.zeros(shape=(480, 640, 3), dtype=np.uint8)
+    sink = cs.CvSink("something")
+    _, img = sink.grabFrame(img)
+    _, img = sink.grabFrame(img)
+    assert (img == img2).all()