From 950d5edc4143532977e785b6db5626126de38e33 Mon Sep 17 00:00:00 2001
From: Serge Tupchii <serge.tupchii@protonmail.com>
Date: Fri, 14 Jul 2023 14:22:43 +0300
Subject: [PATCH] fix: avoid logging unnecessary errors in async cleanup
 functions

Cleanup functions that access ETS tables may fail with `badarg` error during EMQX shutdown.
They are called asynchronously by `emqx_pool` workers and accessed ETS tables
may be already destroyed as their owners are shut down.
This fix catches ETS `badarg` errors before they can be caught and logged by `emqx_pool`.

Fixes: EMQX-9992
---
 apps/emqx/src/emqx_broker_helper.erl      | 22 +++++++++++++---------
 apps/emqx/src/emqx_cm.erl                 |  6 +++++-
 apps/emqx_gateway/src/emqx_gateway_cm.erl |  6 +++++-
 changes/ce/fix-11065.en.md                |  1 +
 4 files changed, 24 insertions(+), 11 deletions(-)
 create mode 100644 changes/ce/fix-11065.en.md

diff --git a/apps/emqx/src/emqx_broker_helper.erl b/apps/emqx/src/emqx_broker_helper.erl
index 06f249678..ea615c2f7 100644
--- a/apps/emqx/src/emqx_broker_helper.erl
+++ b/apps/emqx/src/emqx_broker_helper.erl
@@ -153,13 +153,17 @@ code_change(_OldVsn, State, _Extra) ->
 %%--------------------------------------------------------------------
 
 clean_down(SubPid) ->
-    case ets:lookup(?SUBMON, SubPid) of
-        [{_, SubId}] ->
-            true = ets:delete(?SUBMON, SubPid),
-            true =
-                (SubId =:= undefined) orelse
-                    ets:delete_object(?SUBID, {SubId, SubPid}),
-            emqx_broker:subscriber_down(SubPid);
-        [] ->
-            ok
+    try
+        case ets:lookup(?SUBMON, SubPid) of
+            [{_, SubId}] ->
+                true = ets:delete(?SUBMON, SubPid),
+                true =
+                    (SubId =:= undefined) orelse
+                        ets:delete_object(?SUBID, {SubId, SubPid}),
+                emqx_broker:subscriber_down(SubPid);
+            [] ->
+                ok
+        end
+    catch
+        error:badarg -> ok
     end.
diff --git a/apps/emqx/src/emqx_cm.erl b/apps/emqx/src/emqx_cm.erl
index c193cea44..40dece5a9 100644
--- a/apps/emqx/src/emqx_cm.erl
+++ b/apps/emqx/src/emqx_cm.erl
@@ -734,7 +734,11 @@ code_change(_OldVsn, State, _Extra) ->
 %%--------------------------------------------------------------------
 
 clean_down({ChanPid, ClientId}) ->
-    do_unregister_channel({ClientId, ChanPid}),
+    try
+        do_unregister_channel({ClientId, ChanPid})
+    catch
+        error:badarg -> ok
+    end,
     ok = ?tp(debug, emqx_cm_clean_down, #{client_id => ClientId}).
 
 stats_fun() ->
diff --git a/apps/emqx_gateway/src/emqx_gateway_cm.erl b/apps/emqx_gateway/src/emqx_gateway_cm.erl
index 4c07d3938..e52c81856 100644
--- a/apps/emqx_gateway/src/emqx_gateway_cm.erl
+++ b/apps/emqx_gateway/src/emqx_gateway_cm.erl
@@ -823,7 +823,11 @@ code_change(_OldVsn, State, _Extra) ->
 do_unregister_channel_task(Items, GwName, CmTabs) ->
     lists:foreach(
         fun({ChanPid, ClientId}) ->
-            do_unregister_channel(GwName, {ClientId, ChanPid}, CmTabs)
+            try
+                do_unregister_channel(GwName, {ClientId, ChanPid}, CmTabs)
+            catch
+                error:badarg -> ok
+            end
         end,
         Items
     ).
diff --git a/changes/ce/fix-11065.en.md b/changes/ce/fix-11065.en.md
new file mode 100644
index 000000000..e5742bfe0
--- /dev/null
+++ b/changes/ce/fix-11065.en.md
@@ -0,0 +1 @@
+Avoid logging irrelevant error messages during EMQX shutdown.