Merge pull request #20602 from diablodale:fix20594-refcounts-wiith-exceptions

modules/core/src/matrix.cpp

@@ -749,18 +749,17 @@ Mat::Mat(const Mat& m, const Rect& roi)
     data += roi.x*esz;
     CV_Assert( 0 <= roi.x && 0 <= roi.width && roi.x + roi.width <= m.cols &&
               0 <= roi.y && 0 <= roi.height && roi.y + roi.height <= m.rows );
-    if( u )
-        CV_XADD(&u->refcount, 1);
     if( roi.width < m.cols || roi.height < m.rows )
         flags |= SUBMATRIX_FLAG;
 
     step[0] = m.step[0]; step[1] = esz;
     updateContinuityFlag();
 
+    addref();
     if( rows <= 0 || cols <= 0 )
     {
-        release();
         rows = cols = 0;
+        release();
     }
 }
 

modules/core/src/ocl.cpp

@@ -2774,19 +2774,33 @@ struct Kernel::Impl
 
     void cleanupUMats()
     {
+        bool exceptionOccurred = false;
         for( int i = 0; i < MAX_ARRS; i++ )
+        {
             if( u[i] )
             {
                 if( CV_XADD(&u[i]->urefcount, -1) == 1 )
                 {
                     u[i]->flags |= UMatData::ASYNC_CLEANUP;
-                    u[i]->currAllocator->deallocate(u[i]);
+                    try
+                    {
+                        u[i]->currAllocator->deallocate(u[i]);
+                    }
+                    catch(const std::exception& exc)
+                    {
+                        // limited by legacy before C++11, therefore log and
+                        // remember some exception occurred to throw below
+                        CV_LOG_ERROR(NULL, "OCL: Unexpected C++ exception in OpenCL Kernel::Impl::cleanupUMats(): " << exc.what());
+                        exceptionOccurred = true;
+                    }
                 }
                 u[i] = 0;
             }
+        }
         nu = 0;
         haveTempDstUMats = false;
         haveTempSrcUMats = false;
+        CV_Assert(!exceptionOccurred);
     }
 
     void addUMat(const UMat& m, bool dst)
@@ -2817,8 +2831,16 @@ struct Kernel::Impl
     void finit(cl_event e)
     {
         CV_UNUSED(e);
-        cleanupUMats();
         isInProgress = false;
+        try
+        {
+            cleanupUMats();
+        }
+        catch(...)
+        {
+            release();
+            throw;
+        }
         release();
     }
 

modules/core/src/umatrix.cpp

@@ -540,13 +540,26 @@ UMat Mat::getUMat(int accessFlags, UMatUsageFlags usageFlags) const
         CV_XADD(&(u->refcount), 1);
         CV_XADD(&(u->urefcount), 1);
     }
-    hdr.flags = flags;
-    setSize(hdr, dims, size.p, step.p);
-    finalizeHdr(hdr);
-    hdr.u = new_u;
-    hdr.offset = 0; //data - datastart;
-    hdr.addref();
-    return hdr;
+    try
+    {
+        hdr.flags = flags;
+        setSize(hdr, dims, size.p, step.p);
+        finalizeHdr(hdr);
+        hdr.u = new_u;
+        hdr.offset = 0; //data - datastart;
+        hdr.addref();
+        return hdr;
+    }
+    catch(...)
+    {
+        if (u != NULL)
+        {
+            CV_XADD(&(u->refcount), -1);
+            CV_XADD(&(u->urefcount), -1);
+        }
+        new_u->currAllocator->deallocate(new_u);
+        throw;
+    }
 }
 
 void UMat::create(int d, const int* _sizes, int _type, UMatUsageFlags _usageFlags)
@@ -692,18 +705,17 @@ UMat::UMat(const UMat& m, const Rect& roi)
     offset += roi.x*esz;
     CV_Assert( 0 <= roi.x && 0 <= roi.width && roi.x + roi.width <= m.cols &&
               0 <= roi.y && 0 <= roi.height && roi.y + roi.height <= m.rows );
-    if( u )
-        CV_XADD(&(u->urefcount), 1);
     if( roi.width < m.cols || roi.height < m.rows )
         flags |= SUBMATRIX_FLAG;
 
     step[0] = m.step[0]; step[1] = esz;
     updateContinuityFlag();
 
+    addref();
     if( rows <= 0 || cols <= 0 )
     {
-        release();
         rows = cols = 0;
+        release();
     }
 }
 
@@ -969,24 +981,29 @@ Mat UMat::getMat(int accessFlags) const
     // TODO Support ACCESS_READ (ACCESS_WRITE) without unnecessary data transfers
     accessFlags |= ACCESS_RW;
     UMatDataAutoLock autolock(u);
-    if(CV_XADD(&u->refcount, 1) == 0)
-        u->currAllocator->map(u, accessFlags);
-    if (u->data != 0)
+    try
     {
-        Mat hdr(dims, size.p, type(), u->data + offset, step.p);
-        hdr.flags = flags;
-        hdr.u = u;
-        hdr.datastart = u->data;
-        hdr.data = u->data + offset;
-        hdr.datalimit = hdr.dataend = u->data + u->size;
-        return hdr;
+        if(CV_XADD(&u->refcount, 1) == 0)
+            u->currAllocator->map(u, accessFlags);
+        if (u->data != 0)
+        {
+            Mat hdr(dims, size.p, type(), u->data + offset, step.p);
+            hdr.flags = flags;
+            hdr.u = u;
+            hdr.datastart = u->data;
+            hdr.data = u->data + offset;
+            hdr.datalimit = hdr.dataend = u->data + u->size;
+            return hdr;
+        }
     }
-    else
+    catch(...)
     {
         CV_XADD(&u->refcount, -1);
-        CV_Assert(u->data != 0 && "Error mapping of UMat to host memory.");
-        return Mat();
+        throw;
     }
+    CV_XADD(&u->refcount, -1);
+    CV_Assert(u->data != 0 && "Error mapping of UMat to host memory.");
+    return Mat();
 }
 
 void* UMat::handle(int accessFlags) const

modules/core/test/test_umat.cpp

@@ -1419,4 +1419,37 @@ TEST(UMat, resize_Mat_issue_13577)
     cv::ocl::setUseOpenCL(useOCL);  // restore state
 }
 
+TEST(UMat, exceptions_refcounts_issue_20594)
+{
+    if (!cv::ocl::useOpenCL())
+    {
+        // skip test, difficult to create exception scenario without OpenCL
+        std::cout << "OpenCL is not enabled. Skip test" << std::endl;
+        return;
+    }
+
+    UMat umat1(10, 10, CV_8UC1);
+    EXPECT_EQ(0, umat1.u->refcount);
+
+    // cause exception in underlying allocator
+    void* const original_handle = umat1.u->handle;
+    umat1.u->handle = NULL;
+    try
+    {
+        Mat mat1 = umat1.getMat(ACCESS_RW);
+    }
+    catch (...)
+    {
+        // nothing
+    }
+
+    // check for correct refcount, and no change of intentional bad handle
+    EXPECT_EQ(0, umat1.u->refcount);
+    EXPECT_EQ(NULL, umat1.u->handle);
+
+    // reset UMat to good state
+    umat1.u->refcount = 0;
+    umat1.u->handle = original_handle;
+}
+
 } } // namespace opencv_test::ocl