Implement pthread_rwlock functions for NaCl newlib.

src/untrusted/pthread/nc_rwlock.c

Index: src/untrusted/pthread/nc_rwlock.c
diff --git a/src/untrusted/pthread/nc_rwlock.c b/src/untrusted/pthread/nc_rwlock.c
new file mode 100644
index 0000000000000000000000000000000000000000..dc1feec657793f66656cf05005cad007e8d68d91
--- /dev/null
+++ b/src/untrusted/pthread/nc_rwlock.c
@@ -0,0 +1,219 @@
+/*
+ * Copyright 2014 The Native Client Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+/*
+ * Native Client rwlock implementation
+ *
+ * This implementation avoids writer starvation by preventing readers

[Mark Seaborn, 2014/12/01 22:33:07]

You might want to comment on whether/why you want to avoid writer starvation. 
It's not required by the standard, but might be desirable anyway, though might
come at a performance cost.

[Sam Clegg, 2014/12/10 19:17:43]

I added a link to the wikipedia page with more info.

+ * from acquiring the lock while there are waiting writers.
+ *
+ * The thundering herd problem is avoided by only waking a single
+ * waiter (either a single writer or a single reader) when the
+ * lock is released.
+ */
+
+#include <assert.h>

[Mark Seaborn, 2014/12/01 22:33:07]

Not used

[Sam Clegg, 2014/12/10 19:17:42]

Done.

