[cfe-dev] [libc++] thread safety annotations for unique_lock

[Richard Trieu via cfe-dev]

On Tue, Nov 8, 2016 at 10:54 AM, christoph at ruediger.engineering via cfe-dev
<cfe-dev at lists.llvm.org> wrote:

> Sorry for the incomplete mail; hit the wrong shortcut. The mail continues
> down below.
>
> Am 08.11.16, 19:38 schrieb "cfe-dev im Auftrag von cfe-dev at lists.llvm.org"
> <cfe-dev-bounces at lists.llvm.org im Auftrag von cfe-dev at lists.llvm.org>:
>
> >Hi everyone,
> >
> >this is my first post to this list, so please remind me if I’m violating
> your netiquette in some way.
> >
> >I’m trying to introduce the thread safety analysis to one of our projects
> and stumble upon the missing thread safety annotations for unique_lock. I
> thought this might be easy, so let’s provide a patch. But now I’m stuck
> with correctly annotating the move constructor and the lock() and unlock()
> methods of unique_lock.
> >
> >Here is a very basic implementation of my annotated unique_lock:
> >
> >class SCOPED_CAPABILITY unique_lock {
> >  ::bluebox::mutex *mu;
> >  bool lock_held;
> >public:
> >  unique_lock() = delete;
> >  unique_lock(unique_lock &&) = delete;   // We cannot annotate move
> semantics
> >
> >  explicit unique_lock(::bluebox::mutex &m) ACQUIRE(m) : mu(&m),
> lock_held(true) { m.lock(); }
> >  explicit unique_lock(::bluebox::mutex *m) ACQUIRE(m) : mu(m),
> lock_held(true) { m->lock(); }
>

Change this to ACQUIRE(mu) instead of ACQUIRE(m).  The ACQUIRE and RELEASE
annotations need to refer to the same variable for the thread safety
analysis to work.


> >
> >  void unlock() RELEASE(mu) NO_THREAD_SAFETY_ANALYSIS {
> >    if (lock_held) {
> >      lock_held = false;
> >      mu->unlock();
> >    }
> >  }
> >
> >  void lock() ACQUIRE(mu) NO_THREAD_SAFETY_ANALYSIS {
> >    if (!lock_held) {
> >      mu->lock();
> >      lock_held = true;
> >    }
> >  }
> >
> >  ~unique_lock() RELEASE() NO_THREAD_SAFETY_ANALYSIS {
> >    if (lock_held)
> >      mu->unlock();
> >  }
> >};
> >
> >
> >And this is my test case:
> >
> >static std::mutex mtx;
> >void someFunction() {
> >  my::unique_lock lock(mtx);
> >  // … Here you would normally do something meaningful
> >  lock.unlock();
> >  // … Call a method that might modify your protected data
> >  lock.lock();
> >  // … Continue with super important work
> >}
> >
> >And here is my list of issues I can’t get around with:
> >1. How can I correctly annotate the unlock and lock methods? When using
> unlock() in the above example, clang throws this error: fatal error:
> releasing mutex
> >      'lock.mu' that was not held [-Wthread-safety-analysis]
> >2. How can I correctly annotate the move constructor of unique_lock?
>
> 3. I couldn’t find really that much valuable information about the
> annotations except for the one page in the clang documentation and a talk
> from DeLesly Hutchins. Plus the information, that the guys at Google are
> making heavy use of the thread safety analyzer. But are you really limiting
> yourself to std::mutex and std::lock_guard? You can’t even use
> std::condition_variable. Maybe one of the Google engineers on this list can
> shade some light without revealing confidential information.
>
> With these first tests in mind, I’m having troubles to use this really
> helpful feature in my projects. Anybody who wants to share some experience
> is appreciated. I will try to update the documentation accordingly or
> provide any other help.
>
> Thanks and regards,
> Christoph
>
> --
> rüdiger.engineering
> Christoph Rüdiger
> Düsseldorfer Str. 12
> 45145 Essen
> Germany
>
> phone: +49 201 458 478 58
> VAT ID/Ust. ID: DE304572498
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20161108/8717f9b5/attachment.html>