Page MenuHomePhabricator

Convert = operators that take object mutexes to the multi-lock version of std::lock
ClosedPublic

Authored by jingham on Mar 28 2019, 1:00 PM.

Details

Summary

I made a copy-paste error copying the ModuleList trick for avoiding priority inversion in taking the lhs & rhs mutexes. Pavel pointed out you can use the std::lock(lock1, lock2) form to do this more cleanly. I fixed the one I got wrong and I think I found all the other places doing this (or just locking both mutexes) and replaced them with this form.

Diff Detail

Repository
rL LLVM

Event Timeline

jingham created this revision.Mar 28 2019, 1:00 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 28 2019, 1:00 PM

This looks correct to me, but I'm not extremely familiar either, so I'd wait for @labath to sign off.

labath accepted this revision.Mar 28 2019, 11:33 PM

This looks great. Thank you.

As of c++17 there will be an RAII way to do this https://en.cppreference.com/w/cpp/thread/scoped_lock. Theoretically we could jump ahead, and implement something like that ourselves, but there aren't too many uses of this pattern now, so it's not really necessary at this point.

This revision is now accepted and ready to land.Mar 28 2019, 11:33 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2019, 10:06 AM