Fix deadlock in bearer token refresh (#13494)

* Fix deadlock in bearer token refresh

If GetToken() fails the renewing flag wasn't reset, leading to a
deadlock.  Ensure the flag is reset before exiting.

* refine and add test

sdk/azidentity/bearer_token_policy.go

@@ -87,20 +87,26 @@ func (b *bearerTokenPolicy) Do(req *azcore.Request) (*azcore.Response, error) {
 	if getToken {
 		// this go routine has been elected to refresh the token
 		tk, err := b.creds.GetToken(req.Context(), b.options)
+		// update shared state
+		b.cond.L.Lock()
+		// to avoid a deadlock if GetToken() fails we MUST reset b.renewing to false before returning
+		b.renewing = false
 		if err != nil {
+			b.unlock()
 			return nil, err
 		}
 		header = bearerTokenPrefix + tk.Token
-		// update shared state
-		b.cond.L.Lock()
-		b.renewing = false
 		b.header = header
 		b.expiresOn = tk.ExpiresOn
-		// signal any waiters that the token has been refreshed
-		b.cond.Broadcast()
-		b.cond.L.Unlock()
+		b.unlock()
 	}
 	req.Request.Header.Set(azcore.HeaderXmsDate, time.Now().UTC().Format(http.TimeFormat))
 	req.Request.Header.Set(azcore.HeaderAuthorization, header)
 	return req.Next()
 }
+
+// signal any waiters that the token has been refreshed
+func (b *bearerTokenPolicy) unlock() {
+	b.cond.Broadcast()
+	b.cond.L.Unlock()
+}

sdk/azidentity/bearer_token_policy_test.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"net/http"
 	"testing"
+	"time"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
 	"github.com/Azure/azure-sdk-for-go/sdk/internal/mock"
@@ -146,3 +147,34 @@ func TestRetryPolicy_HTTPRequest(t *testing.T) {
 		t.Fatalf("unexpected error type %v", err)
 	}
 }
+
+func TestBearerPolicy_GetTokenFailsNoDeadlock(t *testing.T) {
+	srv, close := mock.NewTLSServer()
+	defer close()
+	srv.AppendResponse(mock.WithBody([]byte(accessTokenRespSuccess)))
+	srv.AppendResponse(mock.WithStatusCode(http.StatusOK))
+	cred, err := NewClientSecretCredential(tenantID, clientID, secret, &ClientSecretCredentialOptions{
+		HTTPClient:    srv,
+		AuthorityHost: srv.URL(),
+		Retry: &azcore.RetryOptions{
+			// leaving TryTimeout at zero will trigger a deadline exceeded error causing GetToken() to fail
+			RetryDelay:    50 * time.Millisecond,
+			MaxRetryDelay: 500 * time.Millisecond,
+			MaxRetries:    3,
+		}})
+	if err != nil {
+		t.Fatalf("Unable to create credential. Received: %v", err)
+	}
+	pipeline := defaultTestPipeline(srv, cred, scope)
+	req, err := azcore.NewRequest(context.Background(), http.MethodGet, srv.URL())
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp, err := pipeline.Do(req)
+	if err == nil {
+		t.Fatal("unexpected nil error")
+	}
+	if resp != nil {
+		t.Fatal("expected nil response")
+	}
+}