From 90549abec7e6ebf7a6af570ea159b6eb7e7a36bf Mon Sep 17 00:00:00 2001
From: Zhongwen Deng <zhongwencool@gmail.com>
Date: Wed, 10 May 2023 14:49:34 +0800
Subject: [PATCH] fix: crash when sysmon.os.mem_check_interval = disabled

---
 apps/emqx/src/emqx_os_mon.erl        | 17 +++---------
 apps/emqx/test/emqx_os_mon_SUITE.erl | 39 ++++++++++++++++++----------
 2 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/apps/emqx/src/emqx_os_mon.erl b/apps/emqx/src/emqx_os_mon.erl
index 4810798eb..144d2bfe5 100644
--- a/apps/emqx/src/emqx_os_mon.erl
+++ b/apps/emqx/src/emqx_os_mon.erl
@@ -23,8 +23,6 @@
 -export([start_link/0]).
 
 -export([
-    get_mem_check_interval/0,
-    set_mem_check_interval/1,
     get_sysmem_high_watermark/0,
     set_sysmem_high_watermark/1,
     get_procmem_high_watermark/0,
@@ -46,6 +44,9 @@
     terminate/2,
     code_change/3
 ]).
+-ifdef(TEST).
+-export([is_sysmem_check_supported/0]).
+-endif.
 
 -include("emqx.hrl").
 
@@ -61,14 +62,6 @@ update(OS) ->
 %% API
 %%--------------------------------------------------------------------
 
-get_mem_check_interval() ->
-    memsup:get_check_interval().
-
-set_mem_check_interval(Seconds) when Seconds < 60000 ->
-    memsup:set_check_interval(1);
-set_mem_check_interval(Seconds) ->
-    memsup:set_check_interval(Seconds div 60000).
-
 get_sysmem_high_watermark() ->
     gen_server:call(?OS_MON, ?FUNCTION_NAME, infinity).
 
@@ -103,11 +96,9 @@ init_os_monitor() ->
 init_os_monitor(OS) ->
     #{
         sysmem_high_watermark := SysHW,
-        procmem_high_watermark := PHW,
-        mem_check_interval := MCI
+        procmem_high_watermark := PHW
     } = OS,
     set_procmem_high_watermark(PHW),
-    set_mem_check_interval(MCI),
     ok = update_mem_alarm_status(SysHW),
     SysHW.
 
diff --git a/apps/emqx/test/emqx_os_mon_SUITE.erl b/apps/emqx/test/emqx_os_mon_SUITE.erl
index 0538d949a..e76928114 100644
--- a/apps/emqx/test/emqx_os_mon_SUITE.erl
+++ b/apps/emqx/test/emqx_os_mon_SUITE.erl
@@ -43,8 +43,8 @@ init_per_testcase(t_cpu_check_alarm, Config) ->
     {ok, _} = supervisor:restart_child(emqx_sys_sup, emqx_os_mon),
     Config;
 init_per_testcase(t_sys_mem_check_alarm, Config) ->
-    case os:type() of
-        {unix, linux} ->
+    case emqx_os_mon:is_sysmem_check_supported() of
+        true ->
             SysMon = emqx_config:get([sysmon, os], #{}),
             emqx_config:put([sysmon, os], SysMon#{
                 sysmem_high_watermark => 0.51,
@@ -54,7 +54,7 @@ init_per_testcase(t_sys_mem_check_alarm, Config) ->
             ok = supervisor:terminate_child(emqx_sys_sup, emqx_os_mon),
             {ok, _} = supervisor:restart_child(emqx_sys_sup, emqx_os_mon),
             Config;
-        _ ->
+        false ->
             Config
     end;
 init_per_testcase(_, Config) ->
@@ -63,12 +63,6 @@ init_per_testcase(_, Config) ->
     Config.
 
 t_api(_) ->
-    ?assertEqual(60000, emqx_os_mon:get_mem_check_interval()),
-    ?assertEqual(ok, emqx_os_mon:set_mem_check_interval(30000)),
-    ?assertEqual(60000, emqx_os_mon:get_mem_check_interval()),
-    ?assertEqual(ok, emqx_os_mon:set_mem_check_interval(122000)),
-    ?assertEqual(120000, emqx_os_mon:get_mem_check_interval()),
-
     ?assertEqual(0.7, emqx_os_mon:get_sysmem_high_watermark()),
     ?assertEqual(ok, emqx_os_mon:set_sysmem_high_watermark(0.8)),
     ?assertEqual(0.8, emqx_os_mon:get_sysmem_high_watermark()),
@@ -86,12 +80,29 @@ t_api(_) ->
     gen_server:stop(emqx_os_mon),
     ok.
 
+t_sys_mem_check_disable(Config) ->
+    case emqx_os_mon:is_sysmem_check_supported() of
+        true -> do_sys_mem_check_disable(Config);
+        false -> skip
+    end.
+
+do_sys_mem_check_disable(_Config) ->
+    MemRef0 = maps:get(mem_time_ref, sys:get_state(emqx_os_mon)),
+    ?assertEqual(true, is_reference(MemRef0), MemRef0),
+    emqx_config:put([sysmon, os, mem_check_interval], 1000),
+    emqx_os_mon:update(emqx_config:get([sysmon, os])),
+    MemRef1 = maps:get(mem_time_ref, sys:get_state(emqx_os_mon)),
+    ?assertEqual(true, is_reference(MemRef1), {MemRef0, MemRef1}),
+    ?assertNotEqual(MemRef0, MemRef1),
+    emqx_config:put([sysmon, os, mem_check_interval], disabled),
+    emqx_os_mon:update(emqx_config:get([sysmon, os])),
+    ?assertEqual(undefined, maps:get(mem_time_ref, sys:get_state(emqx_os_mon))),
+    ok.
+
 t_sys_mem_check_alarm(Config) ->
-    case os:type() of
-        {unix, linux} ->
-            do_sys_mem_check_alarm(Config);
-        _ ->
-            skip
+    case emqx_os_mon:is_sysmem_check_supported() of
+        true -> do_sys_mem_check_alarm(Config);
+        false -> skip
     end.
 
 do_sys_mem_check_alarm(_Config) ->