Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 3.3.3
    • Fix Version/s: 3.4.0
    • Component/s: c client
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      3 issues:
      In zoo_add_auth: there is a race condition:
      2940 // ZOOKEEPER-800 zoo_add_auth should return ZINVALIDSTATE if
      2941 // the connection is closed.
      2942 if (zoo_state(zh) == 0)

      { 2943 return ZINVALIDSTATE; 2944 }

      when we do zookeeper_init, the state is initialized to 0 and above we check if state = 0 then throw exception.
      There is a race condition where the doIo thread is slow and has not changed the state to CONNECTING, then you end up returning back ZKINVALIDSTATE.
      The problem is we use 0 for CLOSED state and UNINITIALIZED state. in case of uninitialized case it should let it go through.

      2nd issue:

      Another Bug: in send_auth_info, the check is not correct

      while (auth->next != NULL)

      { //--BUG: in cases where there is only one auth in the list, this will never send that auth, as its next will be NULL rc = send_info_packet(zh, auth); auth = auth->next; }

      FIX IS:
      do

      { rc = send_info_packet(zh, auth); auth = auth->next; }

      while (auth != NULL); //this will make sure that even if there is one auth ,that will get sent.

      3rd issue:
      2965 add_last_auth(&zh->auth_h, authinfo);
      2966 zoo_unlock_auth(zh);
      2967
      2968 if(zh->state == ZOO_CONNECTED_STATE || zh->state == ZOO_ASSOCIATING_STATE)
      2969 return send_last_auth_info(zh);

      if it is connected, we only send the last_auth_info, which may be different than the one we added, as we unlocked it before sending it.

      1. ZOOKEEPER-1108.patch
        4 kB
        Dheeraj Agrawal
      2. ZOOKEEPER-1108.patch
        3 kB
        Dheeraj Agrawal
      3. ZOOKEEPER-1108.patch
        3 kB
        Dheeraj Agrawal
      4. ZOOKEEPER-1108.patch
        3 kB
        Dheeraj Agrawal
      5. ZOOKEEPER-1108.patch
        0.6 kB
        Dheeraj Agrawal

        Activity

        Dheeraj Agrawal created issue -
        Dheeraj Agrawal made changes -
        Field Original Value New Value
        Priority Minor [ 4 ] Major [ 3 ]
        Dheeraj Agrawal made changes -
        Description 2 issues:
        In zoo_add_auth: there is a race condition:
           2940 // [ZOOKEEPER-800] zoo_add_auth should return ZINVALIDSTATE if
           2941 // the connection is closed.
           2942 if (zoo_state(zh) == 0) {
           2943 return ZINVALIDSTATE;
           2944 }
        when we do zookeeper_init, the state is initialized to 0 and above we check if state = 0 then throw exception.
        There is a race condition where the doIo thread is slow and has not changed the state to CONNECTING, then you end up returning back ZKINVALIDSTATE.
        The problem is we use 0 for CLOSED state and UNINITIALIZED state. in case of uninitialized case it should let it go through.

        Also
           2965 add_last_auth(&zh->auth_h, authinfo);
           2966 zoo_unlock_auth(zh);
           2967
           2968 if(zh->state == ZOO_CONNECTED_STATE || zh->state == ZOO_ASSOCIATING_STATE)
           2969 return send_last_auth_info(zh);


        if it is connected, we only send the last_auth_info, which may be different than the one we added, as we unlocked it before sending it.
        3 issues:
        In zoo_add_auth: there is a race condition:
           2940 // [ZOOKEEPER-800] zoo_add_auth should return ZINVALIDSTATE if
           2941 // the connection is closed.
           2942 if (zoo_state(zh) == 0) {
           2943 return ZINVALIDSTATE;
           2944 }
        when we do zookeeper_init, the state is initialized to 0 and above we check if state = 0 then throw exception.
        There is a race condition where the doIo thread is slow and has not changed the state to CONNECTING, then you end up returning back ZKINVALIDSTATE.
        The problem is we use 0 for CLOSED state and UNINITIALIZED state. in case of uninitialized case it should let it go through.

        2nd issue:

        Another Bug: in send_auth_info, the check is not correct

        while (auth->next != NULL) { //--BUG: in cases where there is only one auth in the list, this will never send that auth, as its next will be NULL
           rc = send_info_packet(zh, auth);
           auth = auth->next;
        }

        FIX IS:
        do {
          rc = send_info_packet(zh, auth);
          auth = auth->next;
         } while (auth != NULL); //this will make sure that even if there is one auth ,that will get sent.

        3rd issue:
           2965 add_last_auth(&zh->auth_h, authinfo);
           2966 zoo_unlock_auth(zh);
           2967
           2968 if(zh->state == ZOO_CONNECTED_STATE || zh->state == ZOO_ASSOCIATING_STATE)
           2969 return send_last_auth_info(zh);

        if it is connected, we only send the last_auth_info, which may be different than the one we added, as we unlocked it before sending it.

        Dheeraj Agrawal made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Dheeraj Agrawal made changes -
        Attachment ZOOKEEPER-1108.patch [ 12484681 ]
        Mahadev konar made changes -
        Assignee Dheeraj Agrawal [ dheerajagrawal ]
        Dheeraj Agrawal made changes -
        Attachment ZOOKEEPER-1108.patch [ 12486924 ]
        Dheeraj Agrawal made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Dheeraj Agrawal made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Dheeraj Agrawal made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Dheeraj Agrawal made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Dheeraj Agrawal made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Dheeraj Agrawal made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Dheeraj Agrawal made changes -
        Attachment ZOOKEEPER-1108.patch [ 12489102 ]
        Dheeraj Agrawal made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Dheeraj Agrawal made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Dheeraj Agrawal made changes -
        Priority Major [ 3 ] Blocker [ 1 ]
        Dheeraj Agrawal made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Dheeraj Agrawal made changes -
        Attachment ZOOKEEPER-1108.patch [ 12490123 ]
        Dheeraj Agrawal made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Mahadev konar made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Dheeraj Agrawal made changes -
        Attachment ZOOKEEPER-1108.patch [ 12491680 ]
        Dheeraj Agrawal made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Mahadev konar made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags [Reviewed]
        Resolution Fixed [ 1 ]
        Mahadev konar made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Dheeraj Agrawal
            Reporter:
            Dheeraj Agrawal
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development