Uploaded image for project: 'ZooKeeper'
  1. ZooKeeper
  2. ZOOKEEPER-1643

Windows: fetch_and_add not 64bit-compatible, may not be correct

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.3.3
    • Fix Version/s: 3.5.0, 3.4.11
    • Component/s: c client
    • Labels:
      None
    • Environment:

      Windows 7
      Microsoft Visual Studio 2005

      Description

      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.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                odysseus654 Erik Anderson
                Reporter:
                odysseus654 Erik Anderson
              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: