Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash when cancelling 2FA email verification #26824

Closed
Walavouchey opened this issue Jan 30, 2024 · 2 comments · Fixed by #26834
Closed

Crash when cancelling 2FA email verification #26824

Walavouchey opened this issue Jan 30, 2024 · 2 comments · Fixed by #26834
Assignees
Labels
priority:0 Showstopper. Critical to the next release. type:online

Comments

@Walavouchey
Copy link
Member

Type

Crash to desktop

Bug description

  1. Be logged out
  2. Log in with username and password
  3. Click "sign out" on the 2FA verification window instead of entering a code
  4. Game crashes on a nullref

Screenshots or videos

No response

Version

2024.130.2-lazer

Logs

1706636517.runtime.log

2024-01-30 17:42:58 [verbose]: ⚠️ An unhandled error has occurred.
2024-01-30 17:42:58 [verbose]: 
2024-01-30 17:42:58 [verbose]: This error has been automatically reported to the devs.
2024-01-30 17:42:58 [error]: An unhandled error has occurred.
2024-01-30 17:42:58 [error]: System.NullReferenceException: Object reference not set to an instance of an object.
2024-01-30 17:42:58 [error]: at osu.Game.Online.API.OAuth.RequestAccessToken()
2024-01-30 17:42:58 [error]: at osu.Game.Online.API.APIAccess.attemptConnect()
2024-01-30 17:42:58 [error]: at osu.Game.Online.API.APIAccess.run()
2024-01-30 17:42:58 [error]: at System.Threading.Thread.StartHelper.Callback(Object state)
2024-01-30 17:42:58 [error]: at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
2024-01-30 17:42:58 [error]: --- End of stack trace from previous location ---
2024-01-30 17:42:58 [error]: at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
2024-01-30 17:42:58 [error]: at System.Threading.Thread.StartCallback()
2024-01-30 17:42:58 [verbose]: Unhandled exception has been allowed with 0 more allowable exceptions .
2024-01-30 17:42:58 [verbose]: 🌅 Global background loading
@bdach bdach added priority:0 Showstopper. Critical to the next release. type:online labels Jan 30, 2024
@bdach
Copy link
Collaborator

bdach commented Jan 30, 2024

Thanks for the reproduction scenario on this one, I saw it on sentry but couldn't figure out how to make happen...

@ppy-sentryintegration
Copy link

Sentry issue: OSU-Y7R

@bdach bdach self-assigned this Jan 30, 2024
@bdach bdach moved this to Working to fix in osu!lazer "pp" release issue tracker Jan 30, 2024
bdach added a commit to bdach/osu that referenced this issue Jan 30, 2024
Closes ppy#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.
@github-project-automation github-project-automation bot moved this from Working to fix to Fixed in osu!lazer "pp" release issue tracker Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:0 Showstopper. Critical to the next release. type:online
Projects
Development

Successfully merging a pull request may close this issue.

2 participants