diff options
| author | viettrungluu@chromium.org <viettrungluu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-04-09 20:42:36 +0000 |
|---|---|---|
| committer | viettrungluu@chromium.org <viettrungluu@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-04-09 20:42:36 +0000 |
| commit | ac7db1bd094ba3036fd4f3d9549e574edb1251e4 (patch) | |
| tree | 9ab0bfffcb6e1d7b7a53802a3acbb190a40a029c /mojo/system/channel.cc | |
| parent | 93daf2c4448c4299f0a847a23ff8ddc4295c2c62 (diff) | |
| download | chromium_src-ac7db1bd094ba3036fd4f3d9549e574edb1251e4.zip chromium_src-ac7db1bd094ba3036fd4f3d9549e574edb1251e4.tar.gz chromium_src-ac7db1bd094ba3036fd4f3d9549e574edb1251e4.tar.bz2 | |
Mojo: Log an error on the remote message pipe leak condition.[1]
Don't enable the DCHECK, since it blows things up. Note that this
doesn't actually fix the bug.
Also fix RemoteMessagePipeTest.Multiplex, which is actually currently
broken[3]. (Though one might think that MessagePipe's destructor would
detect the missing Close()s, one would be wrong: Without closing, the
reference cycle wouldn't get broken (by design[4]), so nothing gets
destroyed. Oops.)
[1] Currently, usually (always?) one side will leak. The problem is that
we (only) detach on receiving a kSubtypeMessagePipePeerClosed
control message. But then we won't send one of our own.[2]
[2] The solution is probably to send an ack for these messages, and
detach on receiving the ack instead. This is tricky since you have
to handle the case when both sides simultaneously send the
peer-closed control message.
[3] That is, when this bug is fixed and the DCHECK enabled, the test
would still blow up.
[4] Possibly a bad design.
R=sky@chromium.org
BUG=360081
Review URL: https://codereview.chromium.org/231483002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@262803 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'mojo/system/channel.cc')
| -rw-r--r-- | mojo/system/channel.cc | 12 |
1 files changed, 10 insertions, 2 deletions
diff --git a/mojo/system/channel.cc b/mojo/system/channel.cc index 5a7e9cd..4b81fea0 100644 --- a/mojo/system/channel.cc +++ b/mojo/system/channel.cc @@ -62,8 +62,16 @@ void Channel::Shutdown() { raw_channel_->Shutdown(); raw_channel_.reset(); - // TODO(vtl): Should I clear |local_id_to_endpoint_info_map_|? Or assert that - // it's empty? + // This should not occur, but it probably mostly results in leaking; + // (Explicitly clearing the |local_id_to_endpoint_info_map_| would likely put + // things in an inconsistent state, which is worse. Note that if the map is + // nonempty, we probably won't be destroyed, since the endpoints have a + // reference to us.) + LOG_IF(ERROR, local_id_to_endpoint_info_map_.empty()) + << "Channel shutting down with endpoints still attached"; + // TODO(vtl): This currently blows up, but the fix will be nontrivial. + // crbug.com/360081 + //DCHECK(local_id_to_endpoint_info_map_.empty()); } MessageInTransit::EndpointId Channel::AttachMessagePipeEndpoint( |