----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64885/#review194634 -----------------------------------------------------------
Ship it! Ship It! src/slave/containerizer/mesos/provisioner/provisioner.cpp Line 568 (original), 568 (patched) <https://reviews.apache.org/r/64885/#comment273513> Thanks for pointing it out now. I like your rule of avoiding default captures in continuation since it makes such issues more readable. I can submit a future patch to capture explicitly if you don't do it this patch. Thanks for fixing this! - Zhitao Li On Jan. 2, 2018, 10:38 a.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64885/ > ----------------------------------------------------------- > > (Updated Jan. 2, 2018, 10:38 a.m.) > > > Review request for mesos, Gilbert Song and Zhitao Li. > > > Repository: mesos > > > Description > ------- > > In `e273efe6976434858edb85bbcf367a02e963a467` we introduced layer > checkpointing to the provisioner, but did not make sure that > continutations making use of internal state of the provisioner actor > installed on futures where always executed in the actor's context. > This was problematic as continuations could be executed while actor > state was changing (data races), or after the the actor had terminated > (use after free). > > In this patch we instead defer execution of continutions to the actor > owning the data. > > This issue was identified with the clang-tidy `mesos-this-capture` > check. > > > Diffs > ----- > > src/slave/containerizer/mesos/provisioner/provisioner.cpp > 61e771872ba2c3f5ecabbe085db102433c7bac3f > > > Diff: https://reviews.apache.org/r/64885/diff/1/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
