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 Wed, 04 Jan 2017 15:01:17 GMT


> On Dec. 14, 2016, 8:45 p.m., Joris Van Remoortere wrote:
> > I ran this with `./libprocess-tests --gtest_filter="*FutureTest*After3*" --gtest_repeat=1000
--gtest_break_on_failure` and ran into the following:
> > 
> > ```
> > Repeating all tests (iteration 412) . . .
> > 
> > Note: Google Test filter = *FutureTest*After3*
> > [==========] Running 1 test from 1 test case.
> > [----------] Global test environment set-up.
> > [----------] 1 test from FutureTest
> > [ RUN      ] FutureTest.After3
> > ../../../3rdparty/libprocess/src/tests/future_tests.cpp:282: Failure
> > Value of: witness.use_count()
> >   Actual: 2
> > Expected: 1
> > *** Aborted at 1481744566 (unix time) try "date -d @1481744566" if you are using
GNU date ***
> > PC: @           0x7a49a6 testing::UnitTest::AddTestPartResult()
> > *** SIGSEGV (@0x0) received by PID 29912 (TID 0x7fa0211707c0) from PID 0; stack
trace: ***
> >     @     0x7fa020d7ad10 (unknown)
> >     @           0x7a49a6 testing::UnitTest::AddTestPartResult()
> >     @           0x7a452e testing::internal::AssertHelper::operator=()
> >     @           0x4bd725 FutureTest_After3_Test::TestBody()
> >     @           0x7e0fc3 testing::internal::HandleSehExceptionsInMethodIfSupported<>()
> >     @           0x7cba71 testing::internal::HandleExceptionsInMethodIfSupported<>()
> >     @           0x7ac7a5 testing::Test::Run()
> >     @           0x7ad40b testing::TestInfo::Run()
> >     @           0x7adb17 testing::TestCase::Run()
> >     @           0x7b51d8 testing::internal::UnitTestImpl::RunAllTests()
> >     @           0x7dda73 testing::internal::HandleSehExceptionsInMethodIfSupported<>()
> >     @           0x7cdbe1 testing::internal::HandleExceptionsInMethodIfSupported<>()
> >     @           0x7b4e95 testing::UnitTest::Run()
> >     @           0x603951 RUN_ALL_TESTS()
> >     @           0x603657 main
> >     @     0x7fa01f229a40 (unknown)
> >     @           0x419019 _start
> > Segmentation fault (core dumped)
> > ```
> > 
> > Can you run this test with repeat enabled and debug this issue?
> 
> Alexander Rojas wrote:
>     I was not that easy to reproduce, but the root of the problem is that `Clock` may
delete the timers after the tests, which causes `use_count` to be different from 1, I have
a fix coming when I have run the test enough times.

So I updated the tests so it doesn't depends on the behavior of a third object to verify the
leak doesn't exist anymore. I haven't been able to verify the root of the failure posted here
but it did occur to me approximatelly once in a million runs. My theory is that there is some
race when deleting the function object used in the callback of `after()`.


- Alexander


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


On Jan. 4, 2017, 3:58 p.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53299/
> -----------------------------------------------------------
> 
> (Updated Jan. 4, 2017, 3:58 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 26bf5851f6562cd73aa4938b3308639144657044

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

> 
> Diff: https://reviews.apache.org/r/53299/diff/
> 
> 
> Testing
> -------
> 
> ```bash
> make check
> # Left to run for at least 3_000_000 iterations.
> ./3rdparty/libprocess/libprocess-tests --gtest_filter="FutureTest.After3" --gtest_repeat=-1
--gtest_break_on_failure
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


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