From 5f4535a97c478036533516081597cecbe3259aec Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 2 Nov 2019 20:28:41 +0100 Subject: [PATCH] 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 Reviewed-By: Colin Ihrig Reviewed-By: Franziska Hinkelmann Reviewed-By: Daniel Bevenius --- doc/api/addons.md | 2 +- src/env.cc | 2 +- test/cctest/test_environment.cc | 29 +++++++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/doc/api/addons.md b/doc/api/addons.md index cf2798c3a43637..08475daa33a420 100644 --- a/doc/api/addons.md +++ b/doc/api/addons.md @@ -1354,10 +1354,10 @@ static void sanity_check(void*) { } void init(Local 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 ff1868e75ecd5a..dd326adf2aa247 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 aabaeb985bf322..4045a7d2a4bba5 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; +}