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

Delesley Hutchins delesley at google.com
Thu Nov 1 11:48:27 PDT 2012


Thanks to everyone who has contributed to the discussion!  Since I've
received a lot of suggestions for extensions that would avoid the need
to refer to private mutexes by name, I get the impression that people
are not comfortable with turning access control off.  Is that a valid
perception?  Does anybody want to take a stab at arguing that it
should be turned off?  :-)

I also wanted to post a slightly different example, to further clarify
the issue.  This example does not have lock() and unlock() functions,
because locking is always done internal to the class; it thus better
represents many of my real-world use cases.  Also, my original
workaround (using LOCK_RETURNED) seems to have gotten buried in the
depths of my original post, so I show it in its entirety here.   Note
in particular that TaskSet is not a friend class, so
TaskSet::addToQueue is not able to lock or unlock q.mu_.  However, it
does require that q.mu_ is held, so the attribute uses the const
getter: q.getMu().

What do people think of this workaround?  Does it seem reasonable?

class TaskSet;

class TaskQueue {
public:
  // makes mu_ visible, but does not allow external code to lock or unlock it.
  const Mutex& getMu() LOCK_RETURNED(mu_) { return mu_; }

  virtual void addTask(int taskID) EXCLUSIVE_LOCKS_REQUIRED(getMu()) {
    queue.push_back(taskID);
  }

  void addTaskSync(int taskID) {
    mu_.Lock();
    addTask(taskID);
    mu_.Unlock();
  }

  void addTaskSetSync(TaskSet& set);

private:
  Mutex mu_;
  std::deque<int> queue GUARDED_BY(mu_);
};


class TaskSet {
public:
  // Adds tasks to queue consecutively, without releasing the lock
between additions.
  void addToQueue(TaskQueue& q) EXCLUSIVE_LOCKS_REQUIRED(q.getMu()) {
    for (unsigned i=0, n=tasks_.size(); i<n; ++i)
      q.addTask(tasks_[i]);
  }

private:
  std::vector<int> tasks_;
};


void TaskQueue::addTaskSetSync(TaskSet& set) {
  mu_.Lock();
  set.addToQueue(*this);
  mu_.Unlock();
}



More information about the cfe-dev mailing list