What's wrong
CredentialedSessionCache.Get() (BuildMonitor/Providers/CredentialedSessionCache.cs:75, called from AzureDevOps.cs:89) handles changed credentials by calling Invalidate(), which disposes the previous session (VssConnection) immediately.
Callers that already got that session keep it across the rate-limit delay and the HTTP call, and up to 5 requests can be in flight at once. The class remarks say a rebuild "does not disturb a request already in flight", but disposing the session does exactly that.
Why it matters
- In-flight calls fail with
ObjectDisposedException.
MakeAzureDevOpsRequestAsync only catches Vss* exceptions and HttpRequestException, so the ObjectDisposedException escapes and faults the update loop.
- The Set Credentials flow changes
AccountId and then Token in two separate popups, so it triggers two rebuilds while polling continues. This is the ordinary path for re-entering a PAT, not a rare race.
- The existing test
ASessionHandedToACallerSurvivesARebuildBehindIt only reads a string off the fake session, so it never checks disposal.
Reproduction (ran)
Get("org", "pat1"), keep the result.
Get("org", "pat2").
- Result: the first session reports
Disposed == true while its holder may still be using it.
Suggested fix / acceptance criteria
- Either lease sessions with a reference count and dispose a replaced session only when its last holder releases it, or defer disposal of replaced sessions.
- At minimum, have
MakeAzureDevOpsRequestAsync catch ObjectDisposedException and retry once with a fresh session.
- Extend the existing test to assert that the session handed out earlier is not disposed while it is held.
What's wrong
CredentialedSessionCache.Get()(BuildMonitor/Providers/CredentialedSessionCache.cs:75, called fromAzureDevOps.cs:89) handles changed credentials by callingInvalidate(), which disposes the previous session (VssConnection) immediately.Callers that already got that session keep it across the rate-limit delay and the HTTP call, and up to 5 requests can be in flight at once. The class remarks say a rebuild "does not disturb a request already in flight", but disposing the session does exactly that.
Why it matters
ObjectDisposedException.MakeAzureDevOpsRequestAsynconly catchesVss*exceptions andHttpRequestException, so theObjectDisposedExceptionescapes and faults the update loop.AccountIdand thenTokenin two separate popups, so it triggers two rebuilds while polling continues. This is the ordinary path for re-entering a PAT, not a rare race.ASessionHandedToACallerSurvivesARebuildBehindItonly reads a string off the fake session, so it never checks disposal.Reproduction (ran)
Get("org", "pat1"), keep the result.Get("org", "pat2").Disposed == truewhile its holder may still be using it.Suggested fix / acceptance criteria
MakeAzureDevOpsRequestAsynccatchObjectDisposedExceptionand retry once with a fresh session.