src: make AtExit() callbacks run in reverse order

This makes the actual behaviour match the documented (and arguably the correct) behaviour. PR-URL: #30230 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Franziska Hinkelmann <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
nodejs · Nov 17, 2019 · 5f4535a · 5f4535a
1 parent 44968f0
commit 5f4535a
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 2 deletions.
diff --git 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
@@ -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
@@ -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;
+}