mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joseph Wu" <jos...@mesosphere.io>
Subject Re: Review Request 40501: Cleanup a leaked reference to a test process living in the stack.
Date Fri, 04 Dec 2015 18:50:03 GMT


> On Dec. 3, 2015, 7:33 p.m., Ben Mahler wrote:
> > src/tests/fetcher_tests.cpp, lines 285-289
> > <https://reviews.apache.org/r/40501/diff/1/?file=1133067#file1133067line285>
> >
> >     This makes spawning and termination asymmetric! :(
> >     
> >     Please follow the approach done here:
> >     
> >     https://github.com/apache/mesos/blob/0.26.0-rc3/3rdparty/libprocess/src/tests/http_tests.cpp#L64-L117
> 
> Bernd Mathiske wrote:
>     Makes sense, this will be better.

I'll open a separate review to fix this, because this commit wasn't reverted.

See: https://reviews.apache.org/r/40978/


- Joseph


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


On Nov. 19, 2015, 1:51 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40501/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2015, 1:51 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Artem Harutyunyan, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3753
>     https://issues.apache.org/jira/browse/MESOS-3753
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Two of the fetcher tests will spawn a process which is stored in the stack (i.e. local
variable in the test).  `spawn` will store a pointer to the process in libprocess's `ProcessManager`.
 When the test finishes, the process goes out of scope and is therefore lost.  However, the
process is **not** terminated.
> 
> Failing to terminate this process will lead to an infinite loop in `~ProcessManager`,
which is called in `process::finalize`.  In `ProcessManager` 's destructor, we will loop and
try to kill all processes.  The process spawned in the test will be running.  However, since
the pointer lives in the stack, the `ProcessManager` will be unable to find the process and
will thereby be stuck trying to kill a process it cannot find.
> 
> 
> Diffs
> -----
> 
>   src/tests/fetcher_tests.cpp 04079964b3539f555351d1444f3635c64700a1a8 
> 
> Diff: https://reviews.apache.org/r/40501/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> Additional testing:
> 
> Insert a `process::finalize` in `src/test/main.cpp`.  i.e.
> ```
>   // Replace `return RUN_ALL_TESTS();` with this:
>   int ret = RUN_ALL_TESTS();
>   process::finalize();
>   return ret;
> ```
> 
> Then `make check GTEST_FILTER="*FetcherTest.OSNetUriTest*:*FetcherTest.OSNetUriSpaceTest*"`.
> The test program should not stall or segfault or abort in some weird way.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


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