race condition fix

esl · Jan 20, 2023 · c4e006f · c4e006f
1 parent 699ec5f
commit c4e006f
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 10 deletions.
diff --git a/big_tests/tests/pep_SUITE.erl b/big_tests/tests/pep_SUITE.erl
@@ -258,10 +258,20 @@ send_caps_after_login_test(Config) ->
 
               handle_requested_caps(NodeNS, Bob),
 
-              pubsub_tools:receive_item_notification(
-                Bob, <<"item2">>, {escalus_utils:get_short_jid(Alice), NodeNS}, []),
-
-              [] = escalus_client:peek_stanzas(Bob)
+              Node = {escalus_utils:get_short_jid(Alice), NodeNS},
+              Check = fun(Message) ->
+                              pubsub_tools:check_item_notification(Message, <<"item2">>, Node, [])
+                      end,
+
+              % Presence exchange triggers asynchronous sending of the last published item.
+              % If it happens after Bob sends his caps, he receives item2 twice.
+              Check(escalus_client:wait_for_stanza(Bob)),
+              case escalus_client:peek_stanzas(Bob) of
+                  [Message2] ->
+                      Check(Message2);
+                  [] ->
+                      ok
+              end
         end).
 
 delayed_receive(Config) ->
@@ -276,6 +286,10 @@ delayed_receive_story(Config, Alice, Bob) ->
     [Message] = make_friends(Bob, Alice),
     Node = {escalus_utils:get_short_jid(Alice), NodeNS},
     pubsub_tools:check_item_notification(Message, <<"item2">>, Node, []),
+
+    %% Known issue: without a mutual presence subscription Bob will not receive further items
+    pubsub_tools:publish(Alice, <<"item3">>, {pep, NodeNS}, []),
+    [] = escalus_client:wait_for_stanzas(Bob, 1),
     ok.
 
 delayed_receive_with_sm(Config) ->
@@ -297,21 +311,34 @@ h_ok_after_notify_test(ConfigIn) ->
     Config = escalus_users:update_userspec(ConfigIn, kate, stream_management, true),
     Config1 = set_caps(Config),
     escalus:fresh_story_with_config(Config1, [{alice, 1}, {kate, 1}],
-                                    fun h_ok_after_notify_test_story/3).
+                                    fun h_ok_after_notify_story/3).
 
-h_ok_after_notify_test_story(Config, Alice, Kate) ->
+h_ok_after_notify_story(Config, Alice, Kate) ->
     NodeNS = ?config(node_ns, Config),
     escalus_story:make_all_clients_friends([Alice, Kate]),
 
     pubsub_tools:publish(Alice, <<"item2">>, {pep, NodeNS}, []),
-    pubsub_tools:receive_item_notification(
-      Kate, <<"item2">>, {escalus_utils:get_short_jid(Alice), NodeNS}, []),
+    Node = {escalus_utils:get_short_jid(Alice), NodeNS},
+    Check = fun(Message) ->
+                    pubsub_tools:check_item_notification(Message, <<"item2">>, Node, [])
+            end,
+    Check(escalus_connection:get_stanza(Kate, item2)),
 
     H = escalus_tcp:get_sm_h(Kate#client.rcv_pid),
     escalus:send(Kate, escalus_stanza:sm_ack(H)),
 
     escalus_connection:send(Kate, escalus_stanza:sm_request()),
-    escalus:assert(is_sm_ack, escalus_connection:get_stanza(Kate, stream_mgmt_ack)).
+
+    % Presence exchange triggers asynchronous sending of the last published item.
+    % If it happens after item2 is published, Kate receives it twice.
+    Stanza = escalus_connection:get_stanza(Kate, stream_mgmt_ack_or_item2),
+    case escalus_pred:is_sm_ack(Stanza) of
+        true ->
+            ok;
+        false ->
+            Check(Stanza),
+            escalus:assert(is_sm_ack, escalus_connection:get_stanza(Kate, stream_mgmt_ack))
+    end.
 
 authorize_access_model(Config) ->
     escalus:fresh_story(Config,

diff --git a/src/mod_caps.erl b/src/mod_caps.erl
@@ -441,7 +441,10 @@ feature_request(Acc, LServer, From, Caps, [SubNode | Tail] = SubNodes) ->
                               {ok, TS} -> os:system_time(second) >= TS + (?BAD_HASH_LIFETIME);
                               _ -> true
                           end,
-            F = fun (_From, _To, Acc1, IQReply) ->
+            F = fun (_From, _To, _Acc1, undefined) ->
+                        %% IQ request timed out
+                        feature_response(Acc, undefined, LServer, From, Caps, SubNodes);
+                    (_From, _To, Acc1, IQReply) ->
                         feature_response(Acc1, IQReply, LServer, From, Caps, SubNodes)
                 end,
             case NeedRequest of