Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: ejabberd 2.1.7, ejabberd 2.1.8
    • Fix Version/s: ejabberd 2.1.10
    • Component/s: XMPP Compliance
    • Labels:
      None
    • Environment:
      Fedora 13, build from git tag 2.1.8

      Description

      There is a problem in presence management after first time contact goes offline.

      t |The contact | My Roster
      --------------------------------------
      t0 |online | online
      t1 |away | away
      t2 |offline | offline
      t3 |online | online
      t4 |away | online
      t5 |offline | online
      tx |whatever | online

      etc...

      The only way to get presence OK is to restart the stream

      This has been tested with PSI and Archipel (Strophe) as client
      This is not happening in 2.1.6 (build from git 2.1.6 tag)

      1. ejabberd_c2s.patch
        0.8 kB
        Christopher Faulet
      2. ejabberd.log
        629 kB
        Antoine Mercadal

        Expenses

          Activity

          Hide
          capflam Christopher Faulet added a comment -

          Fix update of the c2s state into the function presence_update/3

          Show
          capflam Christopher Faulet added a comment - Fix update of the c2s state into the function presence_update/3
          Hide
          capflam Christopher Faulet added a comment -

          The bug was introduced with the commit 4c8b034874afb421a948f2dedfda669106ce255c in the function presence_update/3.
          I have attached a patch to fix it.

          Show
          capflam Christopher Faulet added a comment - The bug was introduced with the commit 4c8b034874afb421a948f2dedfda669106ce255c in the function presence_update/3. I have attached a patch to fix it.
          Hide
          ekhramtsov ekhramtsov added a comment -

          I don't understand how this patch helps. What is the difference?

          Show
          ekhramtsov ekhramtsov added a comment - I don't understand how this patch helps. What is the difference?
          Hide
          capflam Christopher Faulet added a comment -

          Sorry, I just reported the problem without explanation. My bad

          Before the commit 4c8b034874afb421a948f2dedfda669106ce255c, the last case clause in function presence_update/3 returned the updated state, NewState, by assigning it with the return value of the if statement (L.1735,L.1736).

          L.1663  presence_update(From, Packet, StateData) ->
          L.1664      {xmlelement, _Name, Attrs, _Els} = Packet,
          L.1665      case xml:get_attr_s("type", Attrs) of
            ...
          L.1734          ?DEBUG("from unavail = ~p~n", [FromUnavail]),
          L.1735          NewState =
          L.1736              if
            ...
          L.1767              end,
          L.1768          NewState
          L.1769          end.
          

          In the commit 4c8b034874afb421a948f2dedfda669106ce255c, to factorize the code, the variable NewStateData was introduced before the if statement. But at the wrong place, because NewState is now assigned to NewStateData (L.1735,L.1736) and the return value of the if statement is ignored; especially the result of the function presence_broadcast_first/3, where #state.pres_a is updated.

          L.1663  presence_update(From, Packet, StateData) ->
          L.1664      {xmlelement, _Name, Attrs, _Els} = Packet,
          L.1665      case xml:get_attr_s("type", Attrs) of
            ...
          L.1734          ?DEBUG("from unavail = ~p~n", [FromUnavail]),
          L.1735          NewState =
          L.1736              NewStateData = StateData#state{pres_last = Packet,
          L.1737                                             pres_invis = false,
          L.1738                                             pres_timestamp = Timestamp},
          L.1739              if
            ...
          L.1763              end,
          L.1764          NewState
          L.1765          end.
          

          My patch fix this issue. Another way to fix this bug is to remove the variable NewState and let the if statement returns.

          Show
          capflam Christopher Faulet added a comment - Sorry, I just reported the problem without explanation. My bad Before the commit 4c8b034874afb421a948f2dedfda669106ce255c, the last case clause in function presence_update/3 returned the updated state, NewState , by assigning it with the return value of the if statement ( L.1735,L.1736 ). L.1663 presence_update(From, Packet, StateData) -> L.1664 {xmlelement, _Name, Attrs, _Els} = Packet, L.1665 case xml:get_attr_s("type", Attrs) of ... L.1734 ?DEBUG("from unavail = ~p~n", [FromUnavail]), L.1735 NewState = L.1736 if ... L.1767 end, L.1768 NewState L.1769 end. In the commit 4c8b034874afb421a948f2dedfda669106ce255c, to factorize the code, the variable NewStateData was introduced before the if statement. But at the wrong place, because NewState is now assigned to NewStateData ( L.1735,L.1736 ) and the return value of the if statement is ignored; especially the result of the function presence_broadcast_first/3 , where #state.pres_a is updated. L.1663 presence_update(From, Packet, StateData) -> L.1664 {xmlelement, _Name, Attrs, _Els} = Packet, L.1665 case xml:get_attr_s("type", Attrs) of ... L.1734 ?DEBUG("from unavail = ~p~n", [FromUnavail]), L.1735 NewState = L.1736 NewStateData = StateData#state{pres_last = Packet, L.1737 pres_invis = false, L.1738 pres_timestamp = Timestamp}, L.1739 if ... L.1763 end, L.1764 NewState L.1765 end. My patch fix this issue. Another way to fix this bug is to remove the variable NewState and let the if statement returns.
          Hide
          ekhramtsov ekhramtsov added a comment -

          Oops, indeed, my bad. The patch is applied and pushed.

          Show
          ekhramtsov ekhramtsov added a comment - Oops, indeed, my bad. The patch is applied and pushed.

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development