From 000ddc14acadf104af4724731c0aa41e27952ab0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= <dach.bartlomiej@gmail.com>
Date: Tue, 30 Jan 2024 20:50:00 +0100
Subject: [PATCH] Fix broken locking in `OAuth`

Closes https://github.com/ppy/osu/issues/26824... I think?

Can be reproduced via something like

diff --git a/osu.Game/Online/API/OAuth.cs b/osu.Game/Online/API/OAuth.cs
index 485274f349..e6e93ab4c7 100644
--- a/osu.Game/Online/API/OAuth.cs
+++ b/osu.Game/Online/API/OAuth.cs
@@ -151,6 +151,11 @@ internal string RequestAccessToken()
         {
             if (!ensureAccessToken()) return null;

+            for (int i = 0; i < 10000; ++i)
+            {
+                _ = Token.Value.AccessToken;
+            }
+
             return Token.Value.AccessToken;
         }

The cause is `SecondFactorAuthForm` calling `Logout()`, which calls
`OAuth.Clear()`, _while_ the `APIAccess` connect loop is checking if
`authentication.HasValidAccessToken` is true, which happens to
internally check `Token.Value.AccessToken`, which the clearing of
tokens can brutally interrupt.
---
 osu.Game/Online/API/OAuth.cs | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/osu.Game/Online/API/OAuth.cs b/osu.Game/Online/API/OAuth.cs
index 485274f349..4829310870 100644
--- a/osu.Game/Online/API/OAuth.cs
+++ b/osu.Game/Online/API/OAuth.cs
@@ -128,19 +128,12 @@ namespace osu.Game.Online.API
             // if we already have a valid access token, let's use it.
             if (accessTokenValid) return true;
 
-            // we want to ensure only a single authentication update is happening at once.
-            lock (access_token_retrieval_lock)
-            {
-                // re-check if valid, in case another request completed and revalidated our access.
-                if (accessTokenValid) return true;
+            // if not, let's try using our refresh token to request a new access token.
+            if (!string.IsNullOrEmpty(Token.Value?.RefreshToken))
+                // ReSharper disable once PossibleNullReferenceException
+                AuthenticateWithRefresh(Token.Value.RefreshToken);
 
-                // if not, let's try using our refresh token to request a new access token.
-                if (!string.IsNullOrEmpty(Token.Value?.RefreshToken))
-                    // ReSharper disable once PossibleNullReferenceException
-                    AuthenticateWithRefresh(Token.Value.RefreshToken);
-
-                return accessTokenValid;
-            }
+            return accessTokenValid;
         }
 
         private bool accessTokenValid => Token.Value?.IsValid ?? false;
@@ -149,14 +142,18 @@ namespace osu.Game.Online.API
 
         internal string RequestAccessToken()
         {
-            if (!ensureAccessToken()) return null;
+            lock (access_token_retrieval_lock)
+            {
+                if (!ensureAccessToken()) return null;
 
-            return Token.Value.AccessToken;
+                return Token.Value.AccessToken;
+            }
         }
 
         internal void Clear()
         {
-            Token.Value = null;
+            lock (access_token_retrieval_lock)
+                Token.Value = null;
         }
 
         private class AccessTokenRequestRefresh : AccessTokenRequest