Fix borrowable_ptr - #14914
Conversation
How exactly does the |
|
We reset only the private copy, no other is reading from. |
to avoid a race condition with reset(). Also remove copy constructor, because writing the shared_ptr itself is not thread safe.
|
In another branch I have tested this is our CO system and it reveals some issues. This is now in shape, with constant correctness. |
| } | ||
| QThread::usleep(1); | ||
| for (int k = 0; k < 2; ++k) { |
There was a problem hiding this comment.
why the second loop and the sleep?
There was a problem hiding this comment.
I was not able to reproduce the task switch, which causes the test failure. This sleep forces it. I will add a comment.
|
This PR is marked as stale because it has been open 90 days with no activity. |
github-actions
Bot
added
the
stale
JoergAtGithub
removed
the
stale
|
if you move this to a branch inside mixxxdj then I can have copilot do a review, which is often useful (especially on complex code that few really understand) |
|
Done |
|
hm it's still showing up as coming from daschuer:borrowable_ptr, I'm not sure if it's possible to edit this PR without creating a new one |
|
Review comment from #16005 are done. |
ywwg
left a comment
ywwg
left a comment
There was a problem hiding this comment.
this is looking good -- I had one more question on the other PR thread just to clarify: #16005 (comment)
|
one last question about the commented-out unlock |
9021776 to
d3fba27
Compare
|
Done |
|
let me know if you want to make any last tweaks before merge |
|
This is ready to merge |
4 participants
Use an immutable std::weak_ptr for creating borrowed_ptr to avoid a race condition with reset(). Also remove copy constructor, because writing the shared_ptr itself is not thread safe.
Shall fix #14828
Thanks to @Hapsa21 for pointing giving the hints in #14906