> On Aug. 16, 2016, 11:35 p.m., Benjamin Mahler wrote: > > src/zookeeper/zookeeper.cpp, lines 196-220 > > <https://reviews.apache.org/r/51068/diff/1/?file=1472488#file1472488line196> > > > > Promise is now on the stack here, but the asynchronous callbacks > > (voidCompletion, stringCompletion, statCompletion, dataCompletion) need to > > access the promise to satisfy the future. There doesn't appear to have been > > a leak here in that the callbacks (voidCompletion, stringCompletion, > > statCompletion, dataCompletion) delete the promise after satisfying it. Can > > you add more detail as to why you're making this change? > > Aaron Wood wrote: > Hi Ben, thanks for reviewing this patch! Can I ask you the same thing > that I asked in my most recent comment here? > https://github.com/apache/mesos/pull/157 > > I see what you're saying about the callbacks taking care of deleting the > promise when necessary, I missed that the first time around. The callbacks > should be taking the pointer to the promise from the args object that's > allocated on the heap here, right? If that's the case, I'm thinking that > what's happening in the original version is this: > > 1. The promise object is allocated on the heap > 2. The pointer to this object is passed into the copy constructor of the > tuple so that a copy is taken internally in args > 3. The future is returned without deleting the memory for the promise > 4. Later on when one of the callbacks is called and the promise gets > deleted, the promise that actually gets deleted is the copy that was taken in > the tuple which was passed to one of the zookeeper C functions > 5. The original promise that was copied into the tuple via the copy > constructor still lives
Sorry, meant to link directly to the post https://github.com/apache/mesos/pull/157#issuecomment-240482442 - Aaron ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51068/#review145930 ----------------------------------------------------------- On Aug. 17, 2016, 6:21 p.m., Aaron Wood wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51068/ > ----------------------------------------------------------- > > (Updated Aug. 17, 2016, 6:21 p.m.) > > > Review request for mesos. > > > Repository: mesos > > > Description > ------- > > This should prevent any of the promises that are created in the various > ZookeeperProcess class methods from leaking memory. > > > Diffs > ----- > > docs/contributors.yaml 3f06000 > src/zookeeper/zookeeper.cpp e105377 > > Diff: https://reviews.apache.org/r/51068/diff/ > > > Testing > ------- > > make && make check > > > Thanks, > > Aaron Wood > >
