> On July 9, 2016, 7:21 p.m., Vinod Kone wrote: > > src/master/master.hpp, line 1765 > > <https://reviews.apache.org/r/49844/diff/1/?file=1440270#file1440270line1765> > > > > don't follow why this has to be Owned. > > Anand Mazumdar wrote: > Same reasoning as above. If it's not an `Owned`, it would be destroyed > when the stack allocated object (`Subscriber`) goes out of scope leading to > the `Pipe` getting closed. > > Vinod Kone wrote: > Why is ~Subscriber getting called when the subscriber is removed from the > map a problem? I might be missing something here. > > Anand Mazumdar wrote: > Let me elaborate a bit. > > `~Subscriber()` getting called when the subscriber is removed from the > map is not an issue. Infact, that is the desired behavior. > > The problem however is `~Subscriber()` _also_ getting called for the > temporary objects on the stack and thereby un-intentinally closing the `Pipe` > i.e. > > - https://github.com/apache/mesos/blob/master/src/master/master.cpp#L7595 > - https://github.com/apache/mesos/blob/master/src/master/master.cpp#L7576 > > Vinod Kone wrote: > hmm. I see. my question then would be whether automatically doing > http.close() in ~Subscriber is safe? What is the guarantee that someone won't > come along in the future takes a copy of Subscriber? Should we disallow > copy/assignment ?
Agreed. Explicitly disallowing would be a good idea to avoid such potential gotchas in the future. I would update the patch shortly. - Anand ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49844/#review141411 ----------------------------------------------------------- On July 9, 2016, 3:21 a.m., Anand Mazumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49844/ > ----------------------------------------------------------- > > (Updated July 9, 2016, 3:21 a.m.) > > > Review request for mesos, Vinod Kone and Zhitao Li. > > > Bugs: MESOS-5812 > https://issues.apache.org/jira/browse/MESOS-5812 > > > Repository: mesos > > > Description > ------- > > This FD leak would only surface when running tests. We hold on to > a reference of the `Connection` object in the client so that it is > not destroyed before the connection is active. When running tests, > the IP:Port of libprocess remain the same which means the objects > keep on accumulating. In a real world cluster, we remove the > subscriber upon noticing a _disconnection_ i.e. this means the > socket has already been already closed upstream by Libprocess on > the server side. > > > Diffs > ----- > > src/master/master.hpp 845f2f6103b58e114dc5d50e3fcf70607c92a469 > src/master/master.cpp 79e3d78ba45060bc2f2532fdc3d105d1cc888d0f > > Diff: https://reviews.apache.org/r/49844/diff/ > > > Testing > ------- > > make check (gtest_repeat=1000) no FD leaks > > > Thanks, > > Anand Mazumdar > >
