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

mojo/system/raw_shared_buffer.cc

 // Copyright 2014 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.
 
 #include "mojo/system/raw_shared_buffer.h"
 
 #include "base/logging.h"
+#include "mojo/embedder/platform_handle_utils.h"
 
 namespace mojo {
 namespace system {
 
 // static
 RawSharedBuffer* RawSharedBuffer::Create(size_t num_bytes) {
   DCHECK_GT(num_bytes, 0u);
 
   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;
+}
 
 scoped_ptr<RawSharedBufferMapping> RawSharedBuffer::Map(size_t offset,
                                                         size_t length) {
   if (!IsValidMap(offset, length))
     return scoped_ptr<RawSharedBufferMapping>();
 
   return MapNoCheck(offset, length);
 }
 
 bool RawSharedBuffer::IsValidMap(size_t offset, size_t length) {
   if (offset > num_bytes_ || length == 0)
     return false;
 
   // Note: This is an overflow-safe check of |offset + length > num_bytes_|
   // (that |num_bytes >= offset| is verified above).
   if (length > num_bytes_ - offset)
     return false;
 
   return true;
 }
 
 scoped_ptr<RawSharedBufferMapping> RawSharedBuffer::MapNoCheck(size_t offset,
                                                                size_t length) {
   DCHECK(IsValidMap(offset, length));
 
   base::AutoLock locker(lock_);
   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) {
 }
 
 RawSharedBuffer::~RawSharedBuffer() {
 }
 
 }  // namespace system
 }  // namespace mojo