Add PTHREAD_PRIO_INHERIT Locks for Mac

base/synchronization/lock.h

Index: base/synchronization/lock.h
diff --git a/base/synchronization/lock.h b/base/synchronization/lock.h
index fbf6cef76911c80332da1524beeb23fd14737917..fb15c66004457f91ef19080e6d7ff7a38c1c09f1 100644
--- a/base/synchronization/lock.h
+++ b/base/synchronization/lock.h
@@ -14,6 +14,22 @@
 
 namespace base {
 
+#if defined(OS_POSIX)

[danakj, 2016/07/27 22:47:53]

Why is this behind OS_POSIX? I'd expect it to just be 0 on non-posix?

[robliao, 2016/07/28 20:06:09]

The intention was to only have this in POSIX since it only impacts POSIX
platforms. Windows platforms don't have such an option, so they really shouldn't
be checking this.

I've updated the comment to emphasis the compile time nature for this flag.
(e.g. priority inheritance code may not compile on all POSIX platforms).

[danakj, 2016/07/28 20:49:03]

I see. It's just that to check this now you will have to do

#if OS_POSIX && PRIORITY_INHERITANCE_LOCKS_POSSIBLE()

But it'd be simpler to just

#if PRIORITY_INHERITANCE_LOCKS_POSSIBLE() 

and it's always false on posix?

[robliao, 2016/07/28 21:08:38]

My sense is that code will fall under one of two categories:

1) Code in a *_posix* file
The OS_POSIX check isn't necessary and you'd use
PRIORITY_INHERITANCE_LOCKS_POSSIBLE directly.

2) Code in a common file
You'd want to use OS_POSIX to make sure you don't get included in the future on
some other platform.

Is there something I'm missing?

+
+// Determines which platforms can consider using priority inheritance locks. If
+// priority inheritance is possible, it does not necessarily mean it's
+// available. Lock::PriorityInheritanceAvailable must still be checked.
+// If you do not require the conditional inclusion at compile time,
+// simply call Lock::PriorityInheritanceAvailable, which will provide the same
+// result.
+#if defined(OS_NACL) || defined(OS_ANDROID)
+#define PRIORITY_INHERITANCE_LOCKS_POSSIBLE() 0
+#else
+#define PRIORITY_INHERITANCE_LOCKS_POSSIBLE() 1
+#endif
+
+#endif
+
 // A convenient wrapper for an OS specific critical section.  The only real
 // intelligence in this class is in debug mode for the support for the
 // AssertAcquired() method.
@@ -61,6 +77,14 @@ class BASE_EXPORT Lock {
   void AssertAcquired() const;
 #endif  // DCHECK_IS_ON()
 
+#if defined(OS_POSIX)

[danakj, 2016/07/27 22:47:53]

And why is this behind OS_POSIX instead of behind
PRIORITY_INHERITANCE_LOCKS_POSSIBLE()?

[robliao, 2016/07/28 20:06:09]

See above. This is intended to be the runtime check. Code that is universally
compileable (no dependency on pthreads) can use this runtime check.

+  // Whether this platform has priority inheritance available. All locks will
+  // attempt to use the priority inheritance version if available.
+  static bool PriorityInheritanceAvailable() {
+    return internal::LockImpl::PriorityInheritanceAvailable();
+  }

[fdoray, 2016/07/28 14:38:36]

Could this be available on all platforms and called something like
IsProtectedAgainstPriorityInversion() or
CanBeUsedWithMultipleThreadPriorities()? Then we wouldn't need an #if + a call
to this method to know whether we can use thread priorities in
base/task_scheduler.

[robliao, 2016/07/28 20:06:09]

The #if is still necessary if code won't compile. (Both the sandbox and lock
impl ran into this due to non-uniform pthreads headers). Since the complication
is only on POSIX, I'd prefer to limit the check to POSIX.

+#endif
+
 #if defined(OS_POSIX) || defined(OS_WIN)
   // Both Windows and POSIX implementations of ConditionVariable need to be
   // able to see our lock and tweak our debugging counters, as they release and