[azure-core-amqp] AuthenticateAudience holds the token mutex across the credential call and the claims based security operation

Inizia da ConnectionImpl::AuthenticateAudience in sdk/core/azure-core-amqp/src/amqp/connection.cpp e confronta la gestione dei lock con RefreshTokenForAudience. Poi esamina impl/uamqp/amqp/private/connection_impl.hpp, impl/uamqp/amqp/management.cpp e il client Rust per la sicurezza basata su claims nelle sorgenti vendored di fe2o3-amqp-management. Il lavoro è completo quando entrambi gli stack serializzano in modo sicuro le operazioni sui claims, l'arresto attende l'autenticazione in corso, lo stato della cache e lo stato wire rimangono ordinati e un test uAMQP su Linux o Windows copre l'apertura concorrente dei link e la chiusura della connessione.

Summary

ConnectionImpl::AuthenticateAudience takes m_tokenMutex at sdk/core/azure-core-amqp/src/amqp/connection.cpp:173 and holds it to the return at :228. Two network calls sit inside that hold. The credential call is at :205. The claims based security open, put-token, and close are at :193-220. So one caller that authenticates a new audience blocks every other caller on that connection, including a caller whose token is in the cache and would return at :176 with no network call at all.

Give up the mutex for the network work. The mutex also supplies a shutdown barrier by accident today, so the same change must make that barrier explicit.

Motivation

The blocked caller pays for work it does not need. A message sender, a message receiver, and a management client all authenticate through this one function (message_sender.cpp:302, message_receiver.cpp:386, management.cpp:60), and a management client authenticates again for each operation (management.cpp:187). A producer that opens a link for a new audience therefore stalls a properties call that already has a valid token, for the length of an identity service call plus an AMQP round trip.

Pull request #7308 makes the wait longer. It adds a background refresh thread and a second mutex, m_cbsMutex, that serializes the claims based security operation itself. The refresh thread can own that mutex for a full CBS round trip, bounded only by a 60 second deadline. A caller then waits for m_cbsMutex while it holds the token mutex, so every other caller waits behind both. The same pull request adds the pattern that fixes this. ConnectionImpl::RefreshTokenForAudience drops the token mutex for its network work and takes it again, and it applies three tests before it commits a result.

Two properties of the current code make a naive release unsafe.

The token mutex works as a shutdown barrier. StopTokenRefresh cancels the refresh context and then waits for the token mutex, so ConnectionImpl::Close cannot reach EnableAsyncOperation(false) (impl/uamqp/amqp/connection.cpp:447) while a caller sits inside the claims based security operation. The comment at impl/uamqp/amqp/connection.cpp:442-443 states the dependency, because a claims based security operation needs the connection to poll. Release the mutex without a replacement and a concurrent Close can stop the polling under an in-flight put-token. That put-token waits in AsyncOperationQueue::WaitForResult with no poller argument and on the caller's context, and the shutdown cancel never reaches that context, so a caller context with no deadline waits without end. Azure::Core::Context builds a single parent chain through WithDeadline and WithValue (sdk/core/azure-core/inc/azure/core/context.hpp:190-274), so a put-token context cannot be a child of both the caller's context and the shutdown context.

On the Rust stack that same mutex is the only claims based security serialization. m_cbsMutex in #7308 sits inside #if ENABLE_UAMQP. The Rust management client names its claims based security links from a fixed constant in the fe2o3-amqp-management crate (src/client.rs and src/constants.rs in the vendored copy), so two claims based security clients at one time break the same AMQP 1.0 section 2.6.1 rule that the uAMQP comment cites. Any change that stops the token mutex from covering that window must add the serialization to both stacks.

A correct release also needs the cache to agree with the wire. Two callers can authenticate one audience at the same time. A mutex on the claims based security operation orders the two put-token requests, and nothing orders the two cache writes. The cache can keep the token of the first put while the service keeps the claim of the second. When the cached token carries the later expiry, the refresh thread schedules from a value that is too optimistic and the link dies before the refresh runs.

The .NET client holds no broad lock here. CbsTokenProvider.AcquireJwtTokenAsync guards the credential fetch with a SemaphoreSlim(1, 1) and a double checked cache, so a second caller for one audience takes the token that the first caller cached instead of making its own request (sdk/eventhub/Azure.Messaging.EventHubs/src/Amqp/CbsTokenProvider.cs:108-153). Its $cbs link multiplexes put-token requests by correlation id, so those requests run at the same time with no mutual exclusion (src/Cbs/AmqpCbsLink.cs:100-137 in Azure/azure-amqp). The Go client does what this code does and holds a mutex across the claim (sdk/messaging/azeventhubs/internal/namespace.go:346-351).

Proposal

1. Release the token mutex before the credential call, and take it again after the claims based security operation returns.
2. Hold the claims based security mutex for the put-token only, with the token mutex free, in the way RefreshTokenForAudience already does.
3. Make the put-token and the cache write one step, so the cache always holds the token of the last put on the wire. A re-read that keeps "some usable token" is not enough, because that rule produces the disagreement described above.
4. Keep the cache write, the session write, and the refresh thread start in one hold. Two callers that split that hold can both find the refresh thread not joinable and start two threads.
5. Move the claims based security mutex out of #if ENABLE_UAMQP so both stacks keep one claims based security client at a time.
6. Replace the accidental shutdown barrier with an explicit one. Count the authentications in flight on the connection, and make Close wait for that count to reach zero before it stops the polling.
7. Correct the lock order comment in impl/uamqp/amqp/private/connection_impl.hpp, and correct the explanation of the $management guard in impl/uamqp/amqp/management.cpp. A nested AuthenticateAudience self deadlocks on the token mutex today. After this change it self deadlocks on the claims based security mutex, so the guard stays necessary and its reason changes.
8. Add a test that opens a link on one thread while a second thread closes the connection, and make sure the first thread completes. This test needs a Linux or Windows uAMQP build. No pipeline sets DISABLE_RUST_IN_BUILD, and every mock server test is excluded on macOS, so no test that runs today calls AuthenticateAudience at all.

The shutdown hazard in item 6 is the reason this does not belong in #7308. It is a design change to the connection shutdown, and no automated test in the pipeline reaches this code.