[Simn]

AtomicInt_obj::compareExchange (added in #1441) can report a successful exchange that didn't happen.

It loads original, then calls compare_exchange_strong(expected, replacement). If another thread changes the value between the load and the exchange, the exchange fails and writes the current value into expected. original keeps the stale value, which is equal to the caller's expected, so the loop exits and returns it. The caller reads that as success.

This PR passes a copy of expected to compare_exchange_strong and returns it. On failure, that copy holds the current value, so the result is the original value in both cases. AtomicObject_obj::compareExchange doesn't have this problem because it passes original itself.

Reproduction

Four threads each run 200,000 iterations of old = a.load(); if (a.compareExchange(old, old + 1) == old) ++successes;. Each reported success should be one real increment.

• Before: every run reported more successes than increments, for example 592,622 against 592,597.
• After: the counts match in every run.

This surfaced in hxcoro, where a work-stealing queue uses compareExchange to claim slots. Two threads could both claim the same slot, which dispatched a coroutine twice.

🤖 Generated with Claude Code

[Simn]

This is giving me a bit of a headache. Claude insists that AtomicInt is now more similar to AtomicObject, but it actually looks more different to me now... But I also don't really understand why we have a loop in these functions.

[tobil4sk]

It looks like the int _hx_atomic_... definitions were not updated in #1441.

This is _hx_atomic_compare_exchange, which looks similar to this new version:

hxcpp/include/hx/StdLibs.h

Lines 422 to 437 in 1689e5b

	inline int _hx_atomic_compare_exchange(volatile int *a, int expected,
	int replacement) {
	#if defined(HX_GCC_ATOMICS)
	int _expected = expected;
	__atomic_compare_exchange(a, &_expected, &replacement, false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
	return _expected;
	#elif defined(HX_MSVC_ATOMICS)
	return _InterlockedCompareExchange((long volatile *)a, replacement, expected);
	#else
	int old = *a;
	if(old == expected) {
	*a = replacement;
	}
	return old;
	#endif
	}

As a side note, it looks like the old int _hx_atomic_... functions were inline, but now they are defined in the .cpp file so there is now the cost of an extra function call for each atomic int access. @Aidan63 would it be possible to expose std::atomic<int> to haxe and use that instead of having the intermediate hx::atomic::AtomicInt_obj?

[Aidan63]

I didn't update the _hx_atomic_ functions as iirc they're mostly used internally and often in GC code so we can't make GC allocations there.
It is a bit unfortunate that it's now not inlined but I'd imagine if something here were to cause a performance cost it would be the pointer chase required by the PIMPL idiom. I do like this separation of the std implementation though as it allows swapping with another one, this would be especially helpful for consoles, while they do usually have a toolchain providing a C++ std implementation the recommended advice is to use their SDK instead and this makes it easy to swap out.
I'm happy to give things a rethink if something like the hxcoro stuff shows a noticeable difference since it makes heavy use of atomics (I'm sure micro-benchmarks would show something either way).

For exposing std::atomic specifically there are a couple of reasons why ideally I'd not want to do that.

• The compare_exchange functions take a reference to their expected value, which is a pain to deal with in haxe.
• The atomic header would then be included wherever haxe atomics were, these std library headers are usually quite large and heavily template based.
• We dodge atomic alignment requirements by packing it into a native object.

[Aidan63]

On an unrelated note, I've just noticed I forgot the finalisers on the new typed atomic objects so the impl object leaks. That'll need to be fixed.

[Aidan63]

Another comment, I was also a bit confused by the loop in the CAS stuff. But I didn't really look into it much and just assumed it's due to the C++ api returning bool and the haxe one returning an actual value. For the atomic int CAS I just copy and pasted the atomic object one and removed the GC write barriers and called it a day.

[Simn]

I'll merge this because it is definitely better, but yes there's still more to explore here.