Browse Source

Ensure that monitor reference is kept for local pid.

Roberto Ostinelli 5 years ago
parent
commit
3087c64b50
2 changed files with 47 additions and 14 deletions
  1. 11 2
      src/syn_registry.erl
  2. 36 12
      test/syn_registry_SUITE.erl

+ 11 - 2
src/syn_registry.erl

@@ -483,6 +483,15 @@ unregister_on_node(Name) ->
 
 
 -spec add_to_local_table(Name :: any(), Pid :: pid(), Meta :: any(), MonitorRef :: undefined | reference()) -> ok.
 -spec add_to_local_table(Name :: any(), Pid :: pid(), Meta :: any(), MonitorRef :: undefined | reference()) -> ok.
 add_to_local_table(Name, Pid, Meta, MonitorRef) ->
 add_to_local_table(Name, Pid, Meta, MonitorRef) ->
+    %% keep monitor reference if it exists for pid & new monitor is undefined
+    %% (rpc calls from other nodes during conflict resolution may otherwise overwrite this)
+    MonitorRef1 = case MonitorRef of
+        undefined ->
+            find_monitor_for_pid(Pid);
+
+        _ ->
+            MonitorRef
+    end,
     %% remove entry if previous exists
     %% remove entry if previous exists
     case find_registry_tuple_by_name(Name) of
     case find_registry_tuple_by_name(Name) of
         undefined ->
         undefined ->
@@ -492,8 +501,8 @@ add_to_local_table(Name, Pid, Meta, MonitorRef) ->
             ets:delete(syn_registry_by_pid, {OldPid, Name})
             ets:delete(syn_registry_by_pid, {OldPid, Name})
     end,
     end,
     %% overwrite & add
     %% overwrite & add
-    ets:insert(syn_registry_by_name, {Name, Pid, Meta, MonitorRef, node(Pid)}),
-    ets:insert(syn_registry_by_pid, {{Pid, Name}, Meta, MonitorRef, node(Pid)}),
+    ets:insert(syn_registry_by_name, {Name, Pid, Meta, MonitorRef1, node(Pid)}),
+    ets:insert(syn_registry_by_pid, {{Pid, Name}, Meta, MonitorRef1, node(Pid)}),
     ok.
     ok.
 
 
 -spec remove_from_local_table(Name :: any(), Pid :: pid()) -> ok.
 -spec remove_from_local_table(Name :: any(), Pid :: pid()) -> ok.

+ 36 - 12
test/syn_registry_SUITE.erl

@@ -40,7 +40,8 @@
     single_node_register_gen_server/1,
     single_node_register_gen_server/1,
     single_node_callback_on_process_exit/1,
     single_node_callback_on_process_exit/1,
     single_node_ensure_callback_process_exit_is_called_if_process_killed/1,
     single_node_ensure_callback_process_exit_is_called_if_process_killed/1,
-    single_node_monitor_after_registry_crash/1
+    single_node_monitor_after_registry_crash/1,
+    single_node_keep_monitor_reference_for_pid_if_there/1
 ]).
 ]).
 -export([
 -export([
     two_nodes_register_monitor_and_unregister/1,
     two_nodes_register_monitor_and_unregister/1,
@@ -88,9 +89,9 @@
 %% -------------------------------------------------------------------
 %% -------------------------------------------------------------------
 all() ->
 all() ->
     [
     [
-        {group, single_node_process_registration},
-        {group, two_nodes_process_registration},
-        {group, three_nodes_process_registration}
+        {group, single_node_process_registration}
+%%        {group, two_nodes_process_registration},
+%%        {group, three_nodes_process_registration}
     ].
     ].
 
 
 %% -------------------------------------------------------------------
 %% -------------------------------------------------------------------
@@ -108,14 +109,15 @@ all() ->
 groups() ->
 groups() ->
     [
     [
         {single_node_process_registration, [shuffle], [
         {single_node_process_registration, [shuffle], [
-            single_node_register_and_monitor,
-            single_node_register_and_unregister,
-            single_node_registration_errors,
-            single_node_registry_count,
-            single_node_register_gen_server,
-            single_node_callback_on_process_exit,
-            single_node_ensure_callback_process_exit_is_called_if_process_killed,
-            single_node_monitor_after_registry_crash
+%%            single_node_register_and_monitor,
+%%            single_node_register_and_unregister,
+%%            single_node_registration_errors,
+%%            single_node_registry_count,
+%%            single_node_register_gen_server,
+%%            single_node_callback_on_process_exit,
+%%            single_node_ensure_callback_process_exit_is_called_if_process_killed,
+%%            single_node_monitor_after_registry_crash,
+            single_node_keep_monitor_reference_for_pid_if_there
         ]},
         ]},
         {two_nodes_process_registration, [shuffle], [
         {two_nodes_process_registration, [shuffle], [
             two_nodes_register_monitor_and_unregister,
             two_nodes_register_monitor_and_unregister,
@@ -424,6 +426,28 @@ single_node_monitor_after_registry_crash(_Config) ->
     %% retrieve
     %% retrieve
     undefined = syn:whereis(<<"my proc 2">>).
     undefined = syn:whereis(<<"my proc 2">>).
 
 
+single_node_keep_monitor_reference_for_pid_if_there(_Config) ->
+    %% start
+    ok = syn:start(),
+    %% start processes
+    Pid = syn_test_suite_helper:start_process(),
+    Pid2 = syn_test_suite_helper:start_process(),
+    %% register
+    ok = syn:register(<<"my proc">>, Pid),
+    ok = syn:register(<<"my proc 2">>, Pid2),
+    %% get monitor
+    [{<<"my proc">>, Pid, undefined, MonitorRef, _}] = ets:lookup(syn_registry_by_name, <<"my proc">>),
+    %% insert into table and keep reference
+    syn_registry:add_to_local_table(<<"my proc">>, Pid, undefined, undefined),
+    %% check internals
+    [{<<"my proc">>, Pid, undefined, MonitorRef, _}] = ets:lookup(syn_registry_by_name, <<"my proc">>),
+    %% get monitor
+    [{<<"my proc 2">>, Pid2, undefined, MonitorRef2, _}] = ets:lookup(syn_registry_by_name, <<"my proc 2">>),
+    %% insert into table and keep reference
+    syn_registry:add_to_local_table(<<"my proc 3">>, Pid2, undefined, undefined),
+    %% check internals
+    [{<<"my proc 3">>, Pid2, undefined, MonitorRef2, _}] = ets:lookup(syn_registry_by_name, <<"my proc 3">>).
+
 two_nodes_register_monitor_and_unregister(Config) ->
 two_nodes_register_monitor_and_unregister(Config) ->
     %% get slave
     %% get slave
     SlaveNode = proplists:get_value(slave_node, Config),
     SlaveNode = proplists:get_value(slave_node, Config),