Use correct atomic operation while generating client id

[tezc]

To generate a client id, atomicGetIncr() is called. atomicGetIncr() uses memory_order_relaxed which does not guarantee visibility. If createClient() is called from different threads, it might cause generating same id for different clients.

This PR adds incr/decr atomic operations with visibility guarantees and uses atomicGetIncrWithSync() for client id generation.

[oranagra]

@tezc please remind me where we create client outside the main thread without the GIL being locked?
is it just RM_GetThreadSafeContext?
i suppose these all must be createClient(NULL), since passing a non NULL arg will lead to linkClient being called, which is not protected by any synchronization code.

@ShooterIT FYI.

[tezc]

@oranagra Yes, looks like these two right now : RM_GetThreadSafeContext, RM_GetDetachedThreadSafeContext.

[tezc]

Btw, I didn't run into a problem. Feels like quite unlikely to cause an issue(Also looks quite hard to identify the problem and reproduce it). Just discussed a bit with @yossigo, as it's safer to use full memory barrier, I created the PR.

[ShooterIT]

If createClient() is called from different threads, it might cause generating same id for different clients.

I don't think this problem could happen, atomic operation could guarantee atomic. There is a problem that others threads could read the latest update, but in redis, there is no this problem since executing commands always is in the main thread.

FYI, https://en.cppreference.com/w/c/atomic/atomic_fetch_add

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}