reactor: deprecate at_destroy()

at_destroy() tasks are unordered with respect to each other, so
are not very useful. They also consume a scheduling group.

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.

Author: avikivity

include/seastar/core/reactor.hh

@@ -129,6 +129,9 @@ size_t scheduling_group_count();
 
 void increase_thrown_exceptions_counter() noexcept;
 
+template <typename Func>
+void at_destroy(Func&& func);
+
 }
 
 class io_queue;
@@ -162,6 +165,8 @@ private:
     class io_queue_submission_pollfn;
     class syscall_pollfn;
     class execution_stage_pollfn;
+    template <typename Func>
+    friend void internal::at_destroy(Func&&);
     friend class manual_clock;
     friend class file_data_source_impl; // for fstream statistics
     friend class internal::reactor_stall_sampler;
@@ -526,10 +531,16 @@ public:
     void at_exit(noncopyable_function<future<> ()> func);
 
     template <typename Func>
+    [[deprecated("Use app_template::run() for orderly shutdown")]]
     void at_destroy(Func&& func) {
+        do_at_destroy(std::forward<Func>(func));
+    }
+private:
+    template <typename Func>
+    void do_at_destroy(Func&& func) {
         _at_destroy_tasks->_q.push_back(make_task(default_scheduling_group(), std::forward<Func>(func)));
     }
-
+public:
     task* current_task() const { return _current_task; }
     // If a task wants to resume a different task instead of returning control to the reactor,
     // it should set _current_task to the resumed task.
@@ -688,4 +699,13 @@ inline int hrtimer_signal() {
 
 extern logger seastar_logger;
 
+namespace internal {
+
+template <typename Func>
+void at_destroy(Func&& func) {
+    engine().do_at_destroy(std::forward<Func>(func));
+}
+
+} // namespace internal
+
 }

src/net/net.cc

@@ -246,7 +246,7 @@ device::receive(std::function<future<> (packet)> next_packet) {
 void device::set_local_queue(std::unique_ptr<qp> dev) {
     assert(!_queues[this_shard_id()]);
     _queues[this_shard_id()] = dev.get();
-    engine().at_destroy([dev = std::move(dev)] {});
+    internal::at_destroy([dev = std::move(dev)] {});
 }
 
 

tests/unit/scheduling_group_test.cc

@@ -392,7 +392,8 @@ SEASTAR_THREAD_TEST_CASE(sg_key_constructor_exception_when_creating_new_key) {
 SEASTAR_THREAD_TEST_CASE(sg_create_with_destroy_tasks) {
     struct nada{};
 
-    engine().at_destroy([] {}); // nothing really
+    // at_destroy() functionality is deprecated, but test until removed.
+    internal::at_destroy([] {}); // nothing really
 
     scheduling_group_key_config sg_conf = make_scheduling_group_key_config<nada>();
     scheduling_group_key_create(sg_conf).get();