src: make AtExit() callbacks run in reverse order

This makes the actual behaviour match the documented (and arguably
the correct) behaviour.

PR-URL: https://github.com/nodejs/node/pull/30230
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>

3 files changed, 31 insertions, 2 deletions

diff --git a/doc/api/addons.md b/doc/api/addons.md
index cf2798c3a4..08475daa33 100644
--- a/doc/api/addons.md
+++ b/doc/api/addons.md
@@ -1354,10 +1354,10 @@ static void sanity_check(void*) {
 }
 
 void init(Local<Object> exports) {
+  AtExit(sanity_check);
   AtExit(at_exit_cb2, cookie);
   AtExit(at_exit_cb2, cookie);
   AtExit(at_exit_cb1, exports->GetIsolate());
-  AtExit(sanity_check);
 }
 
 NODE_MODULE(NODE_GYP_MODULE_NAME, init)
diff --git a/src/env.cc b/src/env.cc
index ff1868e75e..dd326adf2a 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -637,7 +637,7 @@ void Environment::RunAtExitCallbacks() {
 }
 
 void Environment::AtExit(void (*cb)(void* arg), void* arg) {
-  at_exit_functions_.push_back(ExitCallback{cb, arg});
+  at_exit_functions_.push_front(ExitCallback{cb, arg});
 }
 
 void Environment::RunAndClearNativeImmediates() {
diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc
index aabaeb985b..4045a7d2a4 100644
--- a/test/cctest/test_environment.cc
+++ b/test/cctest/test_environment.cc
@@ -10,8 +10,12 @@ using node::RunAtExit;
 
 static bool called_cb_1 = false;
 static bool called_cb_2 = false;
+static bool called_cb_ordered_1 = false;
+static bool called_cb_ordered_2 = false;
 static void at_exit_callback1(void* arg);
 static void at_exit_callback2(void* arg);
+static void at_exit_callback_ordered1(void* arg);
+static void at_exit_callback_ordered2(void* arg);
 static std::string cb_1_arg;  // NOLINT(runtime/string)
 
 class EnvironmentTest : public EnvironmentTestFixture {
@@ -20,6 +24,8 @@ class EnvironmentTest : public EnvironmentTestFixture {
     NodeTestFixture::TearDown();
     called_cb_1 = false;
     called_cb_2 = false;
+    called_cb_ordered_1 = false;
+    called_cb_ordered_2 = false;
   }
 };
 
@@ -61,6 +67,19 @@ TEST_F(EnvironmentTest, AtExitWithoutEnvironment) {
   EXPECT_TRUE(called_cb_1);
 }
 
+TEST_F(EnvironmentTest, AtExitOrder) {
+  const v8::HandleScope handle_scope(isolate_);
+  const Argv argv;
+  Env env {handle_scope, argv};
+
+  // Test that callbacks are run in reverse order.
+  AtExit(*env, at_exit_callback_ordered1);
+  AtExit(*env, at_exit_callback_ordered2);
+  RunAtExit(*env);
+  EXPECT_TRUE(called_cb_ordered_1);
+  EXPECT_TRUE(called_cb_ordered_2);
+}
+
 TEST_F(EnvironmentTest, AtExitWithArgument) {
   const v8::HandleScope handle_scope(isolate_);
   const Argv argv;
@@ -134,3 +153,13 @@ static void at_exit_callback1(void* arg) {
 static void at_exit_callback2(void* arg) {
   called_cb_2 = true;
 }
+
+static void at_exit_callback_ordered1(void* arg) {
+  EXPECT_TRUE(called_cb_ordered_2);
+  called_cb_ordered_1 = true;
+}
+
+static void at_exit_callback_ordered2(void* arg) {
+  EXPECT_FALSE(called_cb_ordered_1);
+  called_cb_ordered_2 = true;
+}