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

Michael Han edited comment on ZOOKEEPER-1485 at 6/14/16 4:53 AM:
-----------------------------------------------------------------

bq. I didn't see an existing pattern to add test-only code either. Do you have 
any suggestions?
I think zoo_get_current_server could be one existing example of a 'largely for 
testing purposes' code. 

bq. which could cause performance issues with multiple threads making requests 
with different zhandles.
Yeah this was what I thought about. Might not be an issue if the number of 
threads is capped. 

bq. Did you have a specific implementation in mind?
How about this (in C++, could be updated to use gcc intrinsics as well in C):
{code}
#include <atomic>
#include <limits>
using namespace std;

std::atomic<uint32_t> count;

using namespace std;

uint32_t get_count() {
    uint32_t new_count;
    uint32_t cur_count;
    do {
        uint32_t cur_count = count;
        if (cur_count == std::numeric_limits<uint32_t>::max()) {
            new_count = 1;
        } else {
            new_count = cur_count + 1;
        }
    } while(!std::atomic_compare_exchange_weak(&count, &cur_count, new_count));
    return new_count;
}
{code}


was (Author: hanm):
bq. I didn't see an existing pattern to add test-only code either. Do you have 
any suggestions?
I think zoo_get_current_server could be one existing example of a 'largely for 
testing purposes' code. 

bq. which could cause performance issues with multiple threads making requests 
with different zhandles.
Yeah this was what I thought about. Might not be an issue if the number of 
threads is capped. 

bq. Did you have a specific implementation in mind?
How about this (in C++, could be updated to use gcc intrinsics as well in C):
{code}
#include <atomic>
#include <limits>
using namespace std;

std::atomic<uint32_t> count;

using namespace std;

uint32_t get_count() {
    uint32_t new_count;
    uint32_t cur_count;
    do {
        uint32_t cur_count = count;
        if (cur_count == std::numeric_limits<uint32_t>::max()) {
            new_count = 1;
        } else {
            new_count = cur_count + 1;
        }
    } while(!std::atomic_compare_exchange_weak(&count, &cur_count, new_count));
    return new_count;
}
{code}
{code}

> client xid overflow is not handled
> ----------------------------------
>
>                 Key: ZOOKEEPER-1485
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1485
>             Project: ZooKeeper
>          Issue Type: Bug
>          Components: c client, java client
>    Affects Versions: 3.4.3, 3.3.5
>            Reporter: Michi Mutsuzaki
>            Assignee: Martin Kuchta
>         Attachments: ZOOKEEPER-1485.patch
>
>
> Both Java and C clients use signed 32-bit int as XIDs. XIDs are assumed to be 
> non-negative, and zookeeper uses some negative values as special XIDs (e.g. 
> -2 for ping, -4 for auth). However, neither Java nor C client ensures the 
> XIDs it generates are non-negative, and the server doesn't reject negative 
> XIDs.
> Pat had some suggestions on how to fix this:
> - (bin-compat) Expire the session when the client sends a negative XID.
> - (bin-incompat) In addition to expiring the session, use 64-bit int for XID 
> so that overflow will practically never happen.
> --Michi



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to