[
https://issues.apache.org/jira/browse/ZOOKEEPER-1643?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13575629#comment-13575629
]
Erik Anderson commented on ZOOKEEPER-1643:
------------------------------------------
Yah, I did notice that. The first xadd appears to be used to retrieve the
end-return value, where the second is actually doing the work (unless I really
read the asm wrong, I don't have it in front of me right now).
I'm seeing a potential non-atomic operation with the current formulation and
two simultaneous callers, where both callers trade off on the "LOCK XADD 0"
pulling the same return result, then both trade off on the "LOCK XADD n". The
two operations are still individually atomic and the variable still gets bumped
by the proper amount, but the return value for one of the callers would be
wrong.
In any case, it would be good to get something a little less processor-specific
here. I'm actually surprised that no one has tried to compile this on a
non-intel linux machine yet...
> Windows: fetch_and_add not 64bit-compatible, may not be correct
> ---------------------------------------------------------------
>
> Key: ZOOKEEPER-1643
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1643
> Project: ZooKeeper
> Issue Type: Bug
> Components: c client
> Affects Versions: 3.3.3
> Environment: Windows 7
> Microsoft Visual Studio 2005
> Reporter: Erik Anderson
>
> Note: While I am using a really old version of ZK, I did do enough "SVN
> Blame" operations to realize that this code hasn't changed.
> I am currently attempting to compile the C client under MSVC 2005 arch=x64.
> There are three things I can see with fetch_and_add() inside of
> /src/c/src/mt_adapter.c
> (1) MSVC 2005 64bit will not compile inline _asm sections. I'm moderately
> sure this code is x86-specific so I'm unsure whether it should attempt to
> either.
> (2) The Windows intrinsic InterlockedExchangeAdd
> [http://msdn.microsoft.com/en-us/library/windows/desktop/ms683597(v=vs.85).aspx]
> appears to do the same thing this code is attempting to do
> (3) I'm really rusty on my assembly, but why are we doing two separate XADD
> operations here, and is the code as-written anything approaching atomicity?
> If you want an official patch I likely can do an SVN checkout and submit a
> patch the replaces the entire #else on lines 495-505 with a "return
> InterlockedExchangeAdd(operand, incr);"
> Usually when I'm scratching my head this badly there's something I'm missing
> though. As far as I can tell there has been no prior discussion on this code.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
