[cfe-dev] Discussion: should we enforce access control in C++ attributes?

Delesley Hutchins delesley at google.com
Thu Nov 1 10:27:41 PDT 2012


> Your analysis recognizes arbitrary expressions in these contexts as long
> as they resolve to something that works like a lock, right?  Why not also,
> as an alternative, permit arbitrary expressions that happen to resolve
> to calls to methods with the THREAD_SAFETY_ACQUIRES_LOCK
> attribute?  e.g.
>   THREAD_SAFETY_REQUIRES_LOCK(lock())
> In other words, I am simply proposing that you accept an alternate way
> of specifying the lock;  this should not deeply affect your algorithm.

Yes.  I think this is a good idea, and it's similar to what David
Blakie proposed.  However, I think this should be an alternative
mechanism for referring to locks, not the default.  The biggest
problem with referencing the lock() function directly is that locking
is a side-effect.  Functions often do much more than acquire a lock,
and multiple locks can be acquired in one function.  The example I
gave was very simple.  In more complex examples, you have functions
like:

  StatusCode initializeTableAndLock() EXCLUSIVE_LOCK_FUNCTION(mu1, mu2) { ... }

Calling initializeTableAndLock() directly within an attribute is
nonsense, since attribute expressions are supposed to be side-effect
free.  The attribute syntax thus becomes something like:

  EXCLUSIVE_LOCKS_REQUIRED(locks_acquired_by(initializeTableAndLock))

Which is a bit unwieldy.  We have a fairly large user base here at
google, and in my experience, it's usually best to separate the
mutexes from functions which lock them.  In fact, in the vast majority
of my existing use cases, there's no lock method at all -- I put
lock() and unlock() methods in my example mainly for illustrative
purposes, so that people could see how the annotations work.  In most
cases, the mutex is acquired and released directly from within
internal methods, like addTaskSync().

> Sure, but why not befriend the helper class or function if it's going to
> rely on internal details of this other class?

I think this is an argument in favor of keeping access control turned
on.  Is that what you mean?

C++ does provide existing mechanisms (friends, public getters, etc.)
to provide access to other code where necessary.  Right now I require
users to go through those mechanisms.  However, I've received some
requests from users to turn off access control, because they have
found it to be a pain, and they believe thread-safety annotations are
a special case that should not be subject to access control
restrictions.

Part of the motivation is that users typically just want to annotate
their existing code.  They want the annotations to work; they don't
want to refactor their code, make things public that were not
previously public, or add friend declarations just to make the
annotations happy.

  -DeLesley

-- 
DeLesley Hutchins | Software Engineer | delesley at google.com | 505-206-0315



More information about the cfe-dev mailing list