Re: Review Request 42154: Moved push_back to end of removeFramework.

2016-01-11 Thread Ben Mahler

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

Ship it!


Thank you!

- Ben Mahler


On Jan. 11, 2016, 9:41 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42154/
> ---
> 
> (Updated Jan. 11, 2016, 9:41 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the framework being removed was pushed back to the circular
> buffer of completed frameworks as a shared_ptr halfway through the
> computation done to remove a framework from the master. The pointer to
> the framework was then accessed again after this point.
> 
> Pushing a shared_ptr this way is fine so long as the push_pack succeeds
> and the pointer is actually inserted into the buffer. However, with the
> introduction of the new flags to set the size of this buffer via
> max_completed_frameworks, it's possible for the size of this buffer to
> be 0. As such, the circular buffer was taking control of the pointer,
> noticing there was nowhere to push it, and then freeing it.  This causes
> problems since the pointer is then accessed later on.
> 
> This patch moves the insertion of the shared pointer into the circular
> buffer to the end of the removeFramework() call. This change should not
> have any adverse effects.
> 
> Review: https://reviews.apache.org/r/42154
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 
> 
> Diff: https://reviews.apache.org/r/42154/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>



Review Request 42154: Moved push_back to end of removeFramework.

2016-01-11 Thread Kevin Klues

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

Previously, the framework being removed was pushed back to the circular
buffer of completed frameworks as a shared_ptr halfway through the
computation done to remove a framework from the master. The pointer to
the framework was then accessed again after this point.

Pushing a shared_ptr this way is fine so long as the push_pack succeeds
and the pointer is actually inserted into the buffer. However, with the
introduction of the new flags to set the size of this buffer via
max_completed_frameworks, it's possible for the size of this buffer to
be 0. As such, the circular buffer was taking control of the pointer,
noticing there was nowhere to push it, and then freeing it.  This causes
problems since the pointer is then accessed later on.

This patch moves the insertion of the shared pointer into the circular
buffer to the end of the removeFramework() call. This change should not
have any adverse effects.

Review: https://reviews.apache.org/r/42154


Diffs
-

  src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 

Diff: https://reviews.apache.org/r/42154/diff/


Testing
---


Thanks,

Kevin Klues