----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71136/#review216791 -----------------------------------------------------------
src/common/future_tracker.hpp Lines 67-68 (original), 73-74 (patched) <https://reviews.apache.org/r/71136/#comment304015> could we consider to remove .onDiscard()? I believe the registered future or other futures chained in the containerizer already have onDiscard handler, which means that the future will somehow transit to terminated state or a terminated state will be propagated back to this future if someone calls .discard() or from a discard is called by a HTTP disconnection. If that is the case, .onAny + .onAbandoned should be sufficient. wdyt? - Gilbert Song On July 22, 2019, 8:51 a.m., Andrei Budnik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71136/ > ----------------------------------------------------------- > > (Updated July 22, 2019, 8:51 a.m.) > > > Review request for mesos, Benjamin Mahler, Gilbert Song, Meng Zhu, and Qian > Zhang. > > > Repository: mesos > > > Description > ------- > > Previously, the future tracker subscribed on `onAny`, `onAbandoned`, > `onDiscard` events for the future using callbacks, which accept > the same iterator to the `pending` list. Since above mentioned events > can overlap, the same callback can be called more than once, which > leads to accessing invalidated iterator. This patch fixes the problem > by introducing `erased` flag. > > > Diffs > ----- > > src/common/future_tracker.hpp a3f191a58b7ada58153ca33a8d0409b846faedae > > > Diff: https://reviews.apache.org/r/71136/diff/1/ > > > Testing > ------- > > I was able to reproduce a segfault observed in our CI by launching > `ROOT_LaunchGroupFailure` test in the loop with `stress` running in parallel: > ``` > sudo src/mesos-tests --verbose --gtest_repeat=100 --break_on_failure > --gtest_filter=ROOT_DOCKER_DockerAndMesosContainerizers/DefaultExecutorTest.ROOT_LaunchGroupFailure/0 > > stress -c 16 > ``` > After applying the patch, the test passed successfully. > > > Thanks, > > Andrei Budnik > >
