Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please let us know the company's name.

Signed.

CLAs look good, thanks!

Just FYI, this is untested beyond confirming that it compiles without warnings under 10.12. I'd imagine it'll be fine elsewhere. I just haven't tested. If I'm wrong, I'm happy to mod the patch as needed.

Thanks.

ACK

Update: I finally ran all the tests and db_bench in static mode, along with db_bench in shared mode. They all passed (tests) or had what appeared to be reasonable numbers (db_bench).

Thanks.

Isn't it necessary to replace this with something else that implements the same barrier?

From the deprecation message, it seems std::atomic_thread_fence() is recommended:

./port/atomic_pointer.h:59:3: warning: 'OSMemoryBarrier' is deprecated: first deprecated in macOS 10.12 - Use std::atomic_thread_fence() from <atomic>
      instead [-Wdeprecated-declarations]
  OSMemoryBarrier();
  ^
/usr/include/libkern/OSAtomicDeprecated.h:749:9: note: 'OSMemoryBarrier' has been explicitly marked deprecated here
void    OSMemoryBarrier( void );
        ^
1 warning generated.

@tmm1 - Fair point. I have noticed, going through the commit history, that a couple of similar warnings were fixed in a different manner that followed what the compiler prescribed. I'll look into a solution that maintains the barrier.

Thanks.

Added atomic_thread_fence(), as suggested by the compiler. I chose sequentially-consistent ordering, as that seemed like the most logical choice. I'm happy to change this as the LevelDB team sees fit, though.

Re-ran all the tests as mentioned above. Everything seemed fine.

See #449, which is IMO the preferable fix.

@theuni - Thanks. I'll leave this open just in case but, after thinking about it, I think I prefer the fix @sipa put together.

Fixed here. Closing this out.