Teams connector token: promise-cache on the IO pool, not a SemaphoreSlim gate by rbuergi · Pull Request #763 · Systemorph/MeshWeaver

rbuergi · 2026-08-03T06:32:51Z

The find

TeamsClient held the last hand-woven async gate in the repo:

private readonly SemaphoreSlim _tokenGate = new(1, 1);
...
await _tokenGate.WaitAsync(ct);      // double-checked lock around an HTTP token fetch
finally { _tokenGate.Release(); }

A SemaphoreSlim parks the awaiting thread. Awaited from a hub action block or a grain turn that is a deadlock, and it bounds nothing the IO pool does not already bound.

It escaped the usual audit because the target-typed new(1, 1) form does not match a grep "new SemaphoreSlim" — worth knowing for future sweeps.

The fix

The sanctioned promise-cache: the fetch runs via _httpPool.Run(...), cached in an instance ConcurrentDictionary keyed by the expiry being replaced. Callers racing on the same stale token share ONE fetch and replay its completion; a successful refresh moves the expiry, which becomes the next key, and the finally drops the spent entry so the cache cannot grow without bound.

Same single-flight guarantee as the gate, with no parked thread.

`await` only inside a pool body

SendMessageAsync now composes GetConnectorToken() with a pooled PostActivityAsync via SelectMany, leaving a single ToTask at the ITeamsClient boundary as the only Task in the public shape. Both IO leaves — token fetch and activity POST — run on the HTTP pool.

The pool is optional (IoPoolRegistry? → IoPool.Unbounded), exactly as AzureBlobStorageAdapter and InstanceOAuthService do it, so no construction site has to change.

Verification

dotnet build memex/Memex.Portal.Shared -c Release -warnaserror → 0 warnings, 0 errors.

Scope note

This is a genuine latent deadlock, but it is not the cause of the shard-1/3 CI race or the pre-warm false regressions — TeamsClient is outbound Teams messaging and sits on neither path. Fixed on its own merits.

Release note

Skipped — internal concurrency correctness, no user-visible change.

🤖 Generated with Claude Code

…lim gate TeamsClient held the last hand-woven async gate in the repo: private readonly SemaphoreSlim _tokenGate = new(1, 1); ... await _tokenGate.WaitAsync(ct); // double-checked lock around an HTTP token fetch finally { _tokenGate.Release(); } A SemaphoreSlim parks the awaiting thread. Awaited from a hub action block or a grain turn that is a deadlock, and it bounds nothing the IO pool does not already bound. It also escaped the usual grep: the target-typed `new(1, 1)` form does not match "new SemaphoreSlim". Replaced with the sanctioned promise-cache: the fetch runs via _httpPool.Run and is cached in an INSTANCE ConcurrentDictionary keyed by the expiry being replaced. Every caller racing on the same stale token shares ONE fetch and replays its completion; a successful refresh moves the expiry, which is itself the next key, and the finally-block drops the spent entry so the cache cannot grow. Same single-flight guarantee, no gate, no parked thread. Also moves every await inside a pool body: SendMessageAsync now composes GetConnectorToken() with a pooled PostActivityAsync via SelectMany, leaving one ToTask at the ITeamsClient boundary as the only Task in the file's public shape. The pool is optional (IoPoolRegistry? -> IoPool.Unbounded) exactly as AzureBlobStorageAdapter and InstanceOAuthService do it, so nothing else has to change to construct one. Verified: Memex.Portal.Shared builds Release -warnaserror with 0 warnings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR replaces the last SemaphoreSlim-based “single-flight” gate in TeamsClient with an IO-pool-backed, promise-cached token refresh flow to avoid parking threads (which can deadlock hub/grain turns) and to keep outbound Teams HTTP work bounded by the Http IO pool.

Changes:

Removed the SemaphoreSlim token gate and introduced an IO-pool promise-cache for connector token refreshes.
Reworked SendMessageAsync to compose token fetch + activity POST reactively and keep await usage inside pooled leaves.
Added optional IoPoolRegistry injection with IoPool.Unbounded fallback.

Suppressed comments (1)

memex/Memex.Portal.Shared/Teams/TeamsClient.cs:137

The cached-token fast path can throw if _cachedToken becomes visible before _tokenExpiry (unsynchronized publication): _tokenExpiry.AddMinutes(-2) will underflow when _tokenExpiry is still DateTimeOffset.MinValue. Also, once _tokenFetch stores Lazy<IObservable<...>> to avoid duplicate factories, this call site needs to unwrap .Value.

        var cached = _cachedToken;
        if (cached is not null && DateTimeOffset.UtcNow < _tokenExpiry.AddMinutes(-2))
            return Observable.Return(cached);
        var replacing = _tokenExpiry.Ticks;
        return _tokenFetch.GetOrAdd(replacing, key => _httpPool.Run(poolCt => FetchConnectorTokenAsync(key, poolCt)));

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    // being replaced, so every caller that sees the SAME stale token shares ONE fetch and replays
+    // its completion; a successful refresh moves the expiry, which is itself the next key.
+    private readonly IIoPool _httpPool;
+    private readonly ConcurrentDictionary<long, IObservable<string?>> _tokenFetch = new();


github-actions · 2026-08-03T06:49:15Z