File 6971-erts-Fix-crash-dump-assert-when-heart-is-s... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 6971-erts-Fix-crash-dump-assert-when-heart-is-set.patch of Package erlang

From 8637f8395819caaffd14614a403da207e3be6b39 Mon Sep 17 00:00:00 2001
From: Lukas Larsson <lukas@erlang.org>
Date: Tue, 22 Mar 2022 10:14:00 +0100
Subject: [PATCH] erts: Fix crash dump assert when heart is set

If a crash dump was generated while the VM was supervised
with heart, the debug build would assert because the heart
port was flushed.

This commit moves where erts_is_crash_dumping is set so that
the heart flush works as it should.

More importantly (perhaps) the commit also moves the
erts_thr_progress_fatal_error_block to be earlier so
that multiple threads do not race to create the crash dump.
The thr progress block was incorrectly moved by 63e1d64 when
fixing a problem with suspension of schedulers.
---
 erts/emulator/beam/break.c            | 18 ++++++++---------
 erts/emulator/beam/erl_thr_progress.c |  2 +-
 erts/emulator/test/dump_SUITE.erl     | 29 ++++++++++++++++++++++++---
 3 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/erts/emulator/beam/break.c b/erts/emulator/beam/break.c
index 530dee0746..fa2f77dc7c 100644
--- a/erts/emulator/beam/break.c
+++ b/erts/emulator/beam/break.c
@@ -789,6 +789,15 @@ erl_crash_dump_v(char *file, int line, const char* fmt, va_list args)
     if (ERTS_SOMEONE_IS_CRASH_DUMPING)
 	return;
 
+    /* Order all managed threads to block, this has to be done
+       first to guarantee that this is the only thread to generate
+       crash dump. */
+    erts_thr_progress_fatal_error_block(&tpd_buf);
+
+    /* Allow us to pass certain places without locking... */
+    erts_atomic32_set_mb(&erts_writing_erl_crash_dump, 1);
+    erts_tsd_set(erts_is_crash_dumping_key, (void *) 1);
+
     envsz = sizeof(env);
     /* ERL_CRASH_DUMP_SECONDS not set
      * if we have a heart port, break immediately
@@ -886,11 +895,6 @@ erl_crash_dump_v(char *file, int line, const char* fmt, va_list args)
     time(&now);
     erts_cbprintf(to, to_arg, "=erl_crash_dump:0.5\n%s", ctime(&now));
 
-    /* Order all managed threads to block, this has to be done
-       first to guarantee that this is the only thread to generate
-       crash dump. */
-    erts_thr_progress_fatal_error_block(&tpd_buf);
-
 #ifdef ERTS_SYS_SUSPEND_SIGNAL
     /*
      * We suspend all scheduler threads so that we can dump some
@@ -912,10 +916,6 @@ erl_crash_dump_v(char *file, int line, const char* fmt, va_list args)
 
 #endif
 
-    /* Allow us to pass certain places without locking... */
-    erts_atomic32_set_mb(&erts_writing_erl_crash_dump, 1);
-    erts_tsd_set(erts_is_crash_dumping_key, (void *) 1);
-
     if (file != NULL)
        erts_cbprintf(to, to_arg, "The error occurred in file %s, line %d\n", file, line);
 
diff --git a/erts/emulator/beam/erl_thr_progress.c b/erts/emulator/beam/erl_thr_progress.c
index 1929cee6d2..1faf8a7a0f 100644
--- a/erts/emulator/beam/erl_thr_progress.c
+++ b/erts/emulator/beam/erl_thr_progress.c
@@ -1369,7 +1369,7 @@ erts_thr_progress_fatal_error_block(ErtsThrPrgrData *tmp_tpd_bufp)
 	init_tmp_thr_prgr_data(tpd);
     }
 
-    /* Returns number of threads that have not yes been blocked */
+    /* Returns number of threads that have not yet been blocked */
     return thr_progress_block(tpd, 0);
 }
 
diff --git a/erts/emulator/test/dump_SUITE.erl b/erts/emulator/test/dump_SUITE.erl
index ec451fd35b..150f7331b9 100644
--- a/erts/emulator/test/dump_SUITE.erl
+++ b/erts/emulator/test/dump_SUITE.erl
@@ -24,7 +24,8 @@
 
 -export([all/0, suite/0, init_per_testcase/2, end_per_testcase/2]).
 
--export([signal_abort/1, exiting_dump/1, free_dump/1]).
+-export([signal_abort/1, exiting_dump/1, free_dump/1,
+         heart_dump/1, heart_no_dump/1]).
 
 -export([load/1]).
 
@@ -35,7 +36,7 @@ suite() ->
      {timetrap, {minutes, 2}}].
 
 all() ->
-    [signal_abort, exiting_dump, free_dump].
+    [signal_abort, exiting_dump, free_dump, heart_dump, heart_no_dump].
 
 init_per_testcase(signal_abort, Config) ->
     SO = erlang:system_info(schedulers_online),
@@ -212,6 +213,26 @@ free_dump(Config) when is_list(Config) ->
 
     ok.
 
+%% Test that crash dumping works when heart is used
+heart_dump(Config) ->
+    Dump = filename:join(proplists:get_value(priv_dir, Config),"heart.dump"),
+    {ok, Node} = start_node(Config,"-heart"),
+    true = rpc:call(Node, os, putenv, ["ERL_CRASH_DUMP",Dump]),
+    true = rpc:call(Node, os, putenv, ["ERL_CRASH_DUMP_SECONDS","10"]),
+    rpc:call(Node, erlang, halt, ["dump"]),
+    {ok, _Bin} = get_dump_when_done(Dump),
+    ok.
+
+%% Test that there is no crash dump if heart is used and DUMP_SECONDS is not set
+heart_no_dump(Config) ->
+    Dump = filename:join(proplists:get_value(priv_dir, Config),"heart_no.dump"),
+    {ok, Node} = start_node(Config,"-heart"),
+    true = rpc:call(Node, os, putenv, ["ERL_CRASH_DUMP",Dump]),
+    true = rpc:call(Node, os, unsetenv, ["ERL_CRASH_DUMP_SECONDS"]),
+    rpc:call(Node, erlang, halt, ["dump"]),
+    timer:sleep(1000),
+    {error, enoent} = file:read_file_info(Dump),
+    ok.
 
 get_dump_when_done(Dump) ->
     case file:read_file_info(Dump) of
@@ -234,6 +255,8 @@ get_dump_when_done(Dump, Sz) ->
     end.
 
 start_node(Config) when is_list(Config) ->
+    start_node(Config, "").
+start_node(Config, Extra) when is_list(Config) ->
     Pa = filename:dirname(code:which(?MODULE)),
     Name = list_to_atom(atom_to_list(?MODULE)
                         ++ "-"
@@ -242,4 +265,4 @@ start_node(Config) when is_list(Config) ->
                         ++ integer_to_list(erlang:system_time(second))
                         ++ "-"
                         ++ integer_to_list(erlang:unique_integer([positive]))),
-    test_server:start_node(Name, slave, [{args, "-pa "++Pa}]).
+    test_server:start_node(Name, slave, [{args, "-pa "++Pa++" "++Extra}]).
-- 
2.34.1

Places

File 6971-erts-Fix-crash-dump-assert-when-heart-is-set.patch of Package erlang

Places