ppapi_simple

native_client_sdk/src/libraries/utils/thread_safe_queue.h

Index: native_client_sdk/src/libraries/utils/thread_safe_queue.h
diff --git a/native_client_sdk/src/libraries/utils/thread_safe_queue.h b/native_client_sdk/src/libraries/utils/thread_safe_queue.h
new file mode 100644
index 0000000000000000000000000000000000000000..e9e1c7c5cdf8630f85410a52e2aa55ff6c3241a1
--- /dev/null
+++ b/native_client_sdk/src/libraries/utils/thread_safe_queue.h
@@ -0,0 +1,63 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef LIBRARIES_UTILS_THREAD_SAFE_QUEUE_H_
+#define LIBRARIES_UTILS_THREAD_SAFE_QUEUE_H_
+
+#include <pthread.h>
+
+#include <list>
+
+#include "utils/auto_lock.h"
+#include "utils/macros.h"
+
+
+// PointerQueue
+//
+// A simple template to support multithreaded and optionally blocking access
+// to a Queue of object pointers.   We use object pointer to keep the library
+// simple by avoiding issues of template traits.

[nfullagar1, 2013/05/28 18:28:53]

Comment above mentions object pointers, avoiding issues of template traits. 
Code below uses templates.  Suggest re-wording the comment.

[noelallen_use_chromium, 2013/05/30 00:25:07]

Done.

+//
+template<class T> class ThreadSafeQueue {
+ public:
+  ThreadSafeQueue() {
+    pthread_mutex_init(&mutex_, NULL);
+    pthread_cond_init(&cond_, NULL);
+  }
+
+  ~ThreadSafeQueue() {
+    pthread_mutex_destroy(&mutex_);
+    pthread_cond_destroy(&cond_);
+  }
+
+  void Enqueue(T* item) {
+    AutoLock lock(&mutex_);
+    list_.push_back(item);
+
+    pthread_cond_signal(&cond_);
+  }
+
+  T* Dequeue(bool block) {
+    AutoLock lock(&mutex_);
+
+    // If blocking enabled, wait until we get the queue is non-empty
+    if (block) {
+      while(list_.empty()) pthread_cond_wait(&cond_, &mutex_);

[Sam Clegg, 2013/05/29 04:48:25]

Space after while

[noelallen_use_chromium, 2013/05/30 00:25:07]

Done.

+    }
+
+    if (list_.empty()) return NULL;

[Sam Clegg, 2013/05/29 04:48:25]

Perhaps put in an else block?

[noelallen_use_chromium, 2013/05/30 00:25:07]

Early out is the convention.

On 2013/05/29 04:48:25, Sam Clegg wrote:

[Sam Clegg, 2013/05/30 01:16:13]

But isn't this check redundant if you went through the "if" above?  If 'block'
is set to true then the list is guaranteed to be non-empty on this line, right? 
 No need to check again here.

Another way to look at it is that if block is true we should never be able to
return NULL.  

+

[noelallen1, 2013/05/30 03:10:05]

Since there's no real performance benefit to the else, I thought this was more
readable.

+    T* item = list_.front();
+    list_.pop_front();
+    return item;
+  }
+
+ private:
+  std::list<T*> list_;
+  pthread_cond_t  cond_;
+  pthread_mutex_t mutex_;
+  DISALLOW_COPY_AND_ASSIGN(ThreadSafeQueue);
+};
+
+#endif  // LIBRARIES_UTILS_THREAD_SAFE_QUEUE_H_