From 68b41997777652cf5664a18f45caf4da2677a7bb Mon Sep 17 00:00:00 2001 From: Lukas Larsson Date: Wed, 27 Apr 2022 09:19:10 +0200 Subject: erts: Fix gc to fetch sig queue at mqd change When a process is changing from on heap to off heap message queue data, there may still be messages in the external queue with data on the heap. So we need to move all signals in the external queue to the internal queue before doing the GC. Closes #5933 --- erts/emulator/beam/erl_gc.c | 10 +++---- erts/emulator/test/message_queue_data_SUITE.erl | 37 +++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/erts/emulator/beam/erl_gc.c b/erts/emulator/beam/erl_gc.c index 7456686f12..544f2b9330 100644 --- a/erts/emulator/beam/erl_gc.c +++ b/erts/emulator/beam/erl_gc.c @@ -431,7 +431,7 @@ erts_gc_after_bif_call_lhf(Process* p, ErlHeapFragment *live_hf_end, return result; } - if (p->sig_qs.flags & FS_ON_HEAP_MSGQ) { + if (p->sig_qs.flags & (FS_ON_HEAP_MSGQ|FS_OFF_HEAP_MSGQ_CHNG)) { erts_proc_lock(p, ERTS_PROC_LOCK_MSGQ); erts_proc_sig_fetch(p); erts_proc_unlock(p, ERTS_PROC_LOCK_MSGQ); @@ -890,7 +890,7 @@ int erts_garbage_collect_nobump(Process* p, int need, Eterm* objv, int nobj, int fcalls) { int reds, reds_left; - if (p->sig_qs.flags & FS_ON_HEAP_MSGQ) { + if (p->sig_qs.flags & (FS_ON_HEAP_MSGQ|FS_OFF_HEAP_MSGQ_CHNG)) { erts_proc_lock(p, ERTS_PROC_LOCK_MSGQ); erts_proc_sig_fetch(p); erts_proc_unlock(p, ERTS_PROC_LOCK_MSGQ); @@ -907,7 +907,7 @@ void erts_garbage_collect(Process* p, int need, Eterm* objv, int nobj) { int reds; - if (p->sig_qs.flags & FS_ON_HEAP_MSGQ) { + if (p->sig_qs.flags & (FS_ON_HEAP_MSGQ|FS_OFF_HEAP_MSGQ_CHNG)) { erts_proc_lock(p, ERTS_PROC_LOCK_MSGQ); erts_proc_sig_fetch(p); erts_proc_unlock(p, ERTS_PROC_LOCK_MSGQ); @@ -1185,7 +1185,7 @@ erts_garbage_collect_literals(Process* p, Eterm* literals, p->flags |= F_NEED_FULLSWEEP; - if (p->sig_qs.flags & FS_ON_HEAP_MSGQ) { + if (p->sig_qs.flags & (FS_ON_HEAP_MSGQ|FS_OFF_HEAP_MSGQ_CHNG)) { erts_proc_lock(p, ERTS_PROC_LOCK_MSGQ); erts_proc_sig_fetch(p); erts_proc_unlock(p, ERTS_PROC_LOCK_MSGQ); @@ -2637,7 +2637,7 @@ setup_rootset(Process *p, Eterm *objv, int nobj, Rootset *rootset) */ #ifdef DEBUG - if (p->sig_qs.flags & FS_ON_HEAP_MSGQ) { + if (p->sig_qs.flags & (FS_ON_HEAP_MSGQ|FS_OFF_HEAP_MSGQ_CHNG)) { ErtsMessage *mp; erts_proc_lock(p, ERTS_PROC_LOCK_MSGQ); /* diff --git a/erts/emulator/test/message_queue_data_SUITE.erl b/erts/emulator/test/message_queue_data_SUITE.erl index 7f0cbdd885..e162cd9cb3 100644 --- a/erts/emulator/test/message_queue_data_SUITE.erl +++ b/erts/emulator/test/message_queue_data_SUITE.erl @@ -21,9 +21,10 @@ -module(message_queue_data_SUITE). -export([all/0, suite/0]). --export([basic/1, process_info_messages/1, total_heap_size/1]). +-export([basic/1, process_info_messages/1, total_heap_size/1, + change_to_off_heap_gc/1]). --export([basic_test/1]). +-export([basic_test/1, id/1]). -include_lib("common_test/include/ct.hrl"). @@ -32,7 +33,8 @@ suite() -> {timetrap, {minutes, 2}}]. all() -> - [basic, process_info_messages, total_heap_size]. + [basic, process_info_messages, total_heap_size, + change_to_off_heap_gc]. %% %% @@ -186,12 +188,41 @@ total_heap_size(_Config) -> ct:log("OffSize = ~p, OffSizeAfter = ~p",[OffSize, OffSizeAfter]), true = OffSize == OffSizeAfter. +%% Test that setting message queue to off_heap works if a GC is triggered +%% as the message queue if moved off heap. See GH-5933 for more details. +%% This testcase will most likely only fail in debug build. +change_to_off_heap_gc(_Config) -> + Msg = {ok, lists:duplicate(20,20)}, + + %% We test that this process can receive a message and when it is still + %% in its external message queue we change the message queue data to + %% off_heap and then GC. + {Pid, Ref} = spawn_monitor( + fun() -> + spinner(1, 10000), + process_flag(message_queue_data, off_heap), + garbage_collect(), + receive {ok, _M} -> ok end + end), + Pid ! Msg, + receive + {'DOWN',Ref,_,_,_} -> + ok + end. + %% %% %% helpers %% %% +%% This spinner needs to make sure that it does not allocate any memory +%% as a GC in here will break the test +spinner(_N, 0) -> ok; +spinner(N, M) -> spinner(?MODULE:id(N) div 1, M - 1). + +id(N) -> N. + start_node(Config, Opts) when is_list(Config), is_list(Opts) -> Pa = filename:dirname(code:which(?MODULE)), Name = list_to_atom(atom_to_list(?MODULE) -- cgit v1.2.1