OLD | NEW |
---|---|
1 // Copyright 2015 The Chromium Authors. All rights reserved. | 1 // Copyright 2015 The Chromium Authors. All rights reserved. |
2 // Use of this source code is governed by a BSD-style license that can be | 2 // Use of this source code is governed by a BSD-style license that can be |
3 // found in the LICENSE file. | 3 // found in the LICENSE file. |
4 | 4 |
5 #include "mojo/edk/system/message_pipe_dispatcher.h" | 5 #include "mojo/edk/system/message_pipe_dispatcher.h" |
6 | 6 |
7 #include "base/bind.h" | 7 #include "base/bind.h" |
8 #include "base/debug/stack_trace.h" | 8 #include "base/debug/stack_trace.h" |
9 #include "base/logging.h" | 9 #include "base/logging.h" |
10 #include "base/message_loop/message_loop.h" | 10 #include "base/message_loop/message_loop.h" |
(...skipping 136 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... | |
147 base::AutoLock locker(lock()); | 147 base::AutoLock locker(lock()); |
148 CHECK(transferable_); | 148 CHECK(transferable_); |
149 calling_init_ = true; | 149 calling_init_ = true; |
150 if (channel_) | 150 if (channel_) |
151 channel_->Init(this); | 151 channel_->Init(this); |
152 calling_init_ = false; | 152 calling_init_ = false; |
153 } | 153 } |
154 | 154 |
155 void MessagePipeDispatcher::CloseOnIO() { | 155 void MessagePipeDispatcher::CloseOnIO() { |
156 base::AutoLock locker(lock()); | 156 base::AutoLock locker(lock()); |
157 Release(); // To match CloseImplNoLock. | 157 if (release_reference_in_close_) |
158 Release(); // To match CloseImplNoLock. | |
158 if (transferable_) { | 159 if (transferable_) { |
159 if (channel_) { | 160 if (channel_) { |
160 channel_->Shutdown(); | 161 channel_->Shutdown(); |
161 channel_ = nullptr; | 162 channel_ = nullptr; |
162 } | 163 } |
163 } else { | 164 } else { |
164 if (non_transferable_state_ == CONNECT_CALLED || | 165 if (non_transferable_state_ == CONNECT_CALLED || |
165 non_transferable_state_ == WAITING_FOR_READ_OR_WRITE) { | 166 non_transferable_state_ == WAITING_FOR_READ_OR_WRITE) { |
166 if (non_transferable_state_ == WAITING_FOR_READ_OR_WRITE) | 167 if (non_transferable_state_ == WAITING_FOR_READ_OR_WRITE) |
167 RequestNontransferableChannel(); | 168 RequestNontransferableChannel(); |
(...skipping 231 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... | |
399 MessagePipeDispatcher::MessagePipeDispatcher(bool transferable) | 400 MessagePipeDispatcher::MessagePipeDispatcher(bool transferable) |
400 : channel_(nullptr), | 401 : channel_(nullptr), |
401 serialized_read_fds_length_(0u), | 402 serialized_read_fds_length_(0u), |
402 serialized_write_fds_length_(0u), | 403 serialized_write_fds_length_(0u), |
403 serialized_message_fds_length_(0u), | 404 serialized_message_fds_length_(0u), |
404 pipe_id_(0), | 405 pipe_id_(0), |
405 non_transferable_state_(WAITING_FOR_READ_OR_WRITE), | 406 non_transferable_state_(WAITING_FOR_READ_OR_WRITE), |
406 serialized_(false), | 407 serialized_(false), |
407 calling_init_(false), | 408 calling_init_(false), |
408 write_error_(false), | 409 write_error_(false), |
409 transferable_(transferable) { | 410 transferable_(transferable), |
411 release_reference_in_close_(false) { | |
410 } | 412 } |
411 | 413 |
412 MessagePipeDispatcher::~MessagePipeDispatcher() { | 414 MessagePipeDispatcher::~MessagePipeDispatcher() { |
413 // |Close()|/|CloseImplNoLock()| should have taken care of the channel. The | 415 // |Close()|/|CloseImplNoLock()| should have taken care of the channel. The |
414 // exception is if they posted a task to run CloseOnIO but the IO thread shut | 416 // exception is if they posted a task to run CloseOnIO but the IO thread shut |
415 // down and so when it was deleting pending tasks it caused the last reference | 417 // down and so when it was deleting pending tasks it caused the last reference |
416 // to destruct this object. In that case, safe to destroy the channel. | 418 // to destruct this object. In that case, safe to destroy the channel. |
417 if (channel_ && | 419 if (channel_ && |
418 internal::g_io_thread_task_runner->RunsTasksOnCurrentThread()) { | 420 internal::g_io_thread_task_runner->RunsTasksOnCurrentThread()) { |
419 if (transferable_) { | 421 if (transferable_) { |
(...skipping 15 matching lines...) Expand all Loading... | |
435 } | 437 } |
436 | 438 |
437 void MessagePipeDispatcher::CloseImplNoLock() { | 439 void MessagePipeDispatcher::CloseImplNoLock() { |
438 lock().AssertAcquired(); | 440 lock().AssertAcquired(); |
439 // We take a manual refcount because at shutdown, the task below might not get | 441 // We take a manual refcount because at shutdown, the task below might not get |
440 // a chance to execute. If that happens, the RawChannel's will still call our | 442 // a chance to execute. If that happens, the RawChannel's will still call our |
441 // OnError method because it always runs (since it watches thread | 443 // OnError method because it always runs (since it watches thread |
442 // destruction). So to avoid UAF, manually add a reference and only release it | 444 // destruction). So to avoid UAF, manually add a reference and only release it |
443 // if the task runs. | 445 // if the task runs. |
444 AddRef(); | 446 AddRef(); |
447 release_reference_in_close_ = true; | |
445 internal::g_io_thread_task_runner->PostTask( | 448 internal::g_io_thread_task_runner->PostTask( |
446 FROM_HERE, base::Bind(&MessagePipeDispatcher::CloseOnIO, this)); | 449 FROM_HERE, base::Bind(&MessagePipeDispatcher::CloseOnIO, this)); |
447 } | 450 } |
448 | 451 |
449 void MessagePipeDispatcher::SerializeInternal() { | 452 void MessagePipeDispatcher::SerializeInternal() { |
450 serialized_ = true; | 453 serialized_ = true; |
451 if (!transferable_) { | 454 if (!transferable_) { |
452 CHECK(non_transferable_state_ == WAITING_FOR_READ_OR_WRITE) | 455 CHECK(non_transferable_state_ == WAITING_FOR_READ_OR_WRITE) |
453 << "Non transferable message pipe being sent after read/write/waited. " | 456 << "Non transferable message pipe being sent after read/write/waited. " |
454 << "MOJO_CREATE_MESSAGE_PIPE_OPTIONS_FLAG_TRANSFERABLE must be used if " | 457 << "MOJO_CREATE_MESSAGE_PIPE_OPTIONS_FLAG_TRANSFERABLE must be used if " |
(...skipping 453 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... | |
908 break; | 911 break; |
909 case ERROR_WRITE: | 912 case ERROR_WRITE: |
910 // Write errors are slightly notable: they probably shouldn't happen under | 913 // Write errors are slightly notable: they probably shouldn't happen under |
911 // normal operation (but maybe the other side crashed). | 914 // normal operation (but maybe the other side crashed). |
912 LOG(WARNING) << "MessagePipeDispatcher write error"; | 915 LOG(WARNING) << "MessagePipeDispatcher write error"; |
913 DCHECK_EQ(write_error_, false) << "Should only get one write error."; | 916 DCHECK_EQ(write_error_, false) << "Should only get one write error."; |
914 write_error_ = true; | 917 write_error_ = true; |
915 break; | 918 break; |
916 } | 919 } |
917 | 920 |
921 bool call_release = false; | |
918 if (started_transport_.Try()) { | 922 if (started_transport_.Try()) { |
919 base::AutoLock locker(lock()); | 923 base::AutoLock locker(lock()); |
920 // We can get two OnError callbacks before the post task below completes. | 924 // We can get two OnError callbacks before the post task below completes. |
921 // Although RawChannel still has a pointer to this object until Shutdown is | 925 // Although RawChannel still has a pointer to this object until Shutdown is |
922 // called, that is safe since this class always does a PostTask to the IO | 926 // called, that is safe since this class always does a PostTask to the IO |
923 // thread to self destruct. | 927 // thread to self destruct. |
924 if (channel_ && error != ERROR_WRITE) { | 928 if (channel_ && error != ERROR_WRITE) { |
925 if (transferable_) { | 929 if (transferable_) { |
926 channel_->Shutdown(); | 930 channel_->Shutdown(); |
927 } else { | 931 } else { |
928 CHECK_NE(non_transferable_state_, CLOSED); | 932 CHECK_NE(non_transferable_state_, CLOSED); |
929 internal::g_broker->CloseMessagePipe(pipe_id_, this); | 933 internal::g_broker->CloseMessagePipe(pipe_id_, this); |
930 non_transferable_state_ = CLOSED; | 934 non_transferable_state_ = CLOSED; |
931 } | 935 } |
932 channel_ = nullptr; | 936 channel_ = nullptr; |
937 if (release_reference_in_close_) { | |
938 // CloseOnIO might never get called on process shutdown. Release the | |
939 // extra reference here to avoid leaks in tests. | |
940 call_release = true; | |
msw
2015/12/18 01:54:26
nit: could you actually just call Release() here a
jam
2015/12/18 02:03:47
that's what I did in patchset 1 but that failed si
| |
941 release_reference_in_close_ = false; | |
942 } | |
933 } | 943 } |
934 awakable_list_.AwakeForStateChange(GetHandleSignalsStateImplNoLock()); | 944 awakable_list_.AwakeForStateChange(GetHandleSignalsStateImplNoLock()); |
935 started_transport_.Release(); | 945 started_transport_.Release(); |
936 } else { | 946 } else { |
937 // We must be waiting to call ReleaseHandle. It will call Shutdown. | 947 // We must be waiting to call ReleaseHandle. It will call Shutdown. |
938 } | 948 } |
949 | |
950 if (call_release) | |
951 Release(); | |
939 } | 952 } |
940 | 953 |
941 MojoResult MessagePipeDispatcher::AttachTransportsNoLock( | 954 MojoResult MessagePipeDispatcher::AttachTransportsNoLock( |
942 MessageInTransit* message, | 955 MessageInTransit* message, |
943 std::vector<DispatcherTransport>* transports) { | 956 std::vector<DispatcherTransport>* transports) { |
944 DCHECK(!message->has_dispatchers()); | 957 DCHECK(!message->has_dispatchers()); |
945 | 958 |
946 // You're not allowed to send either handle to a message pipe over the message | 959 // You're not allowed to send either handle to a message pipe over the message |
947 // pipe, so check for this. (The case of trying to write a handle to itself is | 960 // pipe, so check for this. (The case of trying to write a handle to itself is |
948 // taken care of by |Core|. That case kind of makes sense, but leads to | 961 // taken care of by |Core|. That case kind of makes sense, but leads to |
(...skipping 48 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... | |
997 // PostTask since the broker can call us back synchronously. | 1010 // PostTask since the broker can call us back synchronously. |
998 internal::g_io_thread_task_runner->PostTask( | 1011 internal::g_io_thread_task_runner->PostTask( |
999 FROM_HERE, | 1012 FROM_HERE, |
1000 base::Bind(&Broker::ConnectMessagePipe, | 1013 base::Bind(&Broker::ConnectMessagePipe, |
1001 base::Unretained(internal::g_broker), pipe_id_, | 1014 base::Unretained(internal::g_broker), pipe_id_, |
1002 base::Unretained(this))); | 1015 base::Unretained(this))); |
1003 } | 1016 } |
1004 | 1017 |
1005 } // namespace edk | 1018 } // namespace edk |
1006 } // namespace mojo | 1019 } // namespace mojo |
OLD | NEW |