Mojo: Implement passing of shared buffers across processes on POSIX.

mojo/system/raw_shared_buffer.cc

Index: mojo/system/raw_shared_buffer.cc
diff --git a/mojo/system/raw_shared_buffer.cc b/mojo/system/raw_shared_buffer.cc
index fb67cca71a77e8cfb3ddee8f13c7c3fbc50a45ec..9e579186cca0611e0d9c86506c38959b62823bd6 100644
--- a/mojo/system/raw_shared_buffer.cc
+++ b/mojo/system/raw_shared_buffer.cc
@@ -5,6 +5,7 @@
 #include "mojo/system/raw_shared_buffer.h"
 
 #include "base/logging.h"
+#include "mojo/embedder/platform_handle_utils.h"
 
 namespace mojo {
 namespace system {
@@ -15,8 +16,29 @@ RawSharedBuffer* RawSharedBuffer::Create(size_t num_bytes) {
 
   RawSharedBuffer* rv = new RawSharedBuffer(num_bytes);
   // No need to take the lock since we haven't given the object to anyone yet.
-  if (!rv->InitNoLock())
+  if (!rv->InitNoLock()) {
+    // We can't just delete it directly, due to the "in destructor" (debug)
+    // check.
+    scoped_refptr<RawSharedBuffer> deleter(rv);

[yzshen1, 2014/05/30 00:09:35]

With the raw and zero-ref-count |rv|, if something (e.g., in |InitNoLock()|)
AddRef() and then Release(), the object will be killed.

It seems safer to use scoped_refptr<...> as the type of |rv|, and change the
return type to scoped_refptr<...>.

(Besides, I feel a little bit uneasy if we return a raw ref-counted object
without increasing the ref count on behalf of the caller. Habit from COM and
Pepper. :))

[viettrungluu, 2014/05/30 00:23:42]

This is equivalent to the scoped_refptr<>.
That leads to annoying refcount churn. Until we make scoped_refptr<>s passable,
I don't want to do this.
Shrug.

     return NULL;
+  }
+
+  return rv;
+}
+
+RawSharedBuffer* RawSharedBuffer::CreateFromPlatformHandle(
+    size_t num_bytes,
+    embedder::ScopedPlatformHandle platform_handle) {
+  DCHECK_GT(num_bytes, 0u);
+
+  RawSharedBuffer* rv = new RawSharedBuffer(num_bytes);

[yzshen1, 2014/05/30 00:09:35]

ditto.

+  // No need to take the lock since we haven't given the object to anyone yet.
+  if (!rv->InitFromPlatformHandleNoLock(platform_handle.Pass())) {
+    // We can't just delete it directly, due to the "in destructor" (debug)
+    // check.
+    scoped_refptr<RawSharedBuffer> deleter(rv);
+    return NULL;
+  }
 
   return rv;
 }
@@ -49,6 +71,15 @@ scoped_ptr<RawSharedBufferMapping> RawSharedBuffer::MapNoCheck(size_t offset,
   return MapImplNoLock(offset, length);
 }
 
+embedder::ScopedPlatformHandle RawSharedBuffer::DuplicatePlatformHandle() {

[yzshen1, 2014/05/30 00:09:35]

Is it necessary to protect the access to handle_ for the two methods below?

[viettrungluu, 2014/05/30 00:23:42]

No and no.

In the case of DuplicatePlatformHandle(), nothing ever changes the handle (see
below), except on deletion (in which case you'd have much bigger problems).

PassPlatformHandle() is designed to be used (and only safe to use) when you can
guarantee that your thread has the only ref to it.

[yzshen1, 2014/05/30 00:27:48]

Sounds good. Maybe adding your reply as comments?

+  return embedder::DuplicatePlatformHandle(handle_.get());
+}
+
+embedder::ScopedPlatformHandle RawSharedBuffer::PassPlatformHandle() {
+  DCHECK(HasOneRef());
+  return handle_.Pass();
+}
+
 RawSharedBuffer::RawSharedBuffer(size_t num_bytes) : num_bytes_(num_bytes) {
 }