server: fix server lifecycle notifier when callback list is empty

source/server/server.cc

@@ -614,23 +614,21 @@ void InstanceImpl::notifyCallbacksForStage(Stage stage, Event::PostCb completion
     }
   }
 
+  // Wrap completion_cb so that it only gets invoked when all callbacks for this stage
+  // have finished their work.
+  std::shared_ptr<Event::PostCb> cb_guard(new Event::PostCb([] {}),
+                                          [this, completion_cb](Event::PostCb* cb) {
+                                            ASSERT(std::this_thread::get_id() == main_thread_id_);
+                                            completion_cb();
+                                            delete cb;
+                                          });
+
   auto it2 = stage_completable_callbacks_.find(stage);
   if (it2 != stage_completable_callbacks_.end()) {
-    ASSERT(!it2->second.empty());
-    // Wrap completion_cb so that it only gets invoked when all callbacks for this stage
-    // have finished their work.
-    auto completion_cb_count = std::make_shared<int>(it2->second.size());
-    Event::PostCb wrapped_cb = [this, completion_cb, completion_cb_count] {
-      ASSERT(std::this_thread::get_id() == main_thread_id_);
-      if (--*completion_cb_count == 0) {
-        completion_cb();
-      }
-    };
+    ENVOY_LOG(info, "Notifying {} callback(s) with completion.", it2->second.size());
     for (const StageCallbackWithCompletion& callback : it2->second) {
-      callback(wrapped_cb);
+      callback([cb_guard] { (*cb_guard)(); });
     }
-  } else {
-    completion_cb();
   }
 }
 

test/server/server_test.cc

@@ -201,6 +201,27 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, ServerInstanceImplTest,
                          testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),
                          TestUtility::ipTestParamsToString);
 
+TEST_P(ServerInstanceImplTest, EmptyShutdownLifecycleNotifications) {
+  absl::Notification started;
+
+  auto server_thread = Thread::threadFactoryForTest().createThread([&] {
+    initialize("test/server/node_bootstrap.yaml");
+    auto startup_handle = server_->registerCallback(ServerLifecycleNotifier::Stage::Startup,
+                                                    [&] { started.Notify(); });
+    auto shutdown_handle = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit,
+                                                     [&](Event::PostCb) { FAIL(); });
+    shutdown_handle = nullptr; // unregister callback
+    server_->run();
+    startup_handle = nullptr;
+    server_ = nullptr;
+    thread_local_ = nullptr;
+  });
+
+  started.WaitForNotification();
+  server_->dispatcher().post([&] { server_->shutdown(); });
+  server_thread->join();
+}
+
 TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
   bool startup = false, shutdown = false, shutdown_with_completion = false;
   absl::Notification started, shutdown_begin, completion_block, completion_done;