fix(image): fix data race in imageURLLoaderForURL: and imageDataDecoderForData:

[mfazekas]

Summary

imageURLLoaderForURL: had a broken double-checked lock on _loaders: the outer if (!_loaders) guard and the for loop iteration both ran without _loadersMutex, while the assignment ran inside it. imageDataDecoderForData: had no lock at all on _decoders. Both methods write the ivar in two steps (raw list then sorted), so a concurrent reader could observe the intermediate value and iterate a concurrently-released array, crashing with EXC_BAD_ACCESS in objc_release on a GCD worker thread.

Verified with a reproducer — crashes 5/5 without the fix, 0/10 with it: https://github.com/mfazekas/rn-rctimageloader-dcl-repro

Fix

The fix mirrors the setUp() pattern from #46153: add std::atomic<BOOL> flags (_loadersReady / _decodersReady) as acquire/release DCL guards, protect the slow path with _loadersMutex, and iterate a local strong ref captured under the lock so the shared ivar is never read outside it.

Two simpler alternatives were considered and rejected:

• Always lock (always acquire _loadersMutex on every call, even after init): correct but ~40× slower on the hot path — imageURLLoaderForURL: is called for every image load.
• dispatch_once: correct and lock-free after init, but involves a function call into libdispatch on every call.

The atomic DCL approach matches what setUp() already does and compiles to a single ldar (load-acquire) instruction on ARM64 on the hot path — effectively free after first use.

Related

Fixes #57296
See also #46115, #46153

[github-actions]

Warning

Missing Test Plan

Please add a "## Test Plan" section to your PR description. A Test Plan lets us know how these changes were tested.

Caution

Missing Changelog

Please add a Changelog to your PR description. See Changelog format

[fabriziocucci]

Nice fix on these two methods. I think one path is still open: canHandleRequest: also does for (id<RCTImageURLLoader> loader in _loaders) without the lock or the ready flag and it does not trigger the lazy init. So if it runs while imageURLLoaderForURL: is building _loaders on another thread, it can still observe the half-built array and hit the same objc_release crash this PR fixes.

Since _loaders now has three readers and _decoders two, would it be worth extracting a small accessor that does the DCL and returns the snapshot (something like - (NSArray<id<RCTImageURLLoader>> *)loaders), then using it everywhere _loaders is read, including canHandleRequest:? That would close this path and drop the duplicated DCL.

[meta-codesync]

@fabriziocucci has imported this pull request. If you are a Meta employee, you can view this in D109404243.