From 9bda96d39e7d63da97ef38a0dcb04ff9d0670a01 Mon Sep 17 00:00:00 2001 From: Dale Phurrough Date: Tue, 24 Aug 2021 20:06:36 +0200 Subject: [PATCH 1/5] add test case --- modules/core/test/test_umat.cpp | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/modules/core/test/test_umat.cpp b/modules/core/test/test_umat.cpp index c2bd6eceba..c323d17c06 100644 --- a/modules/core/test/test_umat.cpp +++ b/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 From 54a9e009703567c03d8fccbee7bb43f945b396a1 Mon Sep 17 00:00:00 2001 From: Dale Phurrough Date: Tue, 24 Aug 2021 18:56:25 +0200 Subject: [PATCH 2/5] fix opencv/opencv#20594 - exception handling with refcounts --- modules/core/src/matrix.cpp | 5 ++- modules/core/src/ocl.cpp | 26 +++++++++++++-- modules/core/src/umatrix.cpp | 63 +++++++++++++++++++++++------------- 3 files changed, 66 insertions(+), 28 deletions(-) diff --git a/modules/core/src/matrix.cpp b/modules/core/src/matrix.cpp index 4db7c7a89e..1deff6b450 100644 --- a/modules/core/src/matrix.cpp +++ b/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(); } } diff --git a/modules/core/src/ocl.cpp b/modules/core/src/ocl.cpp index 8b1377eec6..2d85a32366 100644 --- a/modules/core/src/ocl.cpp +++ b/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(); } diff --git a/modules/core/src/umatrix.cpp b/modules/core/src/umatrix.cpp index 94f828ba60..da01f3785f 100644 --- a/modules/core/src/umatrix.cpp +++ b/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 From 38d0063c367b2697b57ab0809530889cc2e20fd7 Mon Sep 17 00:00:00 2001 From: Vincent Rabaud Date: Thu, 26 Aug 2021 09:46:25 +0200 Subject: [PATCH 3/5] Do not use deprecated ReleaseCleared in protobuf library. This is to make code work with protobuf arenas for memory management (ReleaseCleared is incompatible). The cleaning of the memory is also simpler. --- modules/dnn/src/caffe/caffe_importer.cpp | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/modules/dnn/src/caffe/caffe_importer.cpp b/modules/dnn/src/caffe/caffe_importer.cpp index 8673be03fb..2d615c448a 100644 --- a/modules/dnn/src/caffe/caffe_importer.cpp +++ b/modules/dnn/src/caffe/caffe_importer.cpp @@ -306,16 +306,13 @@ public: caffe::LayerParameter* binLayer = netBinary.mutable_layer(li); const int numBlobs = binLayer->blobs_size(); + std::vector blobs(numBlobs); + binLayer->mutable_blobs()->ExtractSubrange(0, numBlobs, blobs.data()); layerParams.blobs.resize(numBlobs); for (int bi = 0; bi < numBlobs; bi++) { - blobFromProto(binLayer->blobs(bi), layerParams.blobs[bi]); - } - binLayer->clear_blobs(); - CV_Assert(numBlobs == binLayer->blobs().ClearedCount()); - for (int bi = 0; bi < numBlobs; bi++) - { - delete binLayer->mutable_blobs()->ReleaseCleared(); + blobFromProto(*blobs[bi], layerParams.blobs[bi]); + delete blobs[bi]; } } From 8ee33ca5511c186c81f0e2e9b705e5b0c5cc1935 Mon Sep 17 00:00:00 2001 From: Alexander Alekhin Date: Sat, 28 Aug 2021 01:28:34 +0000 Subject: [PATCH 4/5] java(test): avoid deprecation warning - 'new Byte' => 'Byte.valueOf' --- .../org/opencv/test/utils/ConvertersTest.java | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/modules/java/test/common_test/src/org/opencv/test/utils/ConvertersTest.java b/modules/java/test/common_test/src/org/opencv/test/utils/ConvertersTest.java index 54d2736c46..88d75dcdfb 100644 --- a/modules/java/test/common_test/src/org/opencv/test/utils/ConvertersTest.java +++ b/modules/java/test/common_test/src/org/opencv/test/utils/ConvertersTest.java @@ -28,9 +28,9 @@ public class ConvertersTest extends OpenCVTestCase { byte value1 = 2; byte value2 = 4; byte value3 = 3; - truth.add(new Byte(value1)); - truth.add(new Byte(value2)); - truth.add(new Byte(value3)); + truth.add(Byte.valueOf(value1)); + truth.add(Byte.valueOf(value2)); + truth.add(Byte.valueOf(value3)); assertEquals(truth, bs); } @@ -248,9 +248,9 @@ public class ConvertersTest extends OpenCVTestCase { byte value1 = 2; byte value2 = 4; byte value3 = 3; - truth.add(new Byte(value1)); - truth.add(new Byte(value2)); - truth.add(new Byte(value3)); + truth.add(Byte.valueOf(value1)); + truth.add(Byte.valueOf(value2)); + truth.add(Byte.valueOf(value3)); assertEquals(truth, bs); } @@ -276,10 +276,10 @@ public class ConvertersTest extends OpenCVTestCase { byte value2 = 2; byte value3 = 3; byte value4 = 4; - bytes.add(new Byte(value1)); - bytes.add(new Byte(value2)); - bytes.add(new Byte(value3)); - bytes.add(new Byte(value4)); + bytes.add(Byte.valueOf(value1)); + bytes.add(Byte.valueOf(value2)); + bytes.add(Byte.valueOf(value3)); + bytes.add(Byte.valueOf(value4)); dst = Converters.vector_char_to_Mat(bytes); truth = new Mat(4, 1, CvType.CV_8SC1); @@ -499,10 +499,10 @@ public class ConvertersTest extends OpenCVTestCase { byte value2 = 2; byte value3 = 3; byte value4 = 4; - bytes.add(new Byte(value1)); - bytes.add(new Byte(value2)); - bytes.add(new Byte(value3)); - bytes.add(new Byte(value4)); + bytes.add(Byte.valueOf(value1)); + bytes.add(Byte.valueOf(value2)); + bytes.add(Byte.valueOf(value3)); + bytes.add(Byte.valueOf(value4)); dst = Converters.vector_uchar_to_Mat(bytes); truth = new Mat(4, 1, CvType.CV_8UC1); From 076587425e66a1dd66d06e9f274acd6f2eda2a3f Mon Sep 17 00:00:00 2001 From: Alexander Alekhin Date: Sat, 28 Aug 2021 17:11:26 +0000 Subject: [PATCH 5/5] build: eliminate build warnings --- modules/features2d/src/kaze/KAZEFeatures.cpp | 63 ++++++++++--------- modules/photo/src/contrast_preserve.hpp | 4 +- modules/superres/src/btv_l1.cpp | 2 +- .../TrackingMotion/cornerDetector_Demo.cpp | 2 +- 4 files changed, 39 insertions(+), 32 deletions(-) diff --git a/modules/features2d/src/kaze/KAZEFeatures.cpp b/modules/features2d/src/kaze/KAZEFeatures.cpp index 58f0937f00..ab591d417c 100644 --- a/modules/features2d/src/kaze/KAZEFeatures.cpp +++ b/modules/features2d/src/kaze/KAZEFeatures.cpp @@ -311,7 +311,7 @@ private: void KAZEFeatures::Determinant_Hessian(std::vector& kpts) { int level = 0; - float dist = 0.0, smax = 3.0; + float smax = 3.0; int npoints = 0, id_repeated = 0; int left_x = 0, right_x = 0, up_y = 0, down_y = 0; bool is_extremum = false, is_repeated = false, is_out = false; @@ -338,17 +338,24 @@ void KAZEFeatures::Determinant_Hessian(std::vector& kpts) for (int j = 0; j < (int)kpts_par_[i].size(); j++) { level = i + 1; + const TEvolution& evolution_level = evolution_[level]; + is_extremum = true; is_repeated = false; is_out = false; - // Check in case we have the same point as maxima in previous evolution levels - for (int ik = 0; ik < (int)kpts.size(); ik++) { - if (kpts[ik].class_id == level || kpts[ik].class_id == level + 1 || kpts[ik].class_id == level - 1) { - dist = pow(kpts_par_[i][j].pt.x - kpts[ik].pt.x, 2) + pow(kpts_par_[i][j].pt.y - kpts[ik].pt.y, 2); + const KeyPoint& kpts_par_ij = kpts_par_[i][j]; - if (dist < evolution_[level].sigma_size*evolution_[level].sigma_size) { - if (kpts_par_[i][j].response > kpts[ik].response) { + // Check in case we have the same point as maxima in previous evolution levels + for (int ik = 0; ik < (int)kpts.size(); ik++) + { + const KeyPoint& kpts_ik = kpts[ik]; + if (kpts_ik.class_id == level || kpts_ik.class_id == level + 1 || kpts_ik.class_id == level - 1) { + Point2f diff = kpts_par_ij.pt - kpts_ik.pt; + float dist = diff.dot(diff); + + if (dist < evolution_level.sigma_size*evolution_level.sigma_size) { + if (kpts_par_ij.response > kpts_ik.response) { id_repeated = ik; is_repeated = true; } @@ -363,23 +370,23 @@ void KAZEFeatures::Determinant_Hessian(std::vector& kpts) if (is_extremum == true) { // Check that the point is under the image limits for the descriptor computation - left_x = cvRound(kpts_par_[i][j].pt.x - smax*kpts_par_[i][j].size); - right_x = cvRound(kpts_par_[i][j].pt.x + smax*kpts_par_[i][j].size); - up_y = cvRound(kpts_par_[i][j].pt.y - smax*kpts_par_[i][j].size); - down_y = cvRound(kpts_par_[i][j].pt.y + smax*kpts_par_[i][j].size); + left_x = cvRound(kpts_par_ij.pt.x - smax*kpts_par_ij.size); + right_x = cvRound(kpts_par_ij.pt.x + smax*kpts_par_ij.size); + up_y = cvRound(kpts_par_ij.pt.y - smax*kpts_par_ij.size); + down_y = cvRound(kpts_par_ij.pt.y + smax*kpts_par_ij.size); - if (left_x < 0 || right_x >= evolution_[level].Ldet.cols || - up_y < 0 || down_y >= evolution_[level].Ldet.rows) { + if (left_x < 0 || right_x >= evolution_level.Ldet.cols || + up_y < 0 || down_y >= evolution_level.Ldet.rows) { is_out = true; } if (is_out == false) { if (is_repeated == false) { - kpts.push_back(kpts_par_[i][j]); + kpts.push_back(kpts_par_ij); npoints++; } else { - kpts[id_repeated] = kpts_par_[i][j]; + kpts[id_repeated] = kpts_par_ij; } } } @@ -513,16 +520,16 @@ public: if (options_.upright) { kpts[i].angle = 0.0; - if (options_.extended) + if (options_.extended) Get_KAZE_Upright_Descriptor_128(kpts[i], desc.ptr((int)i)); else Get_KAZE_Upright_Descriptor_64(kpts[i], desc.ptr((int)i)); } else { - KAZEFeatures::Compute_Main_Orientation(kpts[i], evolution, options_); + KAZEFeatures::Compute_Main_Orientation(kpts[i], evolution, options_); - if (options_.extended) + if (options_.extended) Get_KAZE_Descriptor_128(kpts[i], desc.ptr((int)i)); else Get_KAZE_Descriptor_64(kpts[i], desc.ptr((int)i)); @@ -712,26 +719,26 @@ void KAZE_Descriptor_Invoker::Get_KAZE_Upright_Descriptor_64(const KeyPoint &kpt y1 = (int)(sample_y - 0.5f); x1 = (int)(sample_x - 0.5f); - checkDescriptorLimits(x1, y1, options_.img_width, options_.img_height); + checkDescriptorLimits(x1, y1, options_.img_width, options_.img_height); y2 = (int)(sample_y + 0.5f); x2 = (int)(sample_x + 0.5f); - checkDescriptorLimits(x2, y2, options_.img_width, options_.img_height); + checkDescriptorLimits(x2, y2, options_.img_width, options_.img_height); fx = sample_x - x1; fy = sample_y - y1; - res1 = *(evolution[level].Lx.ptr(y1)+x1); - res2 = *(evolution[level].Lx.ptr(y1)+x2); - res3 = *(evolution[level].Lx.ptr(y2)+x1); - res4 = *(evolution[level].Lx.ptr(y2)+x2); + res1 = *(evolution[level].Lx.ptr(y1)+x1); + res2 = *(evolution[level].Lx.ptr(y1)+x2); + res3 = *(evolution[level].Lx.ptr(y2)+x1); + res4 = *(evolution[level].Lx.ptr(y2)+x2); rx = (1.0f - fx)*(1.0f - fy)*res1 + fx*(1.0f - fy)*res2 + (1.0f - fx)*fy*res3 + fx*fy*res4; - res1 = *(evolution[level].Ly.ptr(y1)+x1); - res2 = *(evolution[level].Ly.ptr(y1)+x2); - res3 = *(evolution[level].Ly.ptr(y2)+x1); - res4 = *(evolution[level].Ly.ptr(y2)+x2); + res1 = *(evolution[level].Ly.ptr(y1)+x1); + res2 = *(evolution[level].Ly.ptr(y1)+x2); + res3 = *(evolution[level].Ly.ptr(y2)+x1); + res4 = *(evolution[level].Ly.ptr(y2)+x2); ry = (1.0f - fx)*(1.0f - fy)*res1 + fx*(1.0f - fy)*res2 + (1.0f - fx)*fy*res3 + fx*fy*res4; rx = gauss_s1*rx; diff --git a/modules/photo/src/contrast_preserve.hpp b/modules/photo/src/contrast_preserve.hpp index 5681779fc9..4079272e99 100644 --- a/modules/photo/src/contrast_preserve.hpp +++ b/modules/photo/src/contrast_preserve.hpp @@ -358,9 +358,9 @@ void Decolor::grayImContruct(vector &wei, const Mat &img, Mat &Gray) co { for(int i = 0;i(i,j)=Gray.at(i,j) + + Gray.at(i,j)=static_cast(Gray.at(i,j) + static_cast(wei[kk])*pow(rgb_channel[2].at(i,j),r)*pow(rgb_channel[1].at(i,j),g)* - pow(rgb_channel[0].at(i,j),b); + pow(rgb_channel[0].at(i,j),b)); kk=kk+1; } diff --git a/modules/superres/src/btv_l1.cpp b/modules/superres/src/btv_l1.cpp index cf2ca58ceb..26c75a94ce 100644 --- a/modules/superres/src/btv_l1.cpp +++ b/modules/superres/src/btv_l1.cpp @@ -356,7 +356,7 @@ namespace for (int m = 0, ind = 0; m <= ksize; ++m) { for (int l = ksize; l + m >= 0; --l, ++ind) - btvWeights[ind] = pow(alpha_f, std::abs(m) + std::abs(l)); + btvWeights[ind] = static_cast(pow(alpha_f, std::abs(m) + std::abs(l))); } } diff --git a/samples/cpp/tutorial_code/TrackingMotion/cornerDetector_Demo.cpp b/samples/cpp/tutorial_code/TrackingMotion/cornerDetector_Demo.cpp index a43024e557..e2835531e1 100644 --- a/samples/cpp/tutorial_code/TrackingMotion/cornerDetector_Demo.cpp +++ b/samples/cpp/tutorial_code/TrackingMotion/cornerDetector_Demo.cpp @@ -62,7 +62,7 @@ int main( int argc, char** argv ) { float lambda_1 = myHarris_dst.at(i, j)[0]; float lambda_2 = myHarris_dst.at(i, j)[1]; - Mc.at(i, j) = lambda_1*lambda_2 - 0.04f*pow( ( lambda_1 + lambda_2 ), 2 ); + Mc.at(i, j) = lambda_1*lambda_2 - 0.04f*((lambda_1 + lambda_2) * (lambda_1 + lambda_2)); } }