You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2013/10/31 18:44:46 UTC
Review Request 15138: Fixed race condition that caused Authenticatee process
to deadlock.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15138/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Ben Mahler.
Bugs: MESOS-737
https://issues.apache.org/jira/browse/MESOS-737
Repository: mesos-git
Description
-------
See bug for details.
Diffs
-----
src/sched/sched.cpp 3049096a7aeaea6c1063a1bbcef119517daa236c
Diff: https://reviews.apache.org/r/15138/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request 15138: Fixed race condition that caused Authenticatee
process to deadlock.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On Oct. 31, 2013, 5:59 p.m., Benjamin Hindman wrote:
> > src/sched/sched.cpp, line 288
> > <https://reviews.apache.org/r/15138/diff/1/?file=375163#file375163line288>
> >
> > If this defer never gets invoked now the authenticatee will never get deleted causing a memory leak, hence the use of Owned. Also, I don't understand how this causes a deadlock? Finally, to keep people from rewrapping this with Owned you would probably want to a big comment. ;)
>
> Vinod Kone wrote:
> Also not sure about the leak because afaict the only reason the defer wouldn't be called is when the process is exiting.
Exactly. If the SchedulerProcess is exiting then we won't execute the defer which means the pointer will not get deleted, thus a memory leak.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15138/#review27929
-----------------------------------------------------------
On Oct. 31, 2013, 6:24 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15138/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2013, 6:24 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-787
> https://issues.apache.org/jira/browse/MESOS-787
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See bug for details.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 3049096a7aeaea6c1063a1bbcef119517daa236c
>
> Diff: https://reviews.apache.org/r/15138/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 15138: Fixed race condition that caused Authenticatee
process to deadlock.
Posted by Vinod Kone <vi...@gmail.com>.
> On Oct. 31, 2013, 5:59 p.m., Benjamin Hindman wrote:
> > src/sched/sched.cpp, line 288
> > <https://reviews.apache.org/r/15138/diff/1/?file=375163#file375163line288>
> >
> > If this defer never gets invoked now the authenticatee will never get deleted causing a memory leak, hence the use of Owned. Also, I don't understand how this causes a deadlock? Finally, to keep people from rewrapping this with Owned you would probably want to a big comment. ;)
Also not sure about the leak because afaict the only reason the defer wouldn't be called is when the process is exiting.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15138/#review27929
-----------------------------------------------------------
On Oct. 31, 2013, 5:45 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15138/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2013, 5:45 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-787
> https://issues.apache.org/jira/browse/MESOS-787
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See bug for details.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 3049096a7aeaea6c1063a1bbcef119517daa236c
>
> Diff: https://reviews.apache.org/r/15138/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 15138: Fixed race condition that caused Authenticatee
process to deadlock.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15138/#review27929
-----------------------------------------------------------
src/sched/sched.cpp
<https://reviews.apache.org/r/15138/#comment54355>
If this defer never gets invoked now the authenticatee will never get deleted causing a memory leak, hence the use of Owned. Also, I don't understand how this causes a deadlock? Finally, to keep people from rewrapping this with Owned you would probably want to a big comment. ;)
- Benjamin Hindman
On Oct. 31, 2013, 5:45 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15138/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2013, 5:45 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-787
> https://issues.apache.org/jira/browse/MESOS-787
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See bug for details.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 3049096a7aeaea6c1063a1bbcef119517daa236c
>
> Diff: https://reviews.apache.org/r/15138/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 15138: Fixed race condition that caused Authenticatee
process to deadlock.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15138/#review27938
-----------------------------------------------------------
Ship it!
We could likely use Jie's new Shared::upgrade here to delete the pointer in the correct context!
src/sched/sched.cpp
<https://reviews.apache.org/r/15138/#comment54374>
- Ben Mahler
On Oct. 31, 2013, 6:24 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15138/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2013, 6:24 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-787
> https://issues.apache.org/jira/browse/MESOS-787
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See bug for details.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 3049096a7aeaea6c1063a1bbcef119517daa236c
>
> Diff: https://reviews.apache.org/r/15138/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 15138: Fixed race condition that caused Authenticatee
process to deadlock.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15138/
-----------------------------------------------------------
(Updated Nov. 1, 2013, 12:08 a.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
comments. NNFR.
Bugs: MESOS-787
https://issues.apache.org/jira/browse/MESOS-787
Repository: mesos-git
Description
-------
See bug for details.
Diffs (updated)
-----
src/sched/sched.cpp 3049096a7aeaea6c1063a1bbcef119517daa236c
Diff: https://reviews.apache.org/r/15138/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request 15138: Fixed race condition that caused Authenticatee
process to deadlock.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15138/#review27979
-----------------------------------------------------------
Ship it!
If not using Shared -> Owned in the future C++11 rvalue moves might be sufficient too.
src/sched/sched.cpp
<https://reviews.apache.org/r/15138/#comment54430>
How about a new line between these two?
- Benjamin Hindman
On Oct. 31, 2013, 11:09 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15138/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2013, 11:09 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-787
> https://issues.apache.org/jira/browse/MESOS-787
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See bug for details.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 3049096a7aeaea6c1063a1bbcef119517daa236c
>
> Diff: https://reviews.apache.org/r/15138/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 15138: Fixed race condition that caused Authenticatee
process to deadlock.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15138/#review27977
-----------------------------------------------------------
Maybe a TODO to use the new Shared -> Owned upgrade code from Jie that is now committed? We could use that here to avoid storing the pointer as a member variable! With lambdas it would be really clean too ;)
- Ben Mahler
On Oct. 31, 2013, 11:09 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15138/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2013, 11:09 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-787
> https://issues.apache.org/jira/browse/MESOS-787
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See bug for details.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 3049096a7aeaea6c1063a1bbcef119517daa236c
>
> Diff: https://reviews.apache.org/r/15138/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 15138: Fixed race condition that caused Authenticatee
process to deadlock.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15138/
-----------------------------------------------------------
(Updated Oct. 31, 2013, 11:09 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
benm's.
Bugs: MESOS-787
https://issues.apache.org/jira/browse/MESOS-787
Repository: mesos-git
Description
-------
See bug for details.
Diffs (updated)
-----
src/sched/sched.cpp 3049096a7aeaea6c1063a1bbcef119517daa236c
Diff: https://reviews.apache.org/r/15138/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request 15138: Fixed race condition that caused Authenticatee
process to deadlock.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15138/#review27972
-----------------------------------------------------------
src/sched/sched.cpp
<https://reviews.apache.org/r/15138/#comment54423>
delete ignores NULLs
- Ben Mahler
On Oct. 31, 2013, 10:16 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15138/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2013, 10:16 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Ben Mahler.
>
>
> Bugs: MESOS-787
> https://issues.apache.org/jira/browse/MESOS-787
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See bug for details.
>
>
> Diffs
> -----
>
> src/sched/sched.cpp 3049096a7aeaea6c1063a1bbcef119517daa236c
>
> Diff: https://reviews.apache.org/r/15138/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 15138: Fixed race condition that caused Authenticatee
process to deadlock.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15138/
-----------------------------------------------------------
(Updated Oct. 31, 2013, 10:16 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
benh's comments. PTAL.
Bugs: MESOS-787
https://issues.apache.org/jira/browse/MESOS-787
Repository: mesos-git
Description
-------
See bug for details.
Diffs (updated)
-----
src/sched/sched.cpp 3049096a7aeaea6c1063a1bbcef119517daa236c
Diff: https://reviews.apache.org/r/15138/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request 15138: Fixed race condition that caused Authenticatee
process to deadlock.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15138/
-----------------------------------------------------------
(Updated Oct. 31, 2013, 6:24 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
fixed typo.
Bugs: MESOS-787
https://issues.apache.org/jira/browse/MESOS-787
Repository: mesos-git
Description
-------
See bug for details.
Diffs (updated)
-----
src/sched/sched.cpp 3049096a7aeaea6c1063a1bbcef119517daa236c
Diff: https://reviews.apache.org/r/15138/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request 15138: Fixed race condition that caused Authenticatee
process to deadlock.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15138/
-----------------------------------------------------------
(Updated Oct. 31, 2013, 6:22 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
added comment.
Bugs: MESOS-787
https://issues.apache.org/jira/browse/MESOS-787
Repository: mesos-git
Description
-------
See bug for details.
Diffs (updated)
-----
src/sched/sched.cpp 3049096a7aeaea6c1063a1bbcef119517daa236c
Diff: https://reviews.apache.org/r/15138/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request 15138: Fixed race condition that caused Authenticatee
process to deadlock.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15138/
-----------------------------------------------------------
(Updated Oct. 31, 2013, 5:45 p.m.)
Review request for mesos, Benjamin Hindman and Ben Mahler.
Changes
-------
put the right bug.
Bugs: MESOS-787
https://issues.apache.org/jira/browse/MESOS-787
Repository: mesos-git
Description
-------
See bug for details.
Diffs
-----
src/sched/sched.cpp 3049096a7aeaea6c1063a1bbcef119517daa236c
Diff: https://reviews.apache.org/r/15138/diff/
Testing
-------
make check
Thanks,
Vinod Kone