+#include <errno.h>
+#include <unistd.h>
+
+#include "native_client/src/include/nacl_compiler_annotations.h"
+#include "native_client/src/untrusted/nacl/nacl_irt.h"
+#include "native_client/src/untrusted/pthread/pthread.h"
+#include "native_client/src/untrusted/pthread/pthread_internal.h"
+#include "native_client/src/untrusted/pthread/pthread_types.h"
+
+int pthread_rwlockattr_init(pthread_rwlockattr_t *attr) {
+  attr->type = PTHREAD_PROCESS_PRIVATE;
+  return 0;
+}
+
+int pthread_rwlockattr_destroy(pthread_rwlockattr_t *attr) {
+  return 0;
+}
+
+int pthread_rwlock_init(pthread_rwlock_t *rwlock,
+                        const pthread_rwlockattr_t *attr) {
+  rwlock->writer_thread_id = NACL_PTHREAD_ILLEGAL_THREAD_ID;
+  rwlock->writers_waiting = 0;
+  rwlock->reader_count = 0;
+  pthread_mutex_init(&rwlock->mutex, NULL);

[Mark Seaborn, 2014/12/01 22:33:07]

You should really check for errors from all the pthread calls, even if it's just
to abort execution.

[Sam Clegg, 2014/12/10 19:17:42]

Done.

+  pthread_cond_init(&rwlock->write_possible, NULL);
+  pthread_cond_init(&rwlock->read_possible, NULL);
+  return 0;
+}
+
+/**

[Mark Seaborn, 2014/12/01 22:33:07]

Don't need "/**" here -- it's for doc comments, which wouldn't be needed for
internal functions, and we're not supporting Doxygen anyway.

[Sam Clegg, 2014/12/10 19:17:42]

Done.

+ * Helper function used by waiting writers to determine if they can take
+ * the lock. The rwlock->mutex must be held when calling this function.
+ * Returns 1 if the write lock can be taken, 0 it it can't.
+ */
+static inline int write_lock_available(pthread_rwlock_t *rwlock) {
+  /*
+   * write lock is available if there is no current writer and no current

[Mark Seaborn, 2014/12/01 22:33:07]

Nit: capitalise as "Write"

[Sam Clegg, 2014/12/10 19:17:42]

Done.

+   * readers.
+   */
+  if (rwlock->writer_thread_id != NACL_PTHREAD_ILLEGAL_THREAD_ID)
+    return 0;
+  if (rwlock->reader_count > 0)
+    return 0;
+  return 1;
+}
+
+/**
+ * Helper function used by waiting readers to determine if they can take
+ * the lock. The rwlock->mutex must be held when calling this function.
+ * Returns 1 if the write lock can be taken, 0 it it can't.
+ */
+static inline int read_lock_available(pthread_rwlock_t *rwlock) {
+  /*
+   * read lock is available if there is no current writer and no *waiting*

[Mark Seaborn, 2014/12/01 22:33:07]

Capitalise as "Read" too

[Sam Clegg, 2014/12/10 19:17:42]

Done.

+   * writers.
+   */
+  if (rwlock->writer_thread_id != NACL_PTHREAD_ILLEGAL_THREAD_ID)
+    return 0;
+  if (rwlock->writers_waiting > 0)
+    return 0;
+  return 1;
+}
+
+int pthread_rwlock_timedrdlock(pthread_rwlock_t *rwlock,
+                               const struct timespec *abs_timeout) {
+  int rc = 0;
+  pthread_mutex_lock(&rwlock->mutex);
+  /*
+   * Wait repeatedly until the write preconditions are met.
+   * In theory this loop should only execute once because the preconditions
+   * should always be true when the condition is signaled.
+   */
+  while (!read_lock_available(rwlock)) {
+    rc = pthread_cond_timedwait(&rwlock->read_possible,
+                                &rwlock->mutex,
+                                abs_timeout);
+    if (rc != 0)
+      goto done;
+  }
+
+  /* Acquire the read lock. */
+  rwlock->reader_count++;
+done:
+  pthread_mutex_unlock(&rwlock->mutex);
+  return rc;
+}
+
+int pthread_rwlock_rdlock(pthread_rwlock_t *rwlock) {
+  struct timespec timeout = { 0, 0 };

[Mark Seaborn, 2014/12/01 22:33:07]

This means a zero timeout, I think, so this won't block, which isn't what you
want.

[Sam Clegg, 2014/12/10 19:17:42]

Done.

+  return pthread_rwlock_timedrdlock(rwlock, &timeout);
+}
+
+int pthread_rwlock_tryrdlock(pthread_rwlock_t *rwlock) {

[Mark Seaborn, 2014/12/01 22:33:07]

You *could* merge this into rwlock_lock(), using a try_only flag to abort with
EBUSY before calling pthread_cond_timedwait().

[Sam Clegg, 2014/12/10 19:17:43]

Done.

+  int rc = 0;
+  pthread_mutex_lock(&rwlock->mutex);
+  if (!read_lock_available(rwlock)) {
+    rc = EBUSY;
+    goto done;
+  }
+
+  /* Acquire the read lock. */
+  rwlock->reader_count++;
+done:
+  pthread_mutex_unlock(&rwlock->mutex);
+  return rc;
+}
+
+int pthread_rwlock_timedwrlock(pthread_rwlock_t *rwlock,
+                               const struct timespec *abs_timeout) {
+  int rc = 0;
+  pthread_mutex_lock(&rwlock->mutex);
+
+  /* Wait repeatedly until the write preconditions are met */
+  while (!write_lock_available(rwlock)) {
+    /*
+     * Before waiting (and releasing the lock) we increment the
+     * waiting_writers count so the unlocking code knows to wake
+     * us first (before any waiting readers).
+     */
+    rwlock->writers_waiting++;
+    rc = pthread_cond_timedwait(&rwlock->write_possible,
+                                &rwlock->mutex,
+                                abs_timeout);
+    rwlock->writers_waiting--;
+    if (rc != 0)
+      goto done;
+  }
+
+  /* Acquire the write lock. */
+  rwlock->writer_thread_id = pthread_self();
+done:
+  pthread_mutex_unlock(&rwlock->mutex);
+  return rc;
+}
+
+int pthread_rwlock_wrlock(pthread_rwlock_t *rwlock) {
+  struct timespec timeout = { 0, 0 };
+  return pthread_rwlock_timedwrlock(rwlock, &timeout);
+}
+
+int pthread_rwlock_trywrlock(pthread_rwlock_t *rwlock) {
+  int rc = 0;
+  pthread_mutex_lock(&rwlock->mutex);
+  /* Wait until there is no writer *and* no readers. */

[Mark Seaborn, 2014/12/01 22:33:07]

Nit: trywrlock() doesn't wait

[Sam Clegg, 2014/12/10 19:17:42]

Done.

+  if (!write_lock_available(rwlock)) {
+    rc = EBUSY;
+    goto done;
+  }
+
+  /* Acquire the write lock. */
+  rwlock->writer_thread_id = pthread_self();
+done:
+  pthread_mutex_unlock(&rwlock->mutex);
+  return rc;
+}
+
+int pthread_rwlock_unlock(pthread_rwlock_t *rwlock) {
+  int rc = 0;
+  pthread_mutex_lock(&rwlock->mutex);
+
+  if (rwlock->writer_thread_id != NACL_PTHREAD_ILLEGAL_THREAD_ID) {
+    /* The write-lock is held. Ensure its us the holds it. */

[Mark Seaborn, 2014/12/01 22:33:07]

"the" -> "that"

[Sam Clegg, 2014/12/10 19:17:42]

Done.

+    if (rwlock->writer_thread_id != pthread_self()) {
+      rc = EPERM;
+      goto done;
+    }
+
+    /* Release write lock. */
+    rwlock->writer_thread_id = NACL_PTHREAD_ILLEGAL_THREAD_ID;
+    if (rwlock->writers_waiting > 0) {
+      /* Wake a waiting writer if there is one. */
+      pthread_cond_signal(&rwlock->write_possible);
+    } else {
+      /* Otherwise wake a waiting reader. */
+      pthread_cond_signal(&rwlock->read_possible);
+    }
+  } else {
+    if (rwlock->reader_count == 0) {
+      rc = EPERM;
+      goto done;
+    }
+
+    /* Release read lock. */
+    rwlock->reader_count--;
+    if (rwlock->reader_count == 0 && rwlock->writers_waiting > 0) {
+      /* Wake a waiting writer. */
+      pthread_cond_signal(&rwlock->write_possible);
+    }
+  }
+
+done:
+  pthread_mutex_unlock(&rwlock->mutex);
+  return rc;
+}
+
+int pthread_rwlock_destroy(pthread_rwlock_t *rwlock) {

[Mark Seaborn, 2014/12/01 22:33:07]

Optional: You could check that reader_count and writers_waiting are both 0.

[Sam Clegg, 2014/12/10 19:17:43]

Done.

+  pthread_mutex_destroy(&rwlock->mutex);
+  pthread_cond_destroy(&rwlock->write_possible);
+  pthread_cond_destroy(&rwlock->read_possible);
+  return 0;
+}