Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(619)

Unified Diff: mojo/system/channel.cc

Issue 612903003: Mojo: Split up Channel::RemoveMessagePipeEndpoint(). (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@del_onremove
Patch Set: rebased Created 6 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « mojo/system/channel.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: mojo/system/channel.cc
diff --git a/mojo/system/channel.cc b/mojo/system/channel.cc
index 0c4033fcac2ba70071e2aeee32bcb32fe2695ab0..18c2df18b4716356d3214a7c472351cd21ec8891 100644
--- a/mojo/system/channel.cc
+++ b/mojo/system/channel.cc
@@ -417,8 +417,8 @@ void Channel::OnReadMessageForChannel(
DVLOG(2) << "Handling channel message to remove message pipe (local ID "
<< message_view.destination_id() << ", remote ID "
<< message_view.source_id() << ")";
- if (!RemoveMessagePipeEndpoint(message_view.destination_id(),
- message_view.source_id())) {
+ if (!OnRemoveMessagePipeEndpoint(message_view.destination_id(),
+ message_view.source_id())) {
HandleRemoteError(
"Received invalid channel message to remove message pipe");
}
@@ -427,8 +427,7 @@ void Channel::OnReadMessageForChannel(
DVLOG(2) << "Handling channel message to ack remove message pipe (local "
"ID " << message_view.destination_id() << ", remote ID "
<< message_view.source_id() << ")";
- if (!RemoveMessagePipeEndpoint(message_view.destination_id(),
- message_view.source_id())) {
+ if (!OnRemoveMessagePipeEndpointAck(message_view.destination_id())) {
HandleRemoteError(
"Received invalid channel message to ack remove message pipe");
}
@@ -440,14 +439,11 @@ void Channel::OnReadMessageForChannel(
}
}
-bool Channel::RemoveMessagePipeEndpoint(
+bool Channel::OnRemoveMessagePipeEndpoint(
MessageInTransit::EndpointId local_id,
MessageInTransit::EndpointId remote_id) {
DCHECK(creation_thread_checker_.CalledOnValidThread());
- // If this is non-null after the locked block, the endpoint should be detached
- // (and no remove ack message sent).
- scoped_refptr<ChannelEndpoint> endpoint_to_detach;
scoped_refptr<MessagePipe> message_pipe;
unsigned port = ~0u;
{
@@ -455,31 +451,31 @@ bool Channel::RemoveMessagePipeEndpoint(
IdToEndpointMap::iterator it = local_id_to_endpoint_map_.find(local_id);
if (it == local_id_to_endpoint_map_.end()) {
- DVLOG(2) << "Remove message pipe error: not found";
+ DVLOG(2) << "Remove message pipe endpoint error: not found";
return false;
}
switch (it->second->state_) {
case ChannelEndpoint::STATE_NORMAL:
- it->second->state_ = ChannelEndpoint::STATE_WAIT_LOCAL_DETACH;
- message_pipe = it->second->message_pipe_;
- port = it->second->port_;
- it->second->message_pipe_ = nullptr;
- // We have to send a remove ack message (outside the lock).
+ // This is the normal case; we'll proceed on to "wait local detach".
break;
+
case ChannelEndpoint::STATE_WAIT_LOCAL_DETACH:
- DVLOG(2) << "Remove message pipe error: wrong state";
+ // We can only be in this state because we got a "remove" already, so
+ // getting another such message is invalid.
+ DVLOG(2) << "Remove message pipe endpoint error: wrong state";
return false;
+
case ChannelEndpoint::STATE_WAIT_REMOTE_REMOVE_ACK:
- endpoint_to_detach = it->second;
- local_id_to_endpoint_map_.erase(it);
- // We have to detach (outside the lock).
- break;
+ // Remove messages "crossed"; we have to wait for the ack.
+ return true;
}
- }
- if (endpoint_to_detach.get()) {
- endpoint_to_detach->DetachFromChannel();
- return true;
+
+ it->second->state_ = ChannelEndpoint::STATE_WAIT_LOCAL_DETACH;
+ message_pipe = it->second->message_pipe_;
+ port = it->second->port_;
+ it->second->message_pipe_ = nullptr;
+ // Send the remove ack message outside the lock.
}
if (!SendControlMessage(
@@ -494,7 +490,34 @@ bool Channel::RemoveMessagePipeEndpoint(
}
message_pipe->Close(port);
+ return true;
+}
+
+bool Channel::OnRemoveMessagePipeEndpointAck(
+ MessageInTransit::EndpointId local_id) {
+ DCHECK(creation_thread_checker_.CalledOnValidThread());
+
+ scoped_refptr<ChannelEndpoint> endpoint_to_detach;
+ {
+ base::AutoLock locker(lock_);
+
+ IdToEndpointMap::iterator it = local_id_to_endpoint_map_.find(local_id);
+ if (it == local_id_to_endpoint_map_.end()) {
+ DVLOG(2) << "Remove message pipe endpoint ack error: not found";
+ return false;
+ }
+
+ if (it->second->state_ != ChannelEndpoint::STATE_WAIT_REMOTE_REMOVE_ACK) {
+ DVLOG(2) << "Remove message pipe endpoint ack error: wrong state";
+ return false;
+ }
+
+ endpoint_to_detach = it->second;
+ local_id_to_endpoint_map_.erase(it);
+ // Detach the endpoint outside the lock.
+ }
+ endpoint_to_detach->DetachFromChannel();
return true;
}
« no previous file with comments | « mojo/system/channel.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698