[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13054507#comment-13054507
 ] 

Dheeraj Agrawal commented on ZOOKEEPER-1108:
--------------------------------------------

Not sure if this is a bug or behavior:

When we get a auth response, every time we process any auth_response, we call 
all the auth completions (might be registered by different add_auth_info 
calls). Is that intended? Or should we be calling only the one that the request 
came from? I guess we dont know for which request the response corresponds to? 
If the requests are processed in FIFO and response are got in order then may be 
we can figure out which add_auth info request the response corresponds to.

We never remove entries from the auth_list, is that intended?

Also the logging is misleading. 
{noformat}
   1193     if (rc) {
   1194         LOG_ERROR(("Authentication scheme %s failed. Connection 
closed.",
   1195                    zh->auth_h.auth->scheme));
   1196     }
   1197     else {
   1198         LOG_INFO(("Authentication scheme %s succeeded", 
zh->auth_h.auth->scheme));
   1199     }


If there are multiple auth_info in the auth_list , we always print 
success/failure for ONLY the first one. So if i had two auths for scehmes, ABCD 
and EFGH and my auth scheme EFGH failed, the logs will still say ABCD failed.


> Various bugs in zoo_add_auth in C
> ---------------------------------
>
>                 Key: ZOOKEEPER-1108
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1108
>             Project: ZooKeeper
>          Issue Type: Bug
>          Components: c client
>    Affects Versions: 3.3.3
>            Reporter: Dheeraj Agrawal
>             Fix For: 3.4.0
>
>
> 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.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to