From eefe659521d81572e4e2b81f35b8dcdc6ca373d2 Mon Sep 17 00:00:00 2001 From: Avi Kivity Date: Wed, 12 Feb 2025 21:11:33 +0200 Subject: [PATCH] reactor: deprecate at_exit() at_exit() tasks are unordered with respect to each other, so are not very useful. Since they are public, we can't remove them outright, but prepare by deprecating the public interface. Internal users are converted to a private interface. An explanation about how to use seastar::app_template is added to the documentation. --- apps/io_tester/io_tester.cc | 2 +- apps/memcached/memcache.cc | 8 ++++---- include/seastar/core/reactor.hh | 18 +++++++++++++++++- src/core/app-template.cc | 2 +- src/core/reactor.cc | 12 ++++++++++-- 5 files changed, 33 insertions(+), 9 deletions(-) diff --git a/apps/io_tester/io_tester.cc b/apps/io_tester/io_tester.cc index 910e0e25e2a..2cc29baa109 100644 --- a/apps/io_tester/io_tester.cc +++ b/apps/io_tester/io_tester.cc @@ -1175,7 +1175,7 @@ int main(int ac, char** av) { } ctx.start(storage, *st_type, reqs, duration).get(); - engine().at_exit([&ctx] { + internal::at_exit([&ctx] { return ctx.stop(); }); std::cout << "Creating initial files..." << std::endl; diff --git a/apps/memcached/memcache.cc b/apps/memcached/memcache.cc index 5d302fb7751..071e7376a73 100644 --- a/apps/memcached/memcache.cc +++ b/apps/memcached/memcache.cc @@ -1454,10 +1454,10 @@ int main(int ac, char** av) { ; return app.run_deprecated(ac, av, [&] { - engine().at_exit([&] { return tcp_server.stop(); }); - engine().at_exit([&] { return udp_server.stop(); }); - engine().at_exit([&] { return cache_peers.stop(); }); - engine().at_exit([&] { return system_stats.stop(); }); + internal::at_exit([&] { return tcp_server.stop(); }); + internal::at_exit([&] { return udp_server.stop(); }); + internal::at_exit([&] { return cache_peers.stop(); }); + internal::at_exit([&] { return system_stats.stop(); }); auto&& config = app.configuration(); uint16_t port = config["port"].as(); diff --git a/include/seastar/core/reactor.hh b/include/seastar/core/reactor.hh index aedb00bab68..9017d031e2e 100644 --- a/include/seastar/core/reactor.hh +++ b/include/seastar/core/reactor.hh @@ -132,6 +132,8 @@ void increase_thrown_exceptions_counter() noexcept; template void at_destroy(Func&& func); +void at_exit(noncopyable_function ()> func); + } class io_queue; @@ -167,6 +169,7 @@ private: class execution_stage_pollfn; template friend void internal::at_destroy(Func&&); + friend void internal::at_exit(noncopyable_function ()> func); friend class manual_clock; friend class file_data_source_impl; // for fstream statistics friend class internal::reactor_stall_sampler; @@ -528,6 +531,18 @@ public: return _stop_requested.wait(timeout, [this] { return _stopping; }); } + /// Deprecated. Use following sequence instead: + /// + /// ``` + /// return seastar::app_template::run([] { + /// return seastar::async([] { + /// // Since the function runs in a thread, it can wait for futures using + /// // future::get(). + /// auto deferred_task = defer(/* the function you want to run at exit */); + /// }); + /// }); + /// ``` + [[deprecated("Use seastar::app_template::run(), seastar::async(), and seastar::defer() for orderly shutdown")]] void at_exit(noncopyable_function ()> func); /// Deprecated. Use following sequence instead: @@ -545,7 +560,8 @@ public: do_at_destroy(std::forward(func)); } private: - template + void do_at_exit(noncopyable_function ()> func); +template void do_at_destroy(Func&& func) { _at_destroy_tasks->_q.push_back(make_task(default_scheduling_group(), std::forward(func))); } diff --git a/src/core/app-template.cc b/src/core/app-template.cc index af76bda3721..ff5189fd6d7 100644 --- a/src/core/app-template.cc +++ b/src/core/app-template.cc @@ -166,7 +166,7 @@ int app_template::run(int ac, char ** av, std::function ()>&& func) noexcept { return run_deprecated(ac, av, [func = std::move(func)] () mutable { auto func_done = make_lw_shared>(); - engine().at_exit([func_done] { return func_done->get_future(); }); + internal::at_exit([func_done] { return func_done->get_future(); }); // No need to wait for this future. // func's returned exit_code is communicated via engine().exit() (void)futurize_invoke(func).finally([func_done] { diff --git a/src/core/reactor.cc b/src/core/reactor.cc index 0fa5e23df4c..db6559b734a 100644 --- a/src/core/reactor.cc +++ b/src/core/reactor.cc @@ -2426,11 +2426,19 @@ void reactor::del_timer(timer* tmr) noexcept { _manual_timers.remove(*tmr, _expired_manual_timers); } -void reactor::at_exit(noncopyable_function ()> func) { +void reactor::do_at_exit(noncopyable_function ()> func) { SEASTAR_ASSERT(!_stopping); _exit_funcs.push_back(std::move(func)); } +void reactor::at_exit(noncopyable_function ()> func) { + do_at_exit(std::move(func)); +} + +void internal::at_exit(noncopyable_function ()> func) { + engine().do_at_exit(std::move(func)); +} + future<> reactor::run_exit_tasks() { _stop_requested.broadcast(); stop_aio_eventfd_loop(); @@ -2954,7 +2962,7 @@ void reactor::start_aio_eventfd_loop() { }); }); // must use make_lw_shared, because at_exit expects a copyable function - at_exit([loop_done = make_lw_shared(std::move(loop_done))] { + do_at_exit([loop_done = make_lw_shared(std::move(loop_done))] { return std::move(*loop_done); }); }