You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jian Qiu <qi...@cn.ibm.com> on 2015/11/23 06:37:29 UTC
Re: Review Request 38287: Check if the futrue is failed before
dispatch in freeze()
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38287/
-----------------------------------------------------------
(Updated Nov. 23, 2015, 5:37 a.m.)
Review request for mesos, Ian Downes, Jie Yu, and Paul Brett.
Changes
-------
rebase
Bugs: MESOS-3272
https://issues.apache.org/jira/browse/MESOS-3272
Repository: mesos
Description
-------
In CgroupsAnyHierarchyWithCpuMemoryTest.ROOT_CGROUPS_FreezeNonFreezer, the freezer process is terminated in it initilize method, which causes the core dump when cgroups::freezer::freeze() calls dispatch(freezer, &internal::Freezer::freeze).
This check is added in cgroups::freezer::freeze() to avoid calling dispatch if the freezer process is terminated.
Diffs (updated)
-----
src/linux/cgroups.cpp f67633e31884b21e943a06100191d2228a637b1a
Diff: https://reviews.apache.org/r/38287/diff/
Testing
-------
./mesos-tests.sh --gtest_filter="CgroupsAnyHierarchyWithCpuMemoryTest.ROOT_CGROUPS_FreezeNonFreezer" --gtest_repeat=1000 --gtest_break_on_failure
Thanks,
Jian Qiu
Re: Review Request 38287: Check if the futrue is failed before
dispatch in freeze()
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38287/#review108403
-----------------------------------------------------------
See the comment below about the issue with this approach. I will commit a fix without the race.
src/linux/cgroups.cpp (lines 2431 - 2440)
<https://reviews.apache.org/r/38287/#comment167871>
The bug here is that when we pass a Process pointer to a managed=true call to spawn, this is passing ownership and we must not access the pointer any further.
The fix you've proposed here is racy, the future failure may occur at any time. If it occurs between your if condition and the dispatch, this will still crash.
I would suggest instead that we obtain the PID before we spawn, so that we can dispatch using the PID instead of a process pointer.
- Ben Mahler
On Nov. 23, 2015, 5:37 a.m., Jian Qiu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38287/
> -----------------------------------------------------------
>
> (Updated Nov. 23, 2015, 5:37 a.m.)
>
>
> Review request for mesos, Ian Downes, Jie Yu, and Paul Brett.
>
>
> Bugs: MESOS-3272
> https://issues.apache.org/jira/browse/MESOS-3272
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In CgroupsAnyHierarchyWithCpuMemoryTest.ROOT_CGROUPS_FreezeNonFreezer, the freezer process is terminated in it initilize method, which causes the core dump when cgroups::freezer::freeze() calls dispatch(freezer, &internal::Freezer::freeze).
>
> This check is added in cgroups::freezer::freeze() to avoid calling dispatch if the freezer process is terminated.
>
>
> Diffs
> -----
>
> src/linux/cgroups.cpp f67633e31884b21e943a06100191d2228a637b1a
>
> Diff: https://reviews.apache.org/r/38287/diff/
>
>
> Testing
> -------
>
> ./mesos-tests.sh --gtest_filter="CgroupsAnyHierarchyWithCpuMemoryTest.ROOT_CGROUPS_FreezeNonFreezer" --gtest_repeat=1000 --gtest_break_on_failure
>
>
> Thanks,
>
> Jian Qiu
>
>
Re: Review Request 38287: Check if the futrue is failed before
dispatch in freeze()
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38287/#review107558
-----------------------------------------------------------
Patch looks great!
Reviews applied: [38287]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Nov. 23, 2015, 5:37 a.m., Jian Qiu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38287/
> -----------------------------------------------------------
>
> (Updated Nov. 23, 2015, 5:37 a.m.)
>
>
> Review request for mesos, Ian Downes, Jie Yu, and Paul Brett.
>
>
> Bugs: MESOS-3272
> https://issues.apache.org/jira/browse/MESOS-3272
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In CgroupsAnyHierarchyWithCpuMemoryTest.ROOT_CGROUPS_FreezeNonFreezer, the freezer process is terminated in it initilize method, which causes the core dump when cgroups::freezer::freeze() calls dispatch(freezer, &internal::Freezer::freeze).
>
> This check is added in cgroups::freezer::freeze() to avoid calling dispatch if the freezer process is terminated.
>
>
> Diffs
> -----
>
> src/linux/cgroups.cpp f67633e31884b21e943a06100191d2228a637b1a
>
> Diff: https://reviews.apache.org/r/38287/diff/
>
>
> Testing
> -------
>
> ./mesos-tests.sh --gtest_filter="CgroupsAnyHierarchyWithCpuMemoryTest.ROOT_CGROUPS_FreezeNonFreezer" --gtest_repeat=1000 --gtest_break_on_failure
>
>
> Thanks,
>
> Jian Qiu
>
>