Skip to content

Commit

Permalink
coroutine: preserve this->container before calling dtor
Browse files Browse the repository at this point in the history
in `intermediate_task`, we intend to destroy the task right after
extracting the value from it, and then call `awaiter.process()`.
but GCC-14 warns at seeing this:

```
FAILED: tests/unit/CMakeFiles/test_unit_coroutines.dir/coroutines_test.cc.o
ccache /usr/bin/g++ -DBOOST_ALL_DYN_LINK -DFMT_SHARED -DSEASTAR_API_LEVEL=7 -DSEASTAR_DEFERRED_ACTION_REQUIRE_NOEXCEPT -DSEASTAR_HAS_MEMBARRIER -DSEASTAR_HAVE_ASAN_FIBER_SUPPORT -DSEASTAR_HAVE_HWLOC -DSEASTAR_HAVE_NUMA -DSEASTAR_HAVE_SYSTEMTAP_SDT -DSEASTAR_HAVE_URING -DSEASTAR_LOGGER_COMPILE_TIME_FMT -DSEASTAR_LOGGER_TYPE_STDOUT -DSEASTAR_SCHEDULING_GROUPS_COUNT=16 -DSEASTAR_SSTRING -DSEASTAR_TESTING_MAIN -DSEASTAR_TESTING_WITH_NETWORKING=1 -I/__w/seastar/seastar/tests/unit -I/__w/seastar/seastar/src -I/__w/seastar/seastar/include -I/__w/seastar/seastar/build/release/gen/include -I/__w/seastar/seastar/build/release/gen/src -O2 -g -DNDEBUG -std=gnu++23 -U_FORTIFY_SOURCE -Wno-maybe-uninitialized -Wno-error=unused-result -UNDEBUG -Wall -Werror -Wimplicit-fallthrough -Wdeprecated -Wno-error=deprecated -Wno-error=stringop-overflow -Wno-error=array-bounds -Wdeprecated-declarations -Wno-error=deprecated-declarations -fvisibility=hidden -gz -MD -MT tests/unit/CMakeFiles/test_unit_coroutines.dir/coroutines_test.cc.o -MF tests/unit/CMakeFiles/test_unit_coroutines.dir/coroutines_test.cc.o.d -o tests/unit/CMakeFiles/test_unit_coroutines.dir/coroutines_test.cc.o -c /__w/seastar/seastar/tests/unit/coroutines_test.cc
In file included from /__w/seastar/seastar/tests/unit/coroutines_test.cc:46:
/__w/seastar/seastar/include/seastar/coroutine/all.hh: In member function 'void seastar::coroutine::all<Futures>::intermediate_task<idx>::run_and_dispose() [with long unsigned int idx = 1; Futures = {seastar::future<void>, seastar::future<void>}]':
/__w/seastar/seastar/include/seastar/coroutine/all.hh:138:13: error: '*this.seastar::coroutine::all<seastar::future<void>, seastar::future<void> >::intermediate_task<1>::container' is used uninitialized [-Werror=uninitialized]
  138 |             container.template process<idx+1>();
      |             ^~~~~~~~~
```

it believes that after returning from the destructor of
`intermediate_task`, the instance is in an "uninitialized" state,
so if we reference any of its member variables. we could have
undefined behavior. this is a false alarm, as the destructor does
not change the value of `container` reference. but this could be
still confusing at first glance. so, to silence the warning, let's
preserve the `container` before calling the destructor, and use
the preserved reference to call `awaiter::process()`.

Signed-off-by: Kefu Chai <[email protected]>
  • Loading branch information
tchaikov committed May 17, 2024
1 parent 54567e1 commit 3bd8855
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion include/seastar/coroutine/all.hh
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,9 @@ class [[nodiscard("must co_await an all() object")]] all {
std::get<idx>(container.state._futures) = make_ready_future<value_type>(std::move(this->_state).get());
}
}
awaiter& c = container;
this->~intermediate_task();
container.template process<idx+1>();
c.template process<idx+1>();
}
};
template <typename IndexSequence>
Expand Down

0 comments on commit 3bd8855

Please sign in to comment.