mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 70533: Implemented the UPDATE_FRAMEWORK call in the V1 API.
Date Wed, 22 May 2019 19:52:40 GMT

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


Fix it, then Ship it!




Nice and clean patch!

There's a bug below, but I'll get this fixed and committed shortly.


src/master/master.cpp
Lines 3172-3173 (patched)
<https://reviews.apache.org/r/70533/#comment302184>

    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));
    ```


- Benjamin Mahler


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