fix: publish the hook list safely across threads

[abelonogov-ld]

Describe the bug

HookRunner keeps its hooks in a plain ArrayList with no synchronization:

private final List<Hook> hooks = new ArrayList<>();

public void addHook(Hook hook) {
    hooks.add(hook);
}

LDClient.addHook is public API and registering a hook after the client starts is its documented purpose, while withEvaluation, identify and afterTrack all read that list and may all be called from any thread. Four things follow:

• A hook registered on a background thread may never become visible to evaluations on another thread, because nothing establishes a happens-before edge between the add and the read.
• It may be seen half written. ArrayList.add stores the element and increments size, and a reader may observe the new size before the element, so hooks.get(i) returns null. The NullPointerException is then caught by the stage's own handler and logged as the hook having reported an error, and getHookName trips over the same null and logs that it could not read the hook's metadata. Two error lines about a hook that did nothing.
• Two concurrent addHook calls can lose one of the hooks or throw from inside ArrayList as it grows.
• A hook arriving mid-series unbalances the stages. This one is not about the list being thread-safe: a series read the field once per stage, so a hook appended between the before and after stages left the after loop starting one index beyond what seriesDataList reaches, and seriesDataList.get(i) threw IndexOutOfBoundsException. That call sits inside the try, so it was caught and logged as the new hook failing afterEvaluation, a stage it was never in. identify holds that window open for a whole round trip, since its after stage runs from the AfterIdentifyMethod it returns.

No flag value or event is affected. What it costs is hooks that quietly miss work they should have seen, and error logs blaming hooks for failures that were not theirs.

Describe the solution you've provided

The list is replaced rather than modified, and every series reads it once:

private volatile List<Hook> hooks;

public synchronized void addHook(Hook hook) {
    List<Hook> updated = new ArrayList<>(hooks);
    updated.add(hook);
    hooks = Collections.unmodifiableList(updated);
}

withEvaluation, identify and afterTrack each take List<Hook> hooks = this.hooks; as their first statement and work from that, so a series runs the hooks it began with and its stages stay paired. The method identify returns closes over the snapshot rather than the field, which is what keeps a hook registered during the round trip out of the second half of a series it was never in. A hook registered while a series is under way joins the next one.

The volatile read is the only cost on the evaluation path; the copy happens per addHook, a handful of times in an application's life.

One deliberate behavior change

The before loop re-read hooks.size() on every iteration, so a hook registered by an earlier hook's beforeEvaluation was picked up by the very series it was registered during. Against a snapshot it joins the next one instead. Nothing documented the old behavior, and it is the same mid-series registration this PR is about: a hook that observes half a series is worse off than one that starts cleanly at the next.

Describe alternatives you've considered

• CopyOnWriteArrayList. One line, and it fixes visibility and concurrent registration, but each get consults the current array, so the reads within a single series can still disagree and the unbalanced stages survive. Once the reads are snapshotted the list needs no concurrency of its own.
• Synchronizing the methods that run hooks. Serializes evaluations across threads and holds a lock across customer hook code, so a hook that evaluates a flag would deadlock.
• Making the list final and dropping addHook, which is what the iOS SDK does — its hooks are fixed at initialization and there is no equivalent method, which is why none of this arises there. Not available to us without removing published API.

Additional context

Three tests, each of which fails against the current implementation:

• an evaluation runs the hooks it began with, and one registered from inside another hook's before stage runs from the next evaluation;
• a hook registered between an identify's two stages is not given the second of them;
• four threads registering fifty hooks each lose none of them.

Found while working on evaluation exposure deduplication, but independent of it and older than it. Not related to the other Android branch in flight, andrey/one-flag-read-per-evaluation; the two touch different files and can land in either order.

---

Note

Overview
HookRunner no longer mutates a shared ArrayList for hooks. It keeps a volatile immutable list, addHook copies and publishes a new list under synchronized, and withEvaluation, identify, and afterTrack each read the list once and run a fixed hook snapshot for the whole series.

Identify’s returned AfterIdentifyMethod closes over that snapshot so hooks registered during the round trip do not run afterIdentify for a series they never entered. Hooks added mid-evaluation (including from another hook’s beforeEvaluation) take effect on the next series, not the current one.

Three unit tests cover snapshot semantics for evaluation and identify and concurrent addHook without lost registrations.

^{Reviewed by Cursor Bugbot for commit fd34a85. Bugbot is set up for automated code reviews on this repo. Configure here.}