ReconfigureAsync: fix re-entrancy issue by NickCraver · Pull Request #1772 · StackExchange/StackExchange.Redis (original) (raw)

When this was refactored long ago, we went from a counter starting at 1 (which handled the first run) to a string reason (which defaults to empty). Combined with the || clause format, this subtly introduced a re-entrancy bug where a subsequent run can enter during the first run (where first was true, so no exchange for reason occurred).

This was causing several Sentinel issues due to on connect handlers setting config as nodes returned and ultimately the Set<T> triggering a ReconfigureIfNeeded....and actually running, while the original config was still in progress. This led to all sorts of race oddness especially if one endpoint was any kind of significantly different in timing than another.

Note: the other changes are only labels on the bool arguments for clarity at the call sites.