Skip to content

Commit

Permalink
[Mojo] Eliminate leak in InterfacePtrSet
Browse files Browse the repository at this point in the history
InterfacePtrSet relies on its ELements to delete themselves. In turn
they rely on receiving a connection error callback from their
InterfacePtr. However, when InterfacePtrSet is destroyed it calls
Element::Close() on any live Elements. Element::Close() resets the
InterfacePtr, meaning that that Entry will *not* receive a connection
error callback from that InterfacePtr at any point in the future and
so will leak.

This CL eliminates the leak by having Element::Close() delete the
Element.

Review-Url: https://codereview.chromium.org/2471033002
Cr-Commit-Position: refs/heads/master@{#429168}
  • Loading branch information
colinblundell authored and Commit bot committed Nov 2, 2016
1 parent d850b4e commit c08c515
Showing 1 changed file with 11 additions and 1 deletion.
12 changes: 11 additions & 1 deletion mojo/public/cpp/bindings/interface_ptr_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@
namespace mojo {
namespace internal {

// TODO(blundell): This class should be rewritten to be structured
// similarly to BindingSet if possible, with PtrSet owning its
// Elements and those Elements calling back into PtrSet on connection
// error.
template <typename Interface, template <typename> class Ptr>
class PtrSet {
public:
Expand Down Expand Up @@ -55,7 +59,13 @@ class PtrSet {

~Element() {}

void Close() { ptr_.reset(); }
void Close() {
ptr_.reset();

// Resetting the interface ptr means that it won't call this object back
// on connection error anymore, so this object must delete itself now.
DeleteElement(this);
}

Interface* get() { return ptr_.get(); }

Expand Down

0 comments on commit c08c515

Please sign in to comment.