-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
fixed ocl.cpp data races without requiring additional locking #27638
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 4.x
Are you sure you want to change the base?
Conversation
not ready for review |
@opencv-alalek could you take a look? |
882cbc5
to
dc0bfd0
Compare
Improved, ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no real data race issues.
You need to provide workarounds for TSAN instead.
@@ -6602,17 +6599,16 @@ class OpenCLAllocator CV_FINAL : public MatAllocator | |||
|
|||
void flushCleanupQueue() const | |||
{ | |||
if (!cleanupQueue.empty()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forcing mutex usage is a performance degradation.
@@ -3500,7 +3500,7 @@ struct Kernel::Impl | |||
|
|||
void addUMat(const UMat& m, bool dst) | |||
{ | |||
CV_Assert(nu < MAX_ARRS && m.u && m.u->urefcount > 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not drop sanity checks.
We don't need exact values of refcount, we just check that object is still alive (any value > 0).
@@ -5670,9 +5670,6 @@ class OpenCLAllocator CV_FINAL : public MatAllocator | |||
if(!u) | |||
return; | |||
|
|||
CV_Assert(u->urefcount == 0); | |||
CV_Assert(u->refcount == 0 && "UMat deallocation error: some derived Mat is still alive"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should not drop sanity checks.
We don't need exact values of refcount, we just check that object is still alive (any value > 0) or not.
Ok, i understand. i will write source comments for those false positives so others can quickly discard them. |
See: #27637
Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.