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

Unified Diff: mojo/edk/system/message_pipe_dispatcher.cc

Issue 1534013003: Fix memory leak in unittests with new Mojo EDK. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: fix Created 5 years 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
« content/app/mojo/mojo_init.cc ('K') | « mojo/edk/system/message_pipe_dispatcher.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: mojo/edk/system/message_pipe_dispatcher.cc
diff --git a/mojo/edk/system/message_pipe_dispatcher.cc b/mojo/edk/system/message_pipe_dispatcher.cc
index 75f7227c9296e86531653c48bda07fc8ffd15453..2a39ebdd77052c1012bd06a61aea8d2865e7b4d1 100644
--- a/mojo/edk/system/message_pipe_dispatcher.cc
+++ b/mojo/edk/system/message_pipe_dispatcher.cc
@@ -154,7 +154,8 @@ void MessagePipeDispatcher::InitOnIO() {
void MessagePipeDispatcher::CloseOnIO() {
base::AutoLock locker(lock());
- Release(); // To match CloseImplNoLock.
+ if (release_reference_in_close_)
+ Release(); // To match CloseImplNoLock.
if (transferable_) {
if (channel_) {
channel_->Shutdown();
@@ -406,7 +407,8 @@ MessagePipeDispatcher::MessagePipeDispatcher(bool transferable)
serialized_(false),
calling_init_(false),
write_error_(false),
- transferable_(transferable) {
+ transferable_(transferable),
+ release_reference_in_close_(false) {
}
MessagePipeDispatcher::~MessagePipeDispatcher() {
@@ -442,6 +444,7 @@ void MessagePipeDispatcher::CloseImplNoLock() {
// destruction). So to avoid UAF, manually add a reference and only release it
// if the task runs.
AddRef();
+ release_reference_in_close_ = true;
internal::g_io_thread_task_runner->PostTask(
FROM_HERE, base::Bind(&MessagePipeDispatcher::CloseOnIO, this));
}
@@ -915,6 +918,7 @@ void MessagePipeDispatcher::OnError(Error error) {
break;
}
+ bool call_release = false;
if (started_transport_.Try()) {
base::AutoLock locker(lock());
// We can get two OnError callbacks before the post task below completes.
@@ -930,12 +934,21 @@ void MessagePipeDispatcher::OnError(Error error) {
non_transferable_state_ = CLOSED;
}
channel_ = nullptr;
+ if (release_reference_in_close_) {
+ // CloseOnIO might never get called on process shutdown. Release the
+ // extra reference here to avoid leaks in tests.
+ 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
+ release_reference_in_close_ = false;
+ }
}
awakable_list_.AwakeForStateChange(GetHandleSignalsStateImplNoLock());
started_transport_.Release();
} else {
// We must be waiting to call ReleaseHandle. It will call Shutdown.
}
+
+ if (call_release)
+ Release();
}
MojoResult MessagePipeDispatcher::AttachTransportsNoLock(
« content/app/mojo/mojo_init.cc ('K') | « mojo/edk/system/message_pipe_dispatcher.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698