mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrei Sekretenko <asekrete...@mesosphere.io>
Subject Re: Review Request 70533: Implemented the UPDATE_FRAMEWORK call in the V1 API.
Date Thu, 23 May 2019 12:22:31 GMT


> On May 22, 2019, 7:52 p.m., Benjamin Mahler wrote:
> > src/master/master.cpp
> > Lines 3172-3173 (patched)
> > <https://reviews.apache.org/r/70533/diff/8/?file=2146402#file2146402line3172>
> >
> >     This is undefined behavior (and may crash, this has caught me before), because
`call` is moved in the same expression as the `call.framework_info()` is touched, and C++
doesn't not guarantee the order of evaluation.
> >     
> >     We have to split it out:
> >     
> >     ```
> >       Future<bool> authorized = authorizeFramework(call.framework_info());
> >       
> >       return authorized
> >         .then(defer(self(), &Self::_updateFramework, std::move(call), lambda::_1));
> >     ```

Yeah, and a crash could be a relatively good outcome of UB ;) Many thanks for catching this!


- Andrei


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


On May 22, 2019, 1:49 p.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70533/
> -----------------------------------------------------------
> 
> (Updated May 22, 2019, 1:49 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7258
>     https://issues.apache.org/jira/browse/MESOS-7258
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch is based on the previous implementation attempt:
> https://reviews.apache.org/r/66229/
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 1618794cdb9f44656e697de2abbf6c7e0b6fa50d 
>   src/master/master.hpp 5ad128d00d3cdf0dca89eb637ae196987bdce412 
>   src/master/master.cpp 7c9d9c3d566e29d3f8a5781ee6cdf11973a755e6 
>   src/master/validation.cpp 39f5bfdb39fd6944a3e893b1587e3c2e26818d27 
> 
> 
> Diff: https://reviews.apache.org/r/70533/diff/8/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>


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