Index: mojo/system/core_impl.cc |
diff --git a/mojo/system/core_impl.cc b/mojo/system/core_impl.cc |
index ea1ebdaa14b6e10d5a0c5e517f1d4720667d8cbb..c3fa10d270ed2f3df3e94c75ad5bd6933c77f853 100644 |
--- a/mojo/system/core_impl.cc |
+++ b/mojo/system/core_impl.cc |
@@ -87,8 +87,7 @@ CoreImpl::HandleTableEntry::~HandleTableEntry() { |
DCHECK(!busy); |
} |
-CoreImpl::CoreImpl() |
- : next_handle_(MOJO_HANDLE_INVALID + 1) { |
+CoreImpl::CoreImpl() { |
} |
CoreImpl::~CoreImpl() { |
@@ -99,7 +98,7 @@ CoreImpl::~CoreImpl() { |
MojoHandle CoreImpl::AddDispatcher( |
const scoped_refptr<Dispatcher>& dispatcher) { |
base::AutoLock locker(handle_table_lock_); |
- return AddDispatcherNoLock(dispatcher); |
+ return handle_table_.AddDispatcher(dispatcher); |
} |
MojoTimeTicks CoreImpl::GetTimeTicksNow() { |
@@ -113,13 +112,10 @@ MojoResult CoreImpl::Close(MojoHandle handle) { |
scoped_refptr<Dispatcher> dispatcher; |
{ |
base::AutoLock locker(handle_table_lock_); |
- HandleTableMap::iterator it = handle_table_.find(handle); |
- if (it == handle_table_.end()) |
- return MOJO_RESULT_INVALID_ARGUMENT; |
- if (it->second.busy) |
- return MOJO_RESULT_BUSY; |
- dispatcher = it->second.dispatcher; |
- handle_table_.erase(it); |
+ MojoResult result = handle_table_.GetAndRemoveDispatcher(handle, |
+ &dispatcher); |
+ if (result != MOJO_RESULT_OK) |
+ return result; |
} |
// The dispatcher doesn't have a say in being closed, but gets notified of it. |
@@ -160,31 +156,25 @@ MojoResult CoreImpl::CreateMessagePipe(MojoHandle* message_pipe_handle0, |
scoped_refptr<MessagePipeDispatcher> dispatcher0(new MessagePipeDispatcher()); |
scoped_refptr<MessagePipeDispatcher> dispatcher1(new MessagePipeDispatcher()); |
- MojoHandle h0, h1; |
+ std::pair<MojoHandle, MojoHandle> handle_pair; |
{ |
base::AutoLock locker(handle_table_lock_); |
- |
- // TODO(vtl): crbug.com/345911: On failure, we should close the dispatcher |
- // (outside the table lock). |
- h0 = AddDispatcherNoLock(dispatcher0); |
- if (h0 == MOJO_HANDLE_INVALID) |
- return MOJO_RESULT_RESOURCE_EXHAUSTED; |
- |
- // TODO(vtl): crbug.com/345911: On failure, we should close both dispatchers |
- // (outside the table lock). |
- h1 = AddDispatcherNoLock(dispatcher1); |
- if (h1 == MOJO_HANDLE_INVALID) { |
- handle_table_.erase(h0); |
- return MOJO_RESULT_RESOURCE_EXHAUSTED; |
- } |
+ handle_pair = handle_table_.AddDispatcherPair(dispatcher0, dispatcher1); |
+ } |
+ if (handle_pair.first == MOJO_HANDLE_INVALID) { |
+ DCHECK_EQ(handle_pair.second, MOJO_HANDLE_INVALID); |
+ LOG(ERROR) << "Handle table full"; |
+ dispatcher0->Close(); |
+ dispatcher1->Close(); |
+ return MOJO_RESULT_RESOURCE_EXHAUSTED; |
} |
scoped_refptr<MessagePipe> message_pipe(new MessagePipe()); |
dispatcher0->Init(message_pipe, 0); |
dispatcher1->Init(message_pipe, 1); |
- *message_pipe_handle0 = h0; |
- *message_pipe_handle1 = h1; |
+ *message_pipe_handle0 = handle_pair.first; |
+ *message_pipe_handle1 = handle_pair.second; |
return MOJO_RESULT_OK; |
} |
@@ -234,71 +224,10 @@ MojoResult CoreImpl::WriteMessage(MojoHandle message_pipe_handle, |
// 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] == message_pipe_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 start the transport. |
- DispatcherTransport transport = |
- Dispatcher::CoreImplAccess::TryStartTransport( |
- entries[i]->dispatcher.get()); |
- if (!transport.is_valid()) { |
- // Unset the busy flag (since it won't be unset below). |
- entries[i]->busy = false; |
- error_result = MOJO_RESULT_BUSY; |
- break; |
- } |
- |
- // Check if the dispatcher is busy (e.g., in a two-phase read/write). |
- // (Note that this must be done after the dispatcher's lock is acquired.) |
- if (transport.IsBusy()) { |
- // Unset the busy flag and end the transport (since it won't be done |
- // below). |
- entries[i]->busy = false; |
- transport.End(); |
- error_result = MOJO_RESULT_BUSY; |
- break; |
- } |
- |
- // Hang on to the transport (which we'll need to end the transport). |
- transports[i] = transport; |
- } |
- 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; |
- transports[j].End(); |
- } |
- return error_result; |
- } |
+ MojoResult result = handle_table_.MarkBusyAndStartTransport( |
+ message_pipe_handle, handles, num_handles, &transports); |
+ if (result != MOJO_RESULT_OK) |
+ return result; |
} |
MojoResult rv = dispatcher->WriteMessage(bytes, num_bytes, &transports, |
@@ -309,28 +238,12 @@ MojoResult CoreImpl::WriteMessage(MojoHandle message_pipe_handle, |
for (uint32_t i = 0; i < num_handles; i++) |
transports[i].End(); |
- 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); |
- it->second.busy = false; // For the sake of a |DCHECK()|. |
- 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; |
- } |
+ if (rv == MOJO_RESULT_OK) |
+ handle_table_.RemoveBusyHandles(handles, num_handles); |
+ else |
+ handle_table_.RestoreBusyHandles(handles, num_handles); |
} |
return rv; |
@@ -366,17 +279,19 @@ MojoResult CoreImpl::ReadMessage(MojoHandle message_pipe_handle, |
DCHECK(num_handles); |
DCHECK_LE(dispatchers.size(), static_cast<size_t>(*num_handles)); |
- 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). |
- // TODO(vtl): crbug.com/345911: On failure, we should close the dispatcher |
- // (outside the table lock). |
- handles[i] = AddDispatcherNoLock(dispatchers[i]); |
- LOG_IF(ERROR, handles[i] == MOJO_HANDLE_INVALID) |
- << "Failed to add dispatcher (" << dispatchers[i].get() << ")"; |
+ bool success; |
+ { |
+ base::AutoLock locker(handle_table_lock_); |
+ success = handle_table_.AddDispatcherVector(dispatchers, handles); |
+ } |
+ if (!success) { |
+ LOG(ERROR) << "Received message with " << dispatchers.size() |
+ << " handles, but handle table full"; |
+ // Close dispatchers (outside the lock). |
+ for (size_t i = 0; i < dispatchers.size(); i++) { |
+ if (dispatchers[i]) |
+ dispatchers[i]->Close(); |
+ } |
} |
} |
@@ -409,31 +324,27 @@ MojoResult CoreImpl::CreateDataPipe(const MojoCreateDataPipeOptions* options, |
scoped_refptr<DataPipeConsumerDispatcher> consumer_dispatcher( |
new DataPipeConsumerDispatcher()); |
- MojoHandle producer_handle, consumer_handle; |
+ std::pair<MojoHandle, MojoHandle> handle_pair; |
{ |
base::AutoLock locker(handle_table_lock_); |
- |
- // TODO(vtl): crbug.com/345911: On failure, we should close the dispatcher |
- // (outside the table lock). |
- producer_handle = AddDispatcherNoLock(producer_dispatcher); |
- if (producer_handle == MOJO_HANDLE_INVALID) |
- return MOJO_RESULT_RESOURCE_EXHAUSTED; |
- |
- // TODO(vtl): crbug.com/345911: On failure, we should close both dispatchers |
- // (outside the table lock). |
- consumer_handle = AddDispatcherNoLock(consumer_dispatcher); |
- if (consumer_handle == MOJO_HANDLE_INVALID) { |
- handle_table_.erase(producer_handle); |
- return MOJO_RESULT_RESOURCE_EXHAUSTED; |
- } |
+ handle_pair = handle_table_.AddDispatcherPair(producer_dispatcher, |
+ consumer_dispatcher); |
+ } |
+ if (handle_pair.first == MOJO_HANDLE_INVALID) { |
+ DCHECK_EQ(handle_pair.second, MOJO_HANDLE_INVALID); |
+ LOG(ERROR) << "Handle table full"; |
+ producer_dispatcher->Close(); |
+ consumer_dispatcher->Close(); |
+ return MOJO_RESULT_RESOURCE_EXHAUSTED; |
} |
+ DCHECK_NE(handle_pair.second, MOJO_HANDLE_INVALID); |
scoped_refptr<DataPipe> data_pipe(new LocalDataPipe(validated_options)); |
producer_dispatcher->Init(data_pipe); |
consumer_dispatcher->Init(data_pipe); |
- *data_pipe_producer_handle = producer_handle; |
- *data_pipe_consumer_handle = consumer_handle; |
+ *data_pipe_producer_handle = handle_pair.first; |
+ *data_pipe_consumer_handle = handle_pair.second; |
return MOJO_RESULT_OK; |
} |
@@ -534,15 +445,11 @@ MojoResult CoreImpl::CreateSharedBuffer( |
return result; |
} |
- MojoHandle h; |
- { |
- base::AutoLock locker(handle_table_lock_); |
- |
- // TODO(vtl): crbug.com/345911: On failure, we should close the dispatcher |
- // (outside the table lock). |
- h = AddDispatcherNoLock(dispatcher); |
- if (h == MOJO_HANDLE_INVALID) |
- return MOJO_RESULT_RESOURCE_EXHAUSTED; |
+ MojoHandle h = AddDispatcher(dispatcher); |
+ if (h == MOJO_HANDLE_INVALID) { |
+ LOG(ERROR) << "Handle table full"; |
+ dispatcher->Close(); |
+ return MOJO_RESULT_RESOURCE_EXHAUSTED; |
} |
*shared_buffer_handle = h; |
@@ -567,15 +474,11 @@ MojoResult CoreImpl::DuplicateBufferHandle( |
if (result != MOJO_RESULT_OK) |
return result; |
- MojoHandle new_handle; |
- { |
- base::AutoLock locker(handle_table_lock_); |
- |
- // TODO(vtl): crbug.com/345911: On failure, we should close the dispatcher |
- // (outside the table lock). |
- new_handle = AddDispatcherNoLock(new_dispatcher); |
- if (new_handle == MOJO_HANDLE_INVALID) |
- return MOJO_RESULT_RESOURCE_EXHAUSTED; |
+ MojoHandle new_handle = AddDispatcher(new_dispatcher); |
+ if (new_handle == MOJO_HANDLE_INVALID) { |
+ LOG(ERROR) << "Handle table full"; |
+ dispatcher->Close(); |
+ return MOJO_RESULT_RESOURCE_EXHAUSTED; |
} |
*new_buffer_handle = new_handle; |
@@ -620,37 +523,7 @@ scoped_refptr<Dispatcher> CoreImpl::GetDispatcher(MojoHandle handle) { |
return NULL; |
base::AutoLock locker(handle_table_lock_); |
- HandleTableMap::iterator it = handle_table_.find(handle); |
- if (it == handle_table_.end()) |
- return NULL; |
- |
- return it->second.dispatcher; |
-} |
- |
-MojoHandle CoreImpl::AddDispatcherNoLock( |
- const scoped_refptr<Dispatcher>& dispatcher) { |
- handle_table_lock_.AssertAcquired(); |
- DCHECK_NE(next_handle_, MOJO_HANDLE_INVALID); |
- |
- if (!dispatcher.get() || handle_table_.size() >= kMaxHandleTableSize) |
- return MOJO_HANDLE_INVALID; |
- |
- // TODO(vtl): Maybe we want to do something different/smarter. (Or maybe try |
- // assigning randomly?) |
- while (handle_table_.find(next_handle_) != handle_table_.end()) { |
- next_handle_++; |
- if (next_handle_ == MOJO_HANDLE_INVALID) |
- next_handle_++; |
- } |
- |
- MojoHandle new_handle = next_handle_; |
- handle_table_[new_handle] = HandleTableEntry(dispatcher); |
- |
- next_handle_++; |
- if (next_handle_ == MOJO_HANDLE_INVALID) |
- next_handle_++; |
- |
- return new_handle; |
+ return handle_table_.GetDispatcher(handle); |
} |
// Note: We allow |handles| to repeat the same handle multiple times, since |