mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexander Rojas <alexan...@mesosphere.io>
Subject Re: Review Request 53299: Fixed memory leak in implementation of Future<T>::after().
Date Tue, 13 Dec 2016 12:09:57 GMT


> On Dec. 12, 2016, 9:31 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/tests/future_tests.cpp, line 266
> > <https://reviews.apache.org/r/53299/diff/2/?file=1549125#file1549125line266>
> >
> >     What does the word 'policy' here denote?
> >     Could you pick something more consistent with the other tests?

Sorry, the name came from the original piece of code where I discovered the leak. Through
the many iterations I reduce the code to the most simple way to reproduce the leak but I overlooked
naming.


> On Dec. 12, 2016, 9:31 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/include/process/future.hpp, line 1321
> > <https://reviews.apache.org/r/53299/diff/2/?file=1549124#file1549124line1321>
> >
> >     Can you explain why we're adding this behavior with a comment? It's not immediately
clear to me from the existing code / comment what the implication is of making this association
if the weak future is `none`.

I added a summary comment, but the cause is a little bit more complex than a one line code.
Consider the following code:

```c++
Future<Nothing> future;
{
  future = someFunction().after(Milliseconds(10), [](const Future<Nothing>&) { return
Nothing; });
}
```

In that example, the future returned by `someFunction()` goes out of scope almost as soon
as it appears, but the future returned by the `promised` is still kept.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53299/#review158898
-----------------------------------------------------------


On Oct. 31, 2016, 3:48 p.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53299/
> -----------------------------------------------------------
> 
> (Updated Oct. 31, 2016, 3:48 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-6484
>     https://issues.apache.org/jira/browse/MESOS-6484
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Removes a reference counted pointer that futures kept to themselves
> when using the method `Future<T>::after()`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/future.hpp 175214a9090f8cc8241a81e535c87370102f3011

>   3rdparty/libprocess/src/tests/future_tests.cpp 7c411c7be1849119fe0b070622dbe4488fa11b7a

> 
> Diff: https://reviews.apache.org/r/53299/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message