You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by "Dheeraj Agrawal (JIRA)" <ji...@apache.org> on 2011/06/23 23:02:47 UTC
[jira] [Created] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in C
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
Priority: Minor
Fix For: 3.4.0
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.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13084360#comment-13084360 ]
Mahadev konar commented on ZOOKEEPER-1108:
------------------------------------------
thanks dheeraj,
Will do today/tommorrow.
> 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
> Assignee: Dheeraj Agrawal
> Priority: Blocker
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Assigned] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mahadev konar reassigned ZOOKEEPER-1108:
----------------------------------------
Assignee: Dheeraj Agrawal
> 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
> Assignee: Dheeraj Agrawal
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Updated] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dheeraj Agrawal updated ZOOKEEPER-1108:
---------------------------------------
Attachment: ZOOKEEPER-1108.patch
> 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
> Assignee: Dheeraj Agrawal
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13067305#comment-13067305 ]
Dheeraj Agrawal commented on ZOOKEEPER-1108:
--------------------------------------------
The attached patch is for issue: where the auth requests (first one) are not sent to server if the client has not yet connected to server (and state has not yet transitioned into ZOO_CONNECTING) , due to bug in send_auth_info. to write a test case for this, i had to fix ZOOKEEPER-1126 first, as without that fix, zoo_add_auth will always return ZKINVALIDSTATE.
I will update and close ZOOKEEPER-1126 (as its patch is also available in this).
> 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
> Assignee: Dheeraj Agrawal
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13076274#comment-13076274 ]
Mahadev konar commented on ZOOKEEPER-1108:
------------------------------------------
Dheeraj, Looks like some really bad failure in the tests. Can you take a look and see?
{code}
[exec] Zookeeper_watchers::testChildWatcher2 : elapsed 103 : OK
[exec] [exec] *** glibc detected *** ./zktest-mt: malloc(): memory corruption: 0x00002affc7a83010 ***
[exec] [exec] ======= Backtrace: =========
[exec] [exec] OK (57)
[exec] [exec] /lib/libc.so.6[0x2affc88c701f]
{code}
> 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
> Assignee: Dheeraj Agrawal
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13085084#comment-13085084 ]
Dheeraj Agrawal commented on ZOOKEEPER-1108:
--------------------------------------------
If we get rid of the new state, then we will have to get rid of the tests as well. As we cant test it without having the new state, as zk_init will return INVALID state in that case.
This new state is not publicly exposed.
We already have a different state in java (NULL) . In C, we are using 0 for both CLOSED state and NOT_CONNECTED state. In java, NOT_CONNECTED state is null.
> 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
> Assignee: Dheeraj Agrawal
> Priority: Blocker
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Updated] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dheeraj Agrawal updated ZOOKEEPER-1108:
---------------------------------------
Priority: Blocker (was: Major)
> 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
> Assignee: Dheeraj Agrawal
> Priority: Blocker
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13082947#comment-13082947 ]
Mahadev konar commented on ZOOKEEPER-1108:
------------------------------------------
Looks like memory corruption in the tests.
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/438/console
Dheeraj, can you please take a look?
> 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
> Assignee: Dheeraj Agrawal
> Priority: Blocker
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13076269#comment-13076269 ]
Dheeraj Agrawal commented on ZOOKEEPER-1108:
--------------------------------------------
Mahadev/Michi, is there anything remaining / pending for me in this patch to be able to be checked in?
We would like this patch to be accepted and included in 3.4
> 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
> Assignee: Dheeraj Agrawal
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13076271#comment-13076271 ]
Dheeraj Agrawal commented on ZOOKEEPER-1108:
--------------------------------------------
Also a note regarding the tests, All tests pass when i run it locally using gcc-3.4.6 and gcc-3.2.3. Not sure why it complained here.
> 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
> Assignee: Dheeraj Agrawal
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Updated] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dheeraj Agrawal updated ZOOKEEPER-1108:
---------------------------------------
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.
was:
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.
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13057455#comment-13057455 ]
Hadoop QA commented on ZOOKEEPER-1108:
--------------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12484681/ZOOKEEPER-1108.patch
against trunk revision 1140017.
+1 @author. The patch does not contain any @author tags.
-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed core unit tests.
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/358//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/358//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/358//console
This message is automatically generated.
> 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
>
> Attachments: ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13076276#comment-13076276 ]
Mahadev konar commented on ZOOKEEPER-1108:
------------------------------------------
Here is the link to console output:
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/400/console
> 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
> Assignee: Dheeraj Agrawal
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13076280#comment-13076280 ]
Dheeraj Agrawal commented on ZOOKEEPER-1108:
--------------------------------------------
Yeah, i tired to run them locally and i didn't see any issues. Let me rerun with a clean checkout. I will update the thread soon.
> 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
> Assignee: Dheeraj Agrawal
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13062096#comment-13062096 ]
Hadoop QA commented on ZOOKEEPER-1108:
--------------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12484681/ZOOKEEPER-1108.patch
against trunk revision 1144087.
+1 @author. The patch does not contain any @author tags.
-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed core unit tests.
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/385//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/385//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/385//console
This message is automatically generated.
> 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
>
> Attachments: ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13084345#comment-13084345 ]
Dheeraj Agrawal commented on ZOOKEEPER-1108:
--------------------------------------------
Mahadev, can you review the patch and check it in , if all looks good to you? All tests are passing now without any issues.
> 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
> Assignee: Dheeraj Agrawal
> Priority: Blocker
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Issue Comment Edited] (ZOOKEEPER-1108) Various bugs in
zoo_add_auth in C
Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13084920#comment-13084920 ]
Mahadev konar edited comment on ZOOKEEPER-1108 at 8/15/11 12:24 AM:
--------------------------------------------------------------------
Dheeraj,
Looks like you added, the new state of NOT_CONNECTED_STATE in the header definitions. That probably needs a little more work, since the java client will need to have similar changes. I'd say we get rid of the new state in this patch! Makes sense?
was (Author: mahadev):
Dheeraj,
Looks like you added, the new state of NOT_CONNECTED_STATE in the header definitions. That probably needs a little more work, since the java client will need to have similar changes? I'd say we get rid of the new state in this patch? Makes sense?
> 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
> Assignee: Dheeraj Agrawal
> Priority: Blocker
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13054502#comment-13054502 ]
Dheeraj Agrawal commented on ZOOKEEPER-1108:
--------------------------------------------
another bug
{noformat}
303 static void mark_active_auth(zhandle_t *zh) {
304 auth_list_head_t auth_h = zh->auth_h;
305 auth_info *element;
306 if (auth_h.auth == NULL) {
307 return;
308 }
309 element = auth_h.auth;
310 while (element->next != NULL) {
311 element->state = 1;
312 element = element->next;
313 }
314 }
315
{no format}
Here as well we are skipping the first node in the list and in case of N nodes , we will skip processing the last one. \\
It should be changed to
{no format}
310 while (element != NULL) {
{no format}
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13078922#comment-13078922 ]
Mahadev konar commented on ZOOKEEPER-1108:
------------------------------------------
Dheeraj,
The pre commit build servers are down. We are waiting for them to come up so that we can get back on track with 3.4 release.
> 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
> Assignee: Dheeraj Agrawal
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13084920#comment-13084920 ]
Mahadev konar commented on ZOOKEEPER-1108:
------------------------------------------
Dheeraj,
Looks like you added, the new state of NOT_CONNECTED_STATE in the header definitions. That probably needs a little more work, since the java client will need to have similar changes? I'd say we get rid of the new state in this patch? Makes sense?
> 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
> Assignee: Dheeraj Agrawal
> Priority: Blocker
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13082886#comment-13082886 ]
Hadoop QA commented on ZOOKEEPER-1108:
--------------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12489102/ZOOKEEPER-1108.patch
against trunk revision 1152141.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 6 new or modified tests.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
-1 core tests. The patch failed core unit tests.
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/438//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/438//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/438//console
This message is automatically generated.
> 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
> Assignee: Dheeraj Agrawal
> Priority: Blocker
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13101117#comment-13101117 ]
Hudson commented on ZOOKEEPER-1108:
-----------------------------------
Integrated in ZooKeeper-trunk #1299 (See [https://builds.apache.org/job/ZooKeeper-trunk/1299/])
ZOOKEEPER-1108. Various bugs in zoo_add_auth in C. (Dheeraj Agrawal via mahadev)
mahadev : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1166970
Files :
* /zookeeper/trunk/CHANGES.txt
* /zookeeper/trunk/src/c/src/zk_adaptor.h
* /zookeeper/trunk/src/c/src/zookeeper.c
* /zookeeper/trunk/src/c/tests/TestClient.cc
* /zookeeper/trunk/src/c/tests/TestZookeeperInit.cc
* /zookeeper/trunk/src/java/main/org/apache/zookeeper/ClientCnxn.java
* /zookeeper/trunk/src/java/main/org/apache/zookeeper/ZooKeeper.java
> 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
> Assignee: Dheeraj Agrawal
> Priority: Blocker
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13067325#comment-13067325 ]
Hadoop QA commented on ZOOKEEPER-1108:
--------------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12486924/ZOOKEEPER-1108.patch
against trunk revision 1146961.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 6 new or modified tests.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
-1 core tests. The patch failed core unit tests.
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/400//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/400//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/400//console
This message is automatically generated.
> 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
> Assignee: Dheeraj Agrawal
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13064845#comment-13064845 ]
Mahadev konar commented on ZOOKEEPER-1108:
------------------------------------------
Dheeraj,
Are you going to open new jiras for the others issues?
> 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
>
> Attachments: ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13083180#comment-13083180 ]
Hadoop QA commented on ZOOKEEPER-1108:
--------------------------------------
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12490123/ZOOKEEPER-1108.patch
against trunk revision 1156649.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 6 new or modified tests.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed core unit tests.
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/452//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/452//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/452//console
This message is automatically generated.
> 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
> Assignee: Dheeraj Agrawal
> Priority: Blocker
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Updated] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dheeraj Agrawal updated ZOOKEEPER-1108:
---------------------------------------
Attachment: ZOOKEEPER-1108.patch
Added test case and fix for #1126
> 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
> Assignee: Dheeraj Agrawal
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Camille Fournier (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13085730#comment-13085730 ]
Camille Fournier commented on ZOOKEEPER-1108:
---------------------------------------------
Mahadev, what do you say? We would like to get this into 3.4 since it causes us a lot of grief with the auth in the C client. I think Dheeraj's point is good, and the Java and C clients are already divergent enough in this manner that this shouldn't be an issue.
> 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
> Assignee: Dheeraj Agrawal
> Priority: Blocker
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13054135#comment-13054135 ]
Dheeraj Agrawal commented on ZOOKEEPER-1108:
--------------------------------------------
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.
> 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
>
>
> 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.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13078920#comment-13078920 ]
Dheeraj Agrawal commented on ZOOKEEPER-1108:
--------------------------------------------
I did a fresh checkout and ran the tests through ant and all tests pass.
[exec] Zookeeper_watchers::testObjectSessionWatcher2 : elapsed 63 : OK
[exec] Zookeeper_watchers::testNodeWatcher1 : elapsed 60 : OK
[exec] Zookeeper_watchers::testChildWatcher1 : elapsed 107 : OK
[exec] Zookeeper_watchers::testChildWatcher2 : elapsed 109 : OK
[exec] OK (56)
BUILD SUCCESSFUL
Can you re-trigger the build/test for this patch? I tried to trigger it by re-uploading the patch, but that didn't help.
> 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
> Assignee: Dheeraj Agrawal
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Updated] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dheeraj Agrawal updated ZOOKEEPER-1108:
---------------------------------------
Priority: Major (was: Minor)
> 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
>
>
> 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.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13067211#comment-13067211 ]
Dheeraj Agrawal commented on ZOOKEEPER-1108:
--------------------------------------------
Opened following 2 trackers for the outstanding issues.
https://issues.apache.org/jira/browse/ZOOKEEPER-1126
https://issues.apache.org/jira/browse/ZOOKEEPER-1127
> 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
> Assignee: Dheeraj Agrawal
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13064876#comment-13064876 ]
Dheeraj Agrawal commented on ZOOKEEPER-1108:
--------------------------------------------
I can.. what do you recommend. we can check in this 2 bug fixes and resolve this and i can open another tracker for outstanding issues.
> 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
> Assignee: Dheeraj Agrawal
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13100881#comment-13100881 ]
Hadoop QA commented on ZOOKEEPER-1108:
--------------------------------------
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12491680/ZOOKEEPER-1108.patch
against trunk revision 1165443.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 6 new or modified tests.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed core unit tests.
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/519//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/519//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/519//console
This message is automatically generated.
> 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
> Assignee: Dheeraj Agrawal
> Priority: Blocker
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Updated] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dheeraj Agrawal updated ZOOKEEPER-1108:
---------------------------------------
Attachment: ZOOKEEPER-1108.patch
> 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
> Assignee: Dheeraj Agrawal
> Priority: Blocker
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Commented] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in
C
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13091256#comment-13091256 ]
Hadoop QA commented on ZOOKEEPER-1108:
--------------------------------------
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12491680/ZOOKEEPER-1108.patch
against trunk revision 1159929.
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 6 new or modified tests.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed core unit tests.
+1 contrib tests. The patch passed contrib unit tests.
Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/474//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/474//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/474//console
This message is automatically generated.
> 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
> Assignee: Dheeraj Agrawal
> Priority: Blocker
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Updated] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dheeraj Agrawal updated ZOOKEEPER-1108:
---------------------------------------
Attachment: ZOOKEEPER-1108.patch
> 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
>
> Attachments: ZOOKEEPER-1108.patch
>
>
> 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
[jira] [Updated] (ZOOKEEPER-1108) Various bugs in zoo_add_auth in C
Posted by "Dheeraj Agrawal (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dheeraj Agrawal updated ZOOKEEPER-1108:
---------------------------------------
Attachment: ZOOKEEPER-1108.patch
Adding java changes to the patch
> 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
> Assignee: Dheeraj Agrawal
> Priority: Blocker
> Fix For: 3.4.0
>
> Attachments: ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch, ZOOKEEPER-1108.patch
>
>
> 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