diff options
author | viettrungluu@chromium.org <viettrungluu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-11-11 22:41:48 +0000 |
---|---|---|
committer | viettrungluu@chromium.org <viettrungluu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-11-11 22:41:48 +0000 |
commit | ef3797e4d14ce4b64cbbfdcf95843dc9bbdbb5b7 (patch) | |
tree | 243d458b098f7e06fdd00910a406116f5fb2db25 /mojo | |
parent | 5260b6683833c6d7c014346683b6bfc009f4ae72 (diff) | |
download | chromium_src-ef3797e4d14ce4b64cbbfdcf95843dc9bbdbb5b7.zip chromium_src-ef3797e4d14ce4b64cbbfdcf95843dc9bbdbb5b7.tar.gz chromium_src-ef3797e4d14ce4b64cbbfdcf95843dc9bbdbb5b7.tar.bz2 |
Mojo: Implement plumbing to support passing handles over MessagePipes.
This is tricky for several reasons:
- We have fine-grained locking (and would like to keep it that way),
and need to avoid deadlock. In particular, acquiring multiple
dispatcher locks simultaneously is dangerous -- so we allow it to
fail.
- We want clean failure semantics. In particular, on failure,
WriteMessage() should leave the handles being sent valid. This means
that we may not remove them from the handle table until the call has
succeeded. Thus we have to mark them as busy.
- We need to avoid various races. E.g., still to do: When sending a
handle in-process, it's important that once |WriteMessage()| has
sent a handle, no more calls done on that particular handle may
proceed. As a result, we won't be able to simply transfer
dispatchers to a new handle (in-process) but instead must create a
new dispatcher referencing the same resource. This will also ensure
that |Wait()|s on that handle will be properly cancelled.
R=darin@chromium.org, darin
Review URL: https://codereview.chromium.org/67413003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@234302 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'mojo')
-rw-r--r-- | mojo/public/system/core.h | 6 | ||||
-rw-r--r-- | mojo/system/core_impl.cc | 194 | ||||
-rw-r--r-- | mojo/system/core_impl.h | 38 | ||||
-rw-r--r-- | mojo/system/core_impl_unittest.cc | 140 | ||||
-rw-r--r-- | mojo/system/core_test_base.cc | 21 | ||||
-rw-r--r-- | mojo/system/dispatcher.cc | 47 | ||||
-rw-r--r-- | mojo/system/dispatcher.h | 50 | ||||
-rw-r--r-- | mojo/system/dispatcher_unittest.cc | 12 | ||||
-rw-r--r-- | mojo/system/message_pipe.cc | 14 | ||||
-rw-r--r-- | mojo/system/message_pipe.h | 12 | ||||
-rw-r--r-- | mojo/system/message_pipe_dispatcher.cc | 34 | ||||
-rw-r--r-- | mojo/system/message_pipe_dispatcher.h | 13 | ||||
-rw-r--r-- | mojo/system/message_pipe_dispatcher_unittest.cc | 65 | ||||
-rw-r--r-- | mojo/system/message_pipe_unittest.cc | 66 | ||||
-rw-r--r-- | mojo/system/remote_message_pipe_posix_unittest.cc | 28 |
15 files changed, 538 insertions, 202 deletions
diff --git a/mojo/public/system/core.h b/mojo/public/system/core.h index 88f2af5..2ef42af 100644 --- a/mojo/public/system/core.h +++ b/mojo/public/system/core.h @@ -89,6 +89,10 @@ const MojoHandle MOJO_HANDLE_INVALID = 0; // unavailable. The caller may simply retry the operation (possibly with // a backoff). // |MOJO_RESULT_DATA_LOSS| - Unrecoverable data loss or corruption. +// |MOJO_RESULT_BUSY| - One of the resources involved is currently being used +// (possibly on another thread) in a way that prevents the current +// operation from proceeding, e.g., if the other operation may result in +// the resource being invalidated. // // Note that positive values are also available as success codes. // @@ -111,6 +115,7 @@ const MojoResult MOJO_RESULT_UNIMPLEMENTED = -12; const MojoResult MOJO_RESULT_INTERNAL = -13; const MojoResult MOJO_RESULT_UNAVAILABLE = -14; const MojoResult MOJO_RESULT_DATA_LOSS = -15; +const MojoResult MOJO_RESULT_BUSY = -16; #else #define MOJO_RESULT_OK ((MojoResult) 0) #define MOJO_RESULT_CANCELLED ((MojoResult) -1) @@ -128,6 +133,7 @@ const MojoResult MOJO_RESULT_DATA_LOSS = -15; #define MOJO_RESULT_INTERNAL ((MojoResult) -13) #define MOJO_RESULT_UNAVAILABLE ((MojoResult) -14) #define MOJO_RESULT_DATA_LOSS ((MojoResult) -15) +#define MOJO_RESULT_BUSY ((MojoResult) -16) #endif // |MojoDeadline|: diff --git a/mojo/system/core_impl.cc b/mojo/system/core_impl.cc index 104a373..70a8e2b 100644 --- a/mojo/system/core_impl.cc +++ b/mojo/system/core_impl.cc @@ -66,6 +66,20 @@ namespace system { // - Locks at the "INF" level may not have any locks taken while they are // held. +CoreImpl::HandleTableEntry::HandleTableEntry() + : busy(false) { +} + +CoreImpl::HandleTableEntry::HandleTableEntry( + const scoped_refptr<Dispatcher>& dispatcher) + : dispatcher(dispatcher), + busy(false) { +} + +CoreImpl::HandleTableEntry::~HandleTableEntry() { + DCHECK(!busy); +} + // static CoreImpl* CoreImpl::singleton_ = NULL; @@ -85,7 +99,9 @@ MojoResult CoreImpl::Close(MojoHandle handle) { HandleTableMap::iterator it = handle_table_.find(handle); if (it == handle_table_.end()) return MOJO_RESULT_INVALID_ARGUMENT; - dispatcher = it->second; + if (it->second.busy) + return MOJO_RESULT_BUSY; + dispatcher = it->second.dispatcher; handle_table_.erase(it); } @@ -119,6 +135,11 @@ MojoResult CoreImpl::WaitMany(const MojoHandle* handles, MojoResult CoreImpl::CreateMessagePipe(MojoHandle* handle_0, MojoHandle* handle_1) { + if (!VerifyUserPointer<MojoHandle>(handle_0, 1)) + return MOJO_RESULT_INVALID_ARGUMENT; + if (!VerifyUserPointer<MojoHandle>(handle_1, 1)) + return MOJO_RESULT_INVALID_ARGUMENT; + scoped_refptr<MessagePipeDispatcher> dispatcher_0( new MessagePipeDispatcher()); scoped_refptr<MessagePipeDispatcher> dispatcher_1( @@ -157,9 +178,124 @@ MojoResult CoreImpl::WriteMessage( if (!dispatcher.get()) return MOJO_RESULT_INVALID_ARGUMENT; - return dispatcher->WriteMessage(bytes, num_bytes, - handles, num_handles, - flags); + // Easy case: not sending any handles. + if (num_handles == 0) + return dispatcher->WriteMessage(bytes, num_bytes, NULL, flags); + + // We have to handle |handles| here, since we have to mark them busy in the + // global handle table. We can't delegate this to the dispatcher, since the + // handle table lock must be acquired before the dispatcher lock. + // + // (This leads to an oddity: |handles|/|num_handles| are always verified for + // validity, even for dispatchers that don't support |WriteMessage()| and will + // simply return failure unconditionally. It also breaks the usual + // left-to-right verification order of arguments.) + if (!VerifyUserPointer<MojoHandle>(handles, num_handles)) + return MOJO_RESULT_INVALID_ARGUMENT; + if (num_handles > kMaxMessageNumHandles) + return MOJO_RESULT_RESOURCE_EXHAUSTED; + + // We'll need to hold on to the dispatchers so that we can pass them on to + // |WriteMessage()| and also so that we can unlock their locks afterwards + // without accessing the handle table. These can be dumb pointers, since their + // entries in the handle table won't get removed (since they'll be marked as + // busy). + std::vector<Dispatcher*> dispatchers(num_handles); + + // When we pass handles, we have to try to take all their dispatchers' locks + // and mark the handles as busy. If the call succeeds, we then remove the + // handles from the handle table. + { + base::AutoLock locker(handle_table_lock_); + + std::vector<HandleTableEntry*> entries(num_handles); + + // First verify all the handles and get their dispatchers. + uint32_t i; + MojoResult error_result = MOJO_RESULT_INTERNAL; + for (i = 0; i < num_handles; i++) { + // Sending your own handle is not allowed (and, for consistency, returns + // "busy"). + if (handles[i] == handle) { + error_result = MOJO_RESULT_BUSY; + break; + } + + HandleTableMap::iterator it = handle_table_.find(handles[i]); + if (it == handle_table_.end()) { + error_result = MOJO_RESULT_INVALID_ARGUMENT; + break; + } + + entries[i] = &it->second; + if (entries[i]->busy) { + error_result = MOJO_RESULT_BUSY; + break; + } + // Note: By marking the handle as busy here, we're also preventing the + // same handle from being sent multiple times in the same message. + entries[i]->busy = true; + + // Try to take the lock. + if (!entries[i]->dispatcher->lock().Try()) { + // Unset the busy flag (since it won't be unset below). + entries[i]->busy = false; + error_result = MOJO_RESULT_BUSY; + break; + } + + // Hang on to the pointer to the dispatcher (which we'll need to release + // the lock without going through the handle table). + dispatchers[i] = entries[i]->dispatcher; + } + if (i < num_handles) { + DCHECK_NE(error_result, MOJO_RESULT_INTERNAL); + + // Unset the busy flags and release the locks. + for (uint32_t j = 0; j < i; j++) { + DCHECK(entries[j]->busy); + entries[j]->busy = false; + entries[j]->dispatcher->lock().Release(); + } + return error_result; + } + } + + MojoResult rv = dispatcher->WriteMessage(bytes, num_bytes, + &dispatchers, + flags); + + // We need to release the dispatcher locks before we take the handle table + // lock. + for (uint32_t i = 0; i < num_handles; i++) { + dispatchers[i]->lock().AssertAcquired(); + dispatchers[i]->lock().Release(); + } + + if (rv == MOJO_RESULT_OK) { + base::AutoLock locker(handle_table_lock_); + + // Succeeded, so the handles should be removed from the handle table. (The + // transferring to new dispatchers/closing must have already been done.) + for (uint32_t i = 0; i < num_handles; i++) { + HandleTableMap::iterator it = handle_table_.find(handles[i]); + DCHECK(it != handle_table_.end()); + DCHECK(it->second.busy); + handle_table_.erase(it); + } + } else { + base::AutoLock locker(handle_table_lock_); + + // Failed, so the handles should go back to their normal state. + for (uint32_t i = 0; i < num_handles; i++) { + HandleTableMap::iterator it = handle_table_.find(handles[i]); + DCHECK(it != handle_table_.end()); + DCHECK(it->second.busy); + it->second.busy = false; + } + } + + return rv; } MojoResult CoreImpl::ReadMessage( @@ -171,9 +307,40 @@ MojoResult CoreImpl::ReadMessage( if (!dispatcher.get()) return MOJO_RESULT_INVALID_ARGUMENT; - return dispatcher->ReadMessage(bytes, num_bytes, - handles, num_handles, - flags); + uint32_t max_num_dispatchers = 0; + if (num_handles) { + if (!VerifyUserPointer<uint32_t>(num_handles, 1)) + return MOJO_RESULT_INVALID_ARGUMENT; + if (!VerifyUserPointer<MojoHandle>(handles, *num_handles)) + return MOJO_RESULT_INVALID_ARGUMENT; + max_num_dispatchers = *num_handles; + } + + // Easy case: won't receive any handles. + if (max_num_dispatchers == 0) + return dispatcher->ReadMessage(bytes, num_bytes, 0, NULL, flags); + + std::vector<scoped_refptr<Dispatcher> > dispatchers; + MojoResult rv = dispatcher->ReadMessage(bytes, num_bytes, + max_num_dispatchers, &dispatchers, + flags); + if (!dispatchers.empty()) { + DCHECK_EQ(rv, MOJO_RESULT_OK); + + *num_handles = static_cast<uint32_t>(dispatchers.size()); + DCHECK_LE(*num_handles, max_num_dispatchers); + + base::AutoLock locker(handle_table_lock_); + + for (size_t i = 0; i < dispatchers.size(); i++) { + // TODO(vtl): What should we do if we hit the maximum handle table size + // here? Currently, we'll just fill in those handles with + // |MOJO_HANDLE_INVALID| (and return success anyway). + handles[i] = AddDispatcherNoLock(dispatchers[i]); + } + } + + return rv; } CoreImpl::CoreImpl() @@ -194,10 +361,11 @@ scoped_refptr<Dispatcher> CoreImpl::GetDispatcher(MojoHandle handle) { if (it == handle_table_.end()) return NULL; - return it->second; + return it->second.dispatcher; } -MojoHandle CoreImpl::AddDispatcherNoLock(scoped_refptr<Dispatcher> dispatcher) { +MojoHandle CoreImpl::AddDispatcherNoLock( + const scoped_refptr<Dispatcher>& dispatcher) { DCHECK(dispatcher.get()); handle_table_lock_.AssertAcquired(); DCHECK_NE(next_handle_, MOJO_HANDLE_INVALID); @@ -214,7 +382,7 @@ MojoHandle CoreImpl::AddDispatcherNoLock(scoped_refptr<Dispatcher> dispatcher) { } MojoHandle new_handle = next_handle_; - handle_table_[new_handle] = dispatcher; + handle_table_[new_handle] = HandleTableEntry(dispatcher); next_handle_++; if (next_handle_ == MOJO_HANDLE_INVALID) @@ -236,10 +404,10 @@ MojoResult CoreImpl::WaitManyInternal(const MojoHandle* handles, std::vector<scoped_refptr<Dispatcher> > dispatchers; dispatchers.reserve(num_handles); for (uint32_t i = 0; i < num_handles; i++) { - scoped_refptr<Dispatcher> d = GetDispatcher(handles[i]); - if (!d.get()) + scoped_refptr<Dispatcher> dispatcher = GetDispatcher(handles[i]); + if (!dispatcher.get()) return MOJO_RESULT_INVALID_ARGUMENT; - dispatchers.push_back(d); + dispatchers.push_back(dispatcher); } // TODO(vtl): Should make the waiter live (permanently) in TLS. diff --git a/mojo/system/core_impl.h b/mojo/system/core_impl.h index aede29d..c05ac3b 100644 --- a/mojo/system/core_impl.h +++ b/mojo/system/core_impl.h @@ -59,8 +59,40 @@ class MOJO_SYSTEM_EXPORT CoreImpl { private: friend class test::CoreTestBase; - typedef base::hash_map<MojoHandle, scoped_refptr<Dispatcher> > - HandleTableMap; + // The |busy| member is used only to deal with functions (in particular + // |WriteMessage()|) that want to hold on to a dispatcher and later remove it + // from the handle table, without holding on to the handle table lock. + // + // For example, if |WriteMessage()| is called with a handle to be sent, (under + // the handle table lock) it must first check that that handle is not busy (if + // it is busy, then it fails with |MOJO_RESULT_BUSY|) and then marks it as + // busy. To avoid deadlock, it should also try to acquire the locks for all + // the dispatchers for the handles that it is sending (and fail with + // |MOJO_RESULT_BUSY| if the attempt fails). At this point, it can release the + // handle table lock. + // + // If |Close()| is simultaneously called on that handle, it too checks if the + // handle is marked busy. If it is, it fails (with |MOJO_RESULT_BUSY|). This + // prevents |WriteMessage()| from sending a handle that has been closed (or + // learning about this too late). + // + // TODO(vtl): Move this implementation note. + // To properly cancel waiters and avoid other races, |WriteMessage()| does not + // transfer dispatchers from one handle to another, even when sending a + // message in-process. Instead, it must transfer the "contents" of the + // dispatcher to a new dispatcher, and then close the old dispatcher. If this + // isn't done, in the in-process case, calls on the old handle may complete + // after the the message has been received and a new handle created (and + // possibly even after calls have been made on the new handle). + struct HandleTableEntry { + HandleTableEntry(); + explicit HandleTableEntry(const scoped_refptr<Dispatcher>& dispatcher); + ~HandleTableEntry(); + + scoped_refptr<Dispatcher> dispatcher; + bool busy; + }; + typedef base::hash_map<MojoHandle, HandleTableEntry> HandleTableMap; CoreImpl(); ~CoreImpl(); @@ -72,7 +104,7 @@ class MOJO_SYSTEM_EXPORT CoreImpl { // Assigns a new handle for the given dispatcher (which must be valid); // returns |MOJO_HANDLE_INVALID| on failure (due to hitting resource limits). // Must be called under |handle_table_lock_|. - MojoHandle AddDispatcherNoLock(scoped_refptr<Dispatcher> dispatcher); + MojoHandle AddDispatcherNoLock(const scoped_refptr<Dispatcher>& dispatcher); // Internal implementation of |Wait()| and |WaitMany()|; doesn't do basic // validation of arguments. diff --git a/mojo/system/core_impl_unittest.cc b/mojo/system/core_impl_unittest.cc index 1ace04d..bfa2bfa 100644 --- a/mojo/system/core_impl_unittest.cc +++ b/mojo/system/core_impl_unittest.cc @@ -4,6 +4,8 @@ #include "mojo/system/core_impl.h" +#include <limits> + #include "mojo/system/core_test_base.h" namespace mojo { @@ -42,6 +44,10 @@ TEST_F(CoreImplTest, Basic) { core()->ReadMessage(h, NULL, &num_bytes, NULL, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); EXPECT_EQ(2u, info.GetReadMessageCallCount()); + EXPECT_EQ(MOJO_RESULT_OK, + core()->ReadMessage(h, NULL, NULL, NULL, NULL, + MOJO_READ_MESSAGE_FLAG_NONE)); + EXPECT_EQ(3u, info.GetReadMessageCallCount()); EXPECT_EQ(0u, info.GetAddWaiterCallCount()); EXPECT_EQ(MOJO_RESULT_FAILED_PRECONDITION, @@ -133,6 +139,140 @@ TEST_F(CoreImplTest, InvalidArguments) { EXPECT_EQ(MOJO_RESULT_OK, core()->Close(handles[0])); EXPECT_EQ(MOJO_RESULT_OK, core()->Close(handles[1])); } + + // |CreateMessagePipe()|: + { + MojoHandle h; + EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, + core()->CreateMessagePipe(NULL, NULL)); + EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, + core()->CreateMessagePipe(&h, NULL)); + EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, + core()->CreateMessagePipe(NULL, &h)); + } + + // |WriteMessage()|: + // Only check arguments checked by |CoreImpl|, namely |handle|, |handles|, and + // |num_handles|. + { + EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, + core()->WriteMessage(MOJO_HANDLE_INVALID, NULL, 0, NULL, 0, + MOJO_WRITE_MESSAGE_FLAG_NONE)); + + MockHandleInfo info; + MojoHandle h = CreateMockHandle(&info); + MojoHandle handles[2] = { MOJO_HANDLE_INVALID, MOJO_HANDLE_INVALID }; + + // Null |handles| with nonzero |num_handles|. + EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, + core()->WriteMessage(h, NULL, 0, NULL, 1, + MOJO_WRITE_MESSAGE_FLAG_NONE)); + // Checked by |CoreImpl|, shouldn't go through to the dispatcher. + EXPECT_EQ(0u, info.GetWriteMessageCallCount()); + + // Huge handle count (implausibly big on some systems -- more than can be + // stored in a 32-bit address space). + // Note: This may return either |MOJO_RESULT_INVALID_ARGUMENT| or + // |MOJO_RESULT_RESOURCE_EXHAUSTED|, depending on whether it's plausible or + // not. + EXPECT_NE(MOJO_RESULT_OK, + core()->WriteMessage(h, NULL, 0, handles, + std::numeric_limits<uint32_t>::max(), + MOJO_WRITE_MESSAGE_FLAG_NONE)); + EXPECT_EQ(0u, info.GetWriteMessageCallCount()); + + // Huge handle count (plausibly big). + EXPECT_EQ(MOJO_RESULT_RESOURCE_EXHAUSTED, + core()->WriteMessage(h, NULL, 0, handles, + std::numeric_limits<uint32_t>::max() / + sizeof(handles[0]), + MOJO_WRITE_MESSAGE_FLAG_NONE)); + EXPECT_EQ(0u, info.GetWriteMessageCallCount()); + + // Invalid handle in |handles|. + EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, + core()->WriteMessage(h, NULL, 0, handles, 1, + MOJO_WRITE_MESSAGE_FLAG_NONE)); + EXPECT_EQ(0u, info.GetWriteMessageCallCount()); + + // Two invalid handles in |handles|. + EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, + core()->WriteMessage(h, NULL, 0, handles, 2, + MOJO_WRITE_MESSAGE_FLAG_NONE)); + EXPECT_EQ(0u, info.GetWriteMessageCallCount()); + + // Can't send a handle over itself. + handles[0] = h; + EXPECT_EQ(MOJO_RESULT_BUSY, + core()->WriteMessage(h, NULL, 0, handles, 1, + MOJO_WRITE_MESSAGE_FLAG_NONE)); + EXPECT_EQ(0u, info.GetWriteMessageCallCount()); + + MockHandleInfo info_2; + MojoHandle h_2 = CreateMockHandle(&info_2); + + // This is "okay", but |MockDispatcher| doesn't implement it. + handles[0] = h_2; + EXPECT_EQ(MOJO_RESULT_UNIMPLEMENTED, + core()->WriteMessage(h, NULL, 0, handles, 1, + MOJO_WRITE_MESSAGE_FLAG_NONE)); + EXPECT_EQ(1u, info.GetWriteMessageCallCount()); + + // One of the |handles| is still invalid. + EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, + core()->WriteMessage(h, NULL, 0, handles, 2, + MOJO_WRITE_MESSAGE_FLAG_NONE)); + EXPECT_EQ(1u, info.GetWriteMessageCallCount()); + + // One of the |handles| is the same as |handle|. + handles[1] = h; + EXPECT_EQ(MOJO_RESULT_BUSY, + core()->WriteMessage(h, NULL, 0, handles, 2, + MOJO_WRITE_MESSAGE_FLAG_NONE)); + EXPECT_EQ(1u, info.GetWriteMessageCallCount()); + + // Can't send a handle twice in the same message. + handles[1] = h_2; + EXPECT_EQ(MOJO_RESULT_BUSY, + core()->WriteMessage(h, NULL, 0, handles, 2, + MOJO_WRITE_MESSAGE_FLAG_NONE)); + EXPECT_EQ(1u, info.GetWriteMessageCallCount()); + + // Note: Since we never successfully sent anything with it, |h_2| should + // still be valid. + EXPECT_EQ(MOJO_RESULT_OK, core()->Close(h_2)); + + EXPECT_EQ(MOJO_RESULT_OK, core()->Close(h)); + } + + // |ReadMessage()|: + // Only check arguments checked by |CoreImpl|, namely |handle|, |handles|, and + // |num_handles|. + { + EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, + core()->ReadMessage(MOJO_HANDLE_INVALID, NULL, NULL, NULL, NULL, + MOJO_READ_MESSAGE_FLAG_NONE)); + + MockHandleInfo info; + MojoHandle h = CreateMockHandle(&info); + + uint32_t handle_count = 1; + EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, + core()->ReadMessage(h, NULL, NULL, NULL, &handle_count, + MOJO_READ_MESSAGE_FLAG_NONE)); + // Checked by |CoreImpl|, shouldn't go through to the dispatcher. + EXPECT_EQ(0u, info.GetReadMessageCallCount()); + + // Okay. + handle_count = 0; + EXPECT_EQ(MOJO_RESULT_OK, + core()->ReadMessage(h, NULL, NULL, NULL, &handle_count, + MOJO_READ_MESSAGE_FLAG_NONE)); + // Checked by |CoreImpl|, shouldn't go through to the dispatcher. + EXPECT_EQ(1u, info.GetReadMessageCallCount()); + + EXPECT_EQ(MOJO_RESULT_OK, core()->Close(h)); + } } // TODO(vtl): test |Wait()| and |WaitMany()| properly diff --git a/mojo/system/core_test_base.cc b/mojo/system/core_test_base.cc index 93b11de..15b9f02 100644 --- a/mojo/system/core_test_base.cc +++ b/mojo/system/core_test_base.cc @@ -11,6 +11,7 @@ #include "base/memory/ref_counted.h" #include "mojo/system/core_impl.h" #include "mojo/system/dispatcher.h" +#include "mojo/system/limits.h" #include "mojo/system/memory.h" namespace mojo { @@ -45,34 +46,32 @@ class MockDispatcher : public Dispatcher { virtual MojoResult WriteMessageImplNoLock( const void* bytes, uint32_t num_bytes, - const MojoHandle* handles, - uint32_t num_handles, + const std::vector<Dispatcher*>* dispatchers, MojoWriteMessageFlags /*flags*/) OVERRIDE { info_->IncrementWriteMessageCallCount(); lock().AssertAcquired(); if (!VerifyUserPointer<void>(bytes, num_bytes)) return MOJO_RESULT_INVALID_ARGUMENT; - if (!VerifyUserPointer<MojoHandle>(handles, num_handles)) - return MOJO_RESULT_INVALID_ARGUMENT; + if (num_bytes > kMaxMessageNumBytes) + return MOJO_RESULT_RESOURCE_EXHAUSTED; + + if (dispatchers) + return MOJO_RESULT_UNIMPLEMENTED; return MOJO_RESULT_OK; } virtual MojoResult ReadMessageImplNoLock( - void* bytes, - uint32_t* num_bytes, - MojoHandle* handles, - uint32_t* num_handles, + void* bytes, uint32_t* num_bytes, + uint32_t /*max_num_dispatchers*/, + std::vector<scoped_refptr<Dispatcher> >* /*dispatchers*/, MojoReadMessageFlags /*flags*/) OVERRIDE { info_->IncrementReadMessageCallCount(); lock().AssertAcquired(); if (num_bytes && !VerifyUserPointer<void>(bytes, *num_bytes)) return MOJO_RESULT_INVALID_ARGUMENT; - if (num_handles && - !VerifyUserPointer<MojoHandle>(handles, *num_handles)) - return MOJO_RESULT_INVALID_ARGUMENT; return MOJO_RESULT_OK; } diff --git a/mojo/system/dispatcher.cc b/mojo/system/dispatcher.cc index 04f3550..bb3327f 100644 --- a/mojo/system/dispatcher.cc +++ b/mojo/system/dispatcher.cc @@ -5,6 +5,7 @@ #include "mojo/system/dispatcher.h" #include "base/logging.h" +#include "mojo/system/limits.h" namespace mojo { namespace system { @@ -19,28 +20,33 @@ MojoResult Dispatcher::Close() { return CloseImplNoLock(); } -MojoResult Dispatcher::WriteMessage(const void* bytes, - uint32_t num_bytes, - const MojoHandle* handles, - uint32_t num_handles, +MojoResult Dispatcher::WriteMessage(const void* bytes, uint32_t num_bytes, + const std::vector<Dispatcher*>* dispatchers, MojoWriteMessageFlags flags) { + DCHECK(!dispatchers || (dispatchers->size() > 0 && + dispatchers->size() < kMaxMessageNumHandles)); + base::AutoLock locker(lock_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; - return WriteMessageImplNoLock(bytes, num_bytes, handles, num_handles, flags); + return WriteMessageImplNoLock(bytes, num_bytes, dispatchers, flags); } -MojoResult Dispatcher::ReadMessage(void* bytes, - uint32_t* num_bytes, - MojoHandle* handles, - uint32_t* num_handles, - MojoReadMessageFlags flags) { +MojoResult Dispatcher::ReadMessage( + void* bytes, uint32_t* num_bytes, + uint32_t max_num_dispatchers, + std::vector<scoped_refptr<Dispatcher> >* dispatchers, + MojoReadMessageFlags flags) { + DCHECK(max_num_dispatchers == 0 || !!dispatchers); + base::AutoLock locker(lock_); if (is_closed_) return MOJO_RESULT_INVALID_ARGUMENT; - return ReadMessageImplNoLock(bytes, num_bytes, handles, num_handles, flags); + return ReadMessageImplNoLock(bytes, num_bytes, + max_num_dispatchers, dispatchers, + flags); } MojoResult Dispatcher::AddWaiter(Waiter* waiter, @@ -86,11 +92,10 @@ MojoResult Dispatcher::CloseImplNoLock() { return MOJO_RESULT_OK; } -MojoResult Dispatcher::WriteMessageImplNoLock(const void* bytes, - uint32_t num_bytes, - const MojoHandle* handles, - uint32_t num_handles, - MojoWriteMessageFlags flags) { +MojoResult Dispatcher::WriteMessageImplNoLock( + const void* bytes, uint32_t num_bytes, + const std::vector<Dispatcher*>* dispatchers, + MojoWriteMessageFlags flags) { lock_.AssertAcquired(); DCHECK(!is_closed_); // By default, this isn't supported. Only dispatchers for message pipes (with @@ -98,11 +103,11 @@ MojoResult Dispatcher::WriteMessageImplNoLock(const void* bytes, return MOJO_RESULT_INVALID_ARGUMENT; } -MojoResult Dispatcher::ReadMessageImplNoLock(void* bytes, - uint32_t* num_bytes, - MojoHandle* handles, - uint32_t* num_handles, - MojoReadMessageFlags flags) { +MojoResult Dispatcher::ReadMessageImplNoLock( + void* bytes, uint32_t* num_bytes, + uint32_t max_num_dispatchers, + std::vector<scoped_refptr<Dispatcher> >* dispatchers, + MojoReadMessageFlags flags) { lock_.AssertAcquired(); DCHECK(!is_closed_); // By default, this isn't supported. Only dispatchers for message pipes (with diff --git a/mojo/system/dispatcher.h b/mojo/system/dispatcher.h index 75f6a97..d1721d6 100644 --- a/mojo/system/dispatcher.h +++ b/mojo/system/dispatcher.h @@ -5,6 +5,8 @@ #ifndef MOJO_SYSTEM_DISPATCHER_H_ #define MOJO_SYSTEM_DISPATCHER_H_ +#include <vector> + #include "base/basictypes.h" #include "base/memory/ref_counted.h" #include "base/synchronization/lock.h" @@ -14,6 +16,7 @@ namespace mojo { namespace system { +class CoreImpl; class Waiter; // A |Dispatcher| implements Mojo primitives that are "attached" to a particular @@ -32,16 +35,21 @@ class MOJO_SYSTEM_EXPORT Dispatcher : // prevents the various |...ImplNoLock()|s from releasing the lock as soon as // possible. If this becomes an issue, we can rethink this. MojoResult Close(); - MojoResult WriteMessage(const void* bytes, - uint32_t num_bytes, - const MojoHandle* handles, - uint32_t num_handles, + // |dispatchers| may be non-null if and only if there are handles to be + // written, in which case this will be called with all the dispatchers' locks + // held. On success, all the dispatchers must have been moved to a closed + // state; on failure, they should remain in their original state. + MojoResult WriteMessage(const void* bytes, uint32_t num_bytes, + const std::vector<Dispatcher*>* dispatchers, MojoWriteMessageFlags flags); - MojoResult ReadMessage(void* bytes, - uint32_t* num_bytes, - MojoHandle* handles, - uint32_t* num_handles, - MojoReadMessageFlags flags); + // |dispatchers| must be non-null if |max_num_dispatchers| is nonzero. On + // success, it will be set to the dispatchers to be received (and assigned + // handles) as part of the message. + MojoResult ReadMessage( + void* bytes, uint32_t* num_bytes, + uint32_t max_num_dispatchers, + std::vector<scoped_refptr<Dispatcher> >* dispatchers, + MojoReadMessageFlags flags); // Adds a waiter to this dispatcher. The waiter will be woken up when this // object changes state to satisfy |flags| with result |wake_result| (which @@ -74,16 +82,17 @@ class MOJO_SYSTEM_EXPORT Dispatcher : // These are to be overridden by subclasses (if necessary). They are never // called after the dispatcher has been closed. They are called under |lock_|. - virtual MojoResult WriteMessageImplNoLock(const void* bytes, - uint32_t num_bytes, - const MojoHandle* handles, - uint32_t num_handles, - MojoWriteMessageFlags flags); - virtual MojoResult ReadMessageImplNoLock(void* bytes, - uint32_t* num_bytes, - MojoHandle* handles, - uint32_t* num_handles, - MojoReadMessageFlags flags); + // See the descriptions of the methods without the "ImplNoLock" for more + // information. + virtual MojoResult WriteMessageImplNoLock( + const void* bytes, uint32_t num_bytes, + const std::vector<Dispatcher*>* dispatchers, + MojoWriteMessageFlags flags); + virtual MojoResult ReadMessageImplNoLock( + void* bytes, uint32_t* num_bytes, + uint32_t max_num_dispatchers, + std::vector<scoped_refptr<Dispatcher> >* dispatchers, + MojoReadMessageFlags flags); virtual MojoResult AddWaiterImplNoLock(Waiter* waiter, MojoWaitFlags flags, MojoResult wake_result); @@ -94,6 +103,9 @@ class MOJO_SYSTEM_EXPORT Dispatcher : base::Lock& lock() const { return lock_; } private: + // For |WriteMessage()|, |CoreImpl| needs access to |lock()|. + friend class CoreImpl; + // This protects the following members as well as any state added by // subclasses. mutable base::Lock lock_; diff --git a/mojo/system/dispatcher_unittest.cc b/mojo/system/dispatcher_unittest.cc index a479fce..46bb3298 100644 --- a/mojo/system/dispatcher_unittest.cc +++ b/mojo/system/dispatcher_unittest.cc @@ -32,9 +32,9 @@ TEST(DispatcherTest, Basic) { scoped_refptr<Dispatcher> d(new TrivialDispatcher()); EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, - d->WriteMessage(NULL, 0, NULL, 0, MOJO_WRITE_MESSAGE_FLAG_NONE)); + d->WriteMessage(NULL, 0, NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, - d->ReadMessage(NULL, NULL, NULL, NULL, + d->ReadMessage(NULL, NULL, 0, NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); Waiter w; w.Init(); @@ -47,9 +47,9 @@ TEST(DispatcherTest, Basic) { EXPECT_EQ(MOJO_RESULT_OK, d->Close()); EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, - d->WriteMessage(NULL, 0, NULL, 0, MOJO_WRITE_MESSAGE_FLAG_NONE)); + d->WriteMessage(NULL, 0, NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, - d->ReadMessage(NULL, NULL, NULL, NULL, + d->ReadMessage(NULL, NULL, 0, NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, d->AddWaiter(&w, MOJO_WAIT_FLAG_EVERYTHING, 0)); @@ -97,12 +97,12 @@ class ThreadSafetyStressThread : public base::SimpleThread { } case WRITE_MESSAGE: EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, - dispatcher_->WriteMessage(NULL, 0, NULL, 0, + dispatcher_->WriteMessage(NULL, 0, NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); break; case READ_MESSAGE: EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, - dispatcher_->ReadMessage(NULL, NULL, NULL, NULL, + dispatcher_->ReadMessage(NULL, NULL, 0, NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); break; case ADD_WAITER: { diff --git a/mojo/system/message_pipe.cc b/mojo/system/message_pipe.cc index 857cdbc..7f95a90 100644 --- a/mojo/system/message_pipe.cc +++ b/mojo/system/message_pipe.cc @@ -64,7 +64,7 @@ void MessagePipe::Close(unsigned port) { MojoResult MessagePipe::WriteMessage( unsigned port, const void* bytes, uint32_t num_bytes, - const MojoHandle* /*handles*/, uint32_t /*num_handles*/, + const std::vector<Dispatcher*>* /*dispatchers*/, MojoWriteMessageFlags flags) { DCHECK(port == 0 || port == 1); return EnqueueMessage( @@ -75,17 +75,19 @@ MojoResult MessagePipe::WriteMessage( bytes, num_bytes)); } -MojoResult MessagePipe::ReadMessage(unsigned port, - void* bytes, uint32_t* num_bytes, - MojoHandle* handles, uint32_t* num_handles, - MojoReadMessageFlags flags) { +MojoResult MessagePipe::ReadMessage( + unsigned port, + void* bytes, uint32_t* num_bytes, + uint32_t max_num_dispatchers, + std::vector<scoped_refptr<Dispatcher> >* dispatchers, + MojoReadMessageFlags flags) { DCHECK(port == 0 || port == 1); base::AutoLock locker(lock_); DCHECK(endpoints_[port].get()); return endpoints_[port]->ReadMessage(bytes, num_bytes, - handles, num_handles, + NULL, NULL, flags); } diff --git a/mojo/system/message_pipe.h b/mojo/system/message_pipe.h index 67f99d2..fc5ec63 100644 --- a/mojo/system/message_pipe.h +++ b/mojo/system/message_pipe.h @@ -5,6 +5,8 @@ #ifndef MOJO_SYSTEM_MESSAGE_PIPE_H_ #define MOJO_SYSTEM_MESSAGE_PIPE_H_ +#include <vector> + #include "base/basictypes.h" #include "base/callback.h" #include "base/memory/ref_counted.h" @@ -18,6 +20,7 @@ namespace mojo { namespace system { class Channel; +class Dispatcher; class MessagePipeEndpoint; class Waiter; @@ -43,16 +46,17 @@ class MOJO_SYSTEM_EXPORT MessagePipe : void CancelAllWaiters(unsigned port); void Close(unsigned port); // Unlike |MessagePipeDispatcher::WriteMessage()|, this does not validate its - // arguments. |bytes|/|num_bytes| and |handles|/|num_handles| must be valid. + // arguments. MojoResult WriteMessage(unsigned port, const void* bytes, uint32_t num_bytes, - const MojoHandle* handles, uint32_t num_handles, + const std::vector<Dispatcher*>* dispatchers, MojoWriteMessageFlags flags); // Unlike |MessagePipeDispatcher::ReadMessage()|, this does not validate its - // arguments. |bytes|/|num_bytes| and |handles|/|num_handles| must be valid. + // arguments. MojoResult ReadMessage(unsigned port, void* bytes, uint32_t* num_bytes, - MojoHandle* handles, uint32_t* num_handles, + uint32_t max_num_dispatchers, + std::vector<scoped_refptr<Dispatcher> >* dispatchers, MojoReadMessageFlags flags); MojoResult AddWaiter(unsigned port, Waiter* waiter, diff --git a/mojo/system/message_pipe_dispatcher.cc b/mojo/system/message_pipe_dispatcher.cc index ab84def..df4def1 100644 --- a/mojo/system/message_pipe_dispatcher.cc +++ b/mojo/system/message_pipe_dispatcher.cc @@ -43,7 +43,7 @@ MojoResult MessagePipeDispatcher::CloseImplNoLock() { MojoResult MessagePipeDispatcher::WriteMessageImplNoLock( const void* bytes, uint32_t num_bytes, - const MojoHandle* handles, uint32_t num_handles, + const std::vector<Dispatcher*>* dispatchers, MojoWriteMessageFlags flags) { lock().AssertAcquired(); @@ -52,40 +52,38 @@ MojoResult MessagePipeDispatcher::WriteMessageImplNoLock( if (num_bytes > kMaxMessageNumBytes) return MOJO_RESULT_RESOURCE_EXHAUSTED; - if (!VerifyUserPointer<MojoHandle>(handles, num_handles)) - return MOJO_RESULT_INVALID_ARGUMENT; - if (num_handles > kMaxMessageNumHandles) - return MOJO_RESULT_RESOURCE_EXHAUSTED; - if (num_handles > 0) { - // TODO(vtl): Verify each handle. + if (dispatchers) { + DCHECK_GT(dispatchers->size(), 0u); + DCHECK_LE(dispatchers->size(), kMaxMessageNumHandles); + + // TODO(vtl) NOTIMPLEMENTED(); return MOJO_RESULT_UNIMPLEMENTED; } return message_pipe_->WriteMessage(port_, bytes, num_bytes, - handles, num_handles, + dispatchers, flags); } MojoResult MessagePipeDispatcher::ReadMessageImplNoLock( void* bytes, uint32_t* num_bytes, - MojoHandle* handles, uint32_t* num_handles, + uint32_t max_num_dispatchers, + std::vector<scoped_refptr<Dispatcher> >* dispatchers, MojoReadMessageFlags flags) { lock().AssertAcquired(); - // TODO(vtl): I suppose we should verify |num_bytes| and |num_handles| (i.e., - // those pointers themselves). Hmmm. - - if (num_bytes && !VerifyUserPointer<void>(bytes, *num_bytes)) - return MOJO_RESULT_INVALID_ARGUMENT; - - if (num_handles && !VerifyUserPointer<MojoHandle>(handles, *num_handles)) - return MOJO_RESULT_INVALID_ARGUMENT; + if (num_bytes) { + if (!VerifyUserPointer<uint32_t>(num_bytes, 1)) + return MOJO_RESULT_INVALID_ARGUMENT; + if (!VerifyUserPointer<void>(bytes, *num_bytes)) + return MOJO_RESULT_INVALID_ARGUMENT; + } return message_pipe_->ReadMessage(port_, bytes, num_bytes, - handles, num_handles, + max_num_dispatchers, dispatchers, flags); } diff --git a/mojo/system/message_pipe_dispatcher.h b/mojo/system/message_pipe_dispatcher.h index a0e2f09..8fda16a 100644 --- a/mojo/system/message_pipe_dispatcher.h +++ b/mojo/system/message_pipe_dispatcher.h @@ -33,16 +33,13 @@ class MOJO_SYSTEM_EXPORT MessagePipeDispatcher : public Dispatcher { virtual void CancelAllWaitersNoLock() OVERRIDE; virtual MojoResult CloseImplNoLock() OVERRIDE; virtual MojoResult WriteMessageImplNoLock( - const void* bytes, - uint32_t num_bytes, - const MojoHandle* handles, - uint32_t num_handles, + const void* bytes, uint32_t num_bytes, + const std::vector<Dispatcher*>* dispatchers, MojoWriteMessageFlags flags) OVERRIDE; virtual MojoResult ReadMessageImplNoLock( - void* bytes, - uint32_t* num_bytes, - MojoHandle* handles, - uint32_t* num_handles, + void* bytes, uint32_t* num_bytes, + uint32_t max_num_dispatchers, + std::vector<scoped_refptr<Dispatcher> >* dispatchers, MojoReadMessageFlags flags) OVERRIDE; virtual MojoResult AddWaiterImplNoLock(Waiter* waiter, MojoWaitFlags flags, diff --git a/mojo/system/message_pipe_dispatcher_unittest.cc b/mojo/system/message_pipe_dispatcher_unittest.cc index 32c978b..b4a4107 100644 --- a/mojo/system/message_pipe_dispatcher_unittest.cc +++ b/mojo/system/message_pipe_dispatcher_unittest.cc @@ -63,7 +63,7 @@ TEST(MessagePipeDispatcherTest, Basic) { buffer[0] = 123456789; EXPECT_EQ(MOJO_RESULT_OK, d_1->WriteMessage(buffer, kBufferSize, - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); stopwatch.Start(); EXPECT_EQ(1, w.Wait(MOJO_DEADLINE_INDEFINITE)); @@ -82,7 +82,7 @@ TEST(MessagePipeDispatcherTest, Basic) { buffer_size = kBufferSize; EXPECT_EQ(MOJO_RESULT_OK, d_0->ReadMessage(buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); EXPECT_EQ(kBufferSize, buffer_size); EXPECT_EQ(123456789, buffer[0]); @@ -115,7 +115,6 @@ TEST(MessagePipeDispatcherTest, Basic) { TEST(MessagePipeDispatcherTest, InvalidParams) { char buffer[1]; - MojoHandle handles[1]; scoped_refptr<MessagePipeDispatcher> d_0(new MessagePipeDispatcher()); scoped_refptr<MessagePipeDispatcher> d_1(new MessagePipeDispatcher()); @@ -129,33 +128,12 @@ TEST(MessagePipeDispatcherTest, InvalidParams) { // Null buffer with nonzero buffer size. EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, d_0->WriteMessage(NULL, 1, - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); // Huge buffer size. EXPECT_EQ(MOJO_RESULT_RESOURCE_EXHAUSTED, d_0->WriteMessage(buffer, std::numeric_limits<uint32_t>::max(), - NULL, 0, - MOJO_WRITE_MESSAGE_FLAG_NONE)); - - // Null handles with nonzero handle count. - EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, - d_0->WriteMessage(buffer, sizeof(buffer), - NULL, 1, - MOJO_WRITE_MESSAGE_FLAG_NONE)); - // Huge handle count (implausibly big on some systems -- more than can be - // stored in a 32-bit address space). - // Note: This may return either |MOJO_RESULT_INVALID_ARGUMENT| or - // |MOJO_RESULT_RESOURCE_EXHAUSTED|, depending on whether it's plausible or - // not. - EXPECT_NE(MOJO_RESULT_OK, - d_0->WriteMessage(buffer, sizeof(buffer), - handles, std::numeric_limits<uint32_t>::max(), - MOJO_WRITE_MESSAGE_FLAG_NONE)); - // Huge handle count (plausibly big). - EXPECT_EQ(MOJO_RESULT_RESOURCE_EXHAUSTED, - d_0->WriteMessage(buffer, sizeof(buffer), - handles, std::numeric_limits<uint32_t>::max() / - sizeof(handles[0]), + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); // |ReadMessage|: @@ -163,14 +141,7 @@ TEST(MessagePipeDispatcherTest, InvalidParams) { uint32_t buffer_size = 1; EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, d_0->ReadMessage(NULL, &buffer_size, - NULL, NULL, - MOJO_READ_MESSAGE_FLAG_NONE)); - // Null handles with nonzero handle count. - buffer_size = static_cast<uint32_t>(sizeof(buffer)); - uint32_t handle_count = 1; - EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, - d_0->ReadMessage(buffer, &buffer_size, - NULL, &handle_count, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); EXPECT_EQ(MOJO_RESULT_OK, d_0->Close()); @@ -198,12 +169,12 @@ TEST(MessagePipeDispatcherTest, BasicClosed) { buffer[0] = 123456789; EXPECT_EQ(MOJO_RESULT_OK, d_1->WriteMessage(buffer, kBufferSize, - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); buffer[0] = 234567890; EXPECT_EQ(MOJO_RESULT_OK, d_1->WriteMessage(buffer, kBufferSize, - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); // Try waiting for readable on |d_0|; should fail (already satisfied). @@ -216,7 +187,7 @@ TEST(MessagePipeDispatcherTest, BasicClosed) { buffer_size = kBufferSize; EXPECT_EQ(MOJO_RESULT_NOT_FOUND, d_1->ReadMessage(buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); // Close |d_1|. @@ -232,7 +203,7 @@ TEST(MessagePipeDispatcherTest, BasicClosed) { buffer_size = kBufferSize; EXPECT_EQ(MOJO_RESULT_OK, d_0->ReadMessage(buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); EXPECT_EQ(kBufferSize, buffer_size); EXPECT_EQ(123456789, buffer[0]); @@ -247,7 +218,7 @@ TEST(MessagePipeDispatcherTest, BasicClosed) { buffer_size = kBufferSize; EXPECT_EQ(MOJO_RESULT_OK, d_0->ReadMessage(buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); EXPECT_EQ(kBufferSize, buffer_size); EXPECT_EQ(234567890, buffer[0]); @@ -268,14 +239,14 @@ TEST(MessagePipeDispatcherTest, BasicClosed) { buffer_size = kBufferSize; EXPECT_EQ(MOJO_RESULT_FAILED_PRECONDITION, d_0->ReadMessage(buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); // Try writing to |d_0|; should fail (other end closed). buffer[0] = 345678901; EXPECT_EQ(MOJO_RESULT_FAILED_PRECONDITION, d_0->WriteMessage(buffer, kBufferSize, - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); EXPECT_EQ(MOJO_RESULT_OK, d_0->Close()); @@ -316,7 +287,7 @@ TEST(MessagePipeDispatcherTest, BasicThreaded) { buffer[0] = 123456789; EXPECT_EQ(MOJO_RESULT_OK, d_0->WriteMessage(buffer, kBufferSize, - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); } // Joins the thread. elapsed_micros = stopwatch.Elapsed(); @@ -345,7 +316,7 @@ TEST(MessagePipeDispatcherTest, BasicThreaded) { buffer_size = kBufferSize; EXPECT_EQ(MOJO_RESULT_OK, d_1->ReadMessage(buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); EXPECT_EQ(kBufferSize, buffer_size); EXPECT_EQ(123456789, buffer[0]); @@ -444,14 +415,15 @@ class WriterThread : public base::SimpleThread { base::RandInt(1, static_cast<int>(kMaxMessageSize))); EXPECT_EQ(MOJO_RESULT_OK, write_dispatcher_->WriteMessage(buffer, bytes_to_write, - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); *bytes_written_ += bytes_to_write; } // Write one last "quit" message. EXPECT_EQ(MOJO_RESULT_OK, - write_dispatcher_->WriteMessage("quit", 4, NULL, 0, + write_dispatcher_->WriteMessage("quit", 4, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); } @@ -502,7 +474,8 @@ class ReaderThread : public base::SimpleThread { // Clear the buffer so that we can check the result. memset(buffer, 0, sizeof(buffer)); uint32_t buffer_size = static_cast<uint32_t>(sizeof(buffer)); - result = read_dispatcher_->ReadMessage(buffer, &buffer_size, NULL, NULL, + result = read_dispatcher_->ReadMessage(buffer, &buffer_size, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE); EXPECT_TRUE(result == MOJO_RESULT_OK || result == MOJO_RESULT_NOT_FOUND) << "result: " << result; diff --git a/mojo/system/message_pipe_unittest.cc b/mojo/system/message_pipe_unittest.cc index cf0d6a2..aea55fa 100644 --- a/mojo/system/message_pipe_unittest.cc +++ b/mojo/system/message_pipe_unittest.cc @@ -41,7 +41,7 @@ TEST(MessagePipeTest, Basic) { EXPECT_EQ(MOJO_RESULT_NOT_FOUND, mp->ReadMessage(0, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); EXPECT_EQ(kBufferSize, buffer_size); EXPECT_EQ(123, buffer[0]); @@ -54,7 +54,7 @@ TEST(MessagePipeTest, Basic) { EXPECT_EQ(MOJO_RESULT_NOT_FOUND, mp->ReadMessage(1, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); // Write from port 1 (to port 0). @@ -63,7 +63,7 @@ TEST(MessagePipeTest, Basic) { EXPECT_EQ(MOJO_RESULT_OK, mp->WriteMessage(1, buffer, static_cast<uint32_t>(sizeof(buffer[0])), - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); // Read from port 0. @@ -73,7 +73,7 @@ TEST(MessagePipeTest, Basic) { EXPECT_EQ(MOJO_RESULT_OK, mp->ReadMessage(0, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); EXPECT_EQ(static_cast<uint32_t>(sizeof(buffer[0])), buffer_size); EXPECT_EQ(789012345, buffer[0]); @@ -84,7 +84,7 @@ TEST(MessagePipeTest, Basic) { EXPECT_EQ(MOJO_RESULT_NOT_FOUND, mp->ReadMessage(0, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); // Write two messages from port 0 (to port 1). @@ -93,14 +93,14 @@ TEST(MessagePipeTest, Basic) { EXPECT_EQ(MOJO_RESULT_OK, mp->WriteMessage(0, buffer, static_cast<uint32_t>(sizeof(buffer[0])), - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); buffer[0] = 234567890; buffer[1] = 0; EXPECT_EQ(MOJO_RESULT_OK, mp->WriteMessage(0, buffer, static_cast<uint32_t>(sizeof(buffer[0])), - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); // Read from port 1 with buffer size 0 (should get the size of next message). @@ -109,7 +109,7 @@ TEST(MessagePipeTest, Basic) { EXPECT_EQ(MOJO_RESULT_RESOURCE_EXHAUSTED, mp->ReadMessage(1, NULL, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); EXPECT_EQ(static_cast<uint32_t>(sizeof(buffer[0])), buffer_size); @@ -121,7 +121,7 @@ TEST(MessagePipeTest, Basic) { EXPECT_EQ(MOJO_RESULT_RESOURCE_EXHAUSTED, mp->ReadMessage(1, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); EXPECT_EQ(static_cast<uint32_t>(sizeof(buffer[0])), buffer_size); EXPECT_EQ(123, buffer[0]); @@ -134,7 +134,7 @@ TEST(MessagePipeTest, Basic) { EXPECT_EQ(MOJO_RESULT_OK, mp->ReadMessage(1, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); EXPECT_EQ(static_cast<uint32_t>(sizeof(buffer[0])), buffer_size); EXPECT_EQ(123456789, buffer[0]); @@ -147,7 +147,7 @@ TEST(MessagePipeTest, Basic) { EXPECT_EQ(MOJO_RESULT_OK, mp->ReadMessage(1, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); EXPECT_EQ(static_cast<uint32_t>(sizeof(buffer[0])), buffer_size); EXPECT_EQ(234567890, buffer[0]); @@ -158,7 +158,7 @@ TEST(MessagePipeTest, Basic) { EXPECT_EQ(MOJO_RESULT_NOT_FOUND, mp->ReadMessage(1, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); // Write from port 0 (to port 1). @@ -167,7 +167,7 @@ TEST(MessagePipeTest, Basic) { EXPECT_EQ(MOJO_RESULT_OK, mp->WriteMessage(0, buffer, static_cast<uint32_t>(sizeof(buffer[0])), - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); // Close port 0. @@ -179,7 +179,7 @@ TEST(MessagePipeTest, Basic) { EXPECT_EQ(MOJO_RESULT_FAILED_PRECONDITION, mp->WriteMessage(1, buffer, static_cast<uint32_t>(sizeof(buffer[0])), - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); // Read from port 1; should still get message (even though port 0 was closed). @@ -189,7 +189,7 @@ TEST(MessagePipeTest, Basic) { EXPECT_EQ(MOJO_RESULT_OK, mp->ReadMessage(1, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); EXPECT_EQ(static_cast<uint32_t>(sizeof(buffer[0])), buffer_size); EXPECT_EQ(345678901, buffer[0]); @@ -200,7 +200,7 @@ TEST(MessagePipeTest, Basic) { EXPECT_EQ(MOJO_RESULT_FAILED_PRECONDITION, mp->ReadMessage(1, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); mp->Close(1); @@ -219,7 +219,7 @@ TEST(MessagePipeTest, CloseWithQueuedIncomingMessages) { EXPECT_EQ(MOJO_RESULT_OK, mp->WriteMessage(1, buffer, kBufferSize, - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); } @@ -228,7 +228,7 @@ TEST(MessagePipeTest, CloseWithQueuedIncomingMessages) { EXPECT_EQ(MOJO_RESULT_RESOURCE_EXHAUSTED, mp->ReadMessage(0, NULL, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); EXPECT_EQ(kBufferSize, buffer_size); @@ -250,7 +250,7 @@ TEST(MessagePipeTest, DiscardMode) { EXPECT_EQ(MOJO_RESULT_OK, mp->WriteMessage(1, buffer, static_cast<uint32_t>(sizeof(buffer[0])), - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); // Read/discard from port 0 (no buffer); get size. @@ -258,7 +258,7 @@ TEST(MessagePipeTest, DiscardMode) { EXPECT_EQ(MOJO_RESULT_RESOURCE_EXHAUSTED, mp->ReadMessage(0, NULL, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_MAY_DISCARD)); EXPECT_EQ(static_cast<uint32_t>(sizeof(buffer[0])), buffer_size); @@ -267,7 +267,7 @@ TEST(MessagePipeTest, DiscardMode) { EXPECT_EQ(MOJO_RESULT_NOT_FOUND, mp->ReadMessage(0, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_MAY_DISCARD)); // Write from port 1 (to port 0). @@ -276,7 +276,7 @@ TEST(MessagePipeTest, DiscardMode) { EXPECT_EQ(MOJO_RESULT_OK, mp->WriteMessage(1, buffer, static_cast<uint32_t>(sizeof(buffer[0])), - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); // Read from port 0 (buffer big enough). @@ -286,7 +286,7 @@ TEST(MessagePipeTest, DiscardMode) { EXPECT_EQ(MOJO_RESULT_OK, mp->ReadMessage(0, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_MAY_DISCARD)); EXPECT_EQ(static_cast<uint32_t>(sizeof(buffer[0])), buffer_size); EXPECT_EQ(890123456, buffer[0]); @@ -297,7 +297,7 @@ TEST(MessagePipeTest, DiscardMode) { EXPECT_EQ(MOJO_RESULT_NOT_FOUND, mp->ReadMessage(0, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_MAY_DISCARD)); // Write from port 1 (to port 0). @@ -306,7 +306,7 @@ TEST(MessagePipeTest, DiscardMode) { EXPECT_EQ(MOJO_RESULT_OK, mp->WriteMessage(1, buffer, static_cast<uint32_t>(sizeof(buffer[0])), - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); // Read/discard from port 0 (buffer too small); get size. @@ -314,7 +314,7 @@ TEST(MessagePipeTest, DiscardMode) { EXPECT_EQ(MOJO_RESULT_RESOURCE_EXHAUSTED, mp->ReadMessage(0, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_MAY_DISCARD)); EXPECT_EQ(static_cast<uint32_t>(sizeof(buffer[0])), buffer_size); @@ -323,7 +323,7 @@ TEST(MessagePipeTest, DiscardMode) { EXPECT_EQ(MOJO_RESULT_NOT_FOUND, mp->ReadMessage(0, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_MAY_DISCARD)); // Write from port 1 (to port 0). @@ -332,7 +332,7 @@ TEST(MessagePipeTest, DiscardMode) { EXPECT_EQ(MOJO_RESULT_OK, mp->WriteMessage(1, buffer, static_cast<uint32_t>(sizeof(buffer[0])), - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); // Discard from port 0. @@ -340,7 +340,7 @@ TEST(MessagePipeTest, DiscardMode) { EXPECT_EQ(MOJO_RESULT_RESOURCE_EXHAUSTED, mp->ReadMessage(0, NULL, NULL, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_MAY_DISCARD)); // Read again from port 0 -- it should be empty. @@ -348,7 +348,7 @@ TEST(MessagePipeTest, DiscardMode) { EXPECT_EQ(MOJO_RESULT_NOT_FOUND, mp->ReadMessage(0, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_MAY_DISCARD)); mp->Close(0); @@ -386,7 +386,7 @@ TEST(MessagePipeTest, BasicWaiting) { EXPECT_EQ(MOJO_RESULT_OK, mp->WriteMessage(0, buffer, kBufferSize, - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); // Port 1 should already be readable now. @@ -423,7 +423,7 @@ TEST(MessagePipeTest, BasicWaiting) { EXPECT_EQ(MOJO_RESULT_OK, mp->ReadMessage(1, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); EXPECT_EQ(123456789, buffer[0]); @@ -456,7 +456,7 @@ TEST(MessagePipeTest, ThreadedWaiting) { EXPECT_EQ(MOJO_RESULT_OK, mp->WriteMessage(0, buffer, kBufferSize, - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); mp->RemoveWaiter(1, thread.waiter()); diff --git a/mojo/system/remote_message_pipe_posix_unittest.cc b/mojo/system/remote_message_pipe_posix_unittest.cc index 5f16166..d5b0d97 100644 --- a/mojo/system/remote_message_pipe_posix_unittest.cc +++ b/mojo/system/remote_message_pipe_posix_unittest.cc @@ -152,7 +152,7 @@ TEST_F(RemoteMessagePipeTest, Basic) { EXPECT_EQ(MOJO_RESULT_OK, mp_0->WriteMessage(0, hello, sizeof(hello), - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); // Wait. @@ -163,7 +163,7 @@ TEST_F(RemoteMessagePipeTest, Basic) { EXPECT_EQ(MOJO_RESULT_OK, mp_1->ReadMessage(1, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); EXPECT_EQ(sizeof(hello), static_cast<size_t>(buffer_size)); EXPECT_EQ(0, strcmp(buffer, hello)); @@ -177,7 +177,7 @@ TEST_F(RemoteMessagePipeTest, Basic) { EXPECT_EQ(MOJO_RESULT_OK, mp_1->WriteMessage(1, world, sizeof(world), - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); EXPECT_EQ(456, waiter.Wait(MOJO_DEADLINE_INDEFINITE)); @@ -187,7 +187,7 @@ TEST_F(RemoteMessagePipeTest, Basic) { EXPECT_EQ(MOJO_RESULT_OK, mp_0->ReadMessage(0, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); EXPECT_EQ(sizeof(world), static_cast<size_t>(buffer_size)); EXPECT_EQ(0, strcmp(buffer, world)); @@ -256,7 +256,7 @@ TEST_F(RemoteMessagePipeTest, Multiplex) { EXPECT_EQ(MOJO_RESULT_OK, mp_2->WriteMessage(0, hello, sizeof(hello), - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); EXPECT_EQ(789, waiter.Wait(MOJO_DEADLINE_INDEFINITE)); @@ -267,19 +267,19 @@ TEST_F(RemoteMessagePipeTest, Multiplex) { EXPECT_EQ(MOJO_RESULT_NOT_FOUND, mp_0->ReadMessage(0, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); buffer_size = static_cast<uint32_t>(sizeof(buffer)); EXPECT_EQ(MOJO_RESULT_NOT_FOUND, mp_1->ReadMessage(1, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); buffer_size = static_cast<uint32_t>(sizeof(buffer)); EXPECT_EQ(MOJO_RESULT_NOT_FOUND, mp_2->ReadMessage(0, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); // Read from MP 3, port 1. @@ -287,7 +287,7 @@ TEST_F(RemoteMessagePipeTest, Multiplex) { EXPECT_EQ(MOJO_RESULT_OK, mp_3->ReadMessage(1, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); EXPECT_EQ(sizeof(hello), static_cast<size_t>(buffer_size)); EXPECT_EQ(0, strcmp(buffer, hello)); @@ -301,7 +301,7 @@ TEST_F(RemoteMessagePipeTest, Multiplex) { EXPECT_EQ(MOJO_RESULT_OK, mp_0->WriteMessage(0, world, sizeof(world), - NULL, 0, + NULL, MOJO_WRITE_MESSAGE_FLAG_NONE)); EXPECT_EQ(123, waiter.Wait(MOJO_DEADLINE_INDEFINITE)); @@ -312,26 +312,26 @@ TEST_F(RemoteMessagePipeTest, Multiplex) { EXPECT_EQ(MOJO_RESULT_NOT_FOUND, mp_0->ReadMessage(0, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); buffer_size = static_cast<uint32_t>(sizeof(buffer)); EXPECT_EQ(MOJO_RESULT_NOT_FOUND, mp_2->ReadMessage(0, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); buffer_size = static_cast<uint32_t>(sizeof(buffer)); EXPECT_EQ(MOJO_RESULT_NOT_FOUND, mp_3->ReadMessage(1, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); buffer_size = static_cast<uint32_t>(sizeof(buffer)); EXPECT_EQ(MOJO_RESULT_OK, mp_1->ReadMessage(1, buffer, &buffer_size, - NULL, NULL, + 0, NULL, MOJO_READ_MESSAGE_FLAG_NONE)); EXPECT_EQ(sizeof(world), static_cast<size_t>(buffer_size)); EXPECT_EQ(0, strcmp(buffer, world)); |