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

OAuth2Client: improve sleep/close detection #8337

Merged
merged 1 commit into from Apr 16, 2024

Conversation

adutra
Copy link
Contributor

@adutra adutra commented Apr 16, 2024

A few improvements on the sleep/close detection mechanisms:

  1. Move the first lastAccess computation to start(), in case the client is instantiated long before being used for the first time;
  2. Update lastAccess whenever fetchNewTokens() require user interaction (authorization and device code flows): prevents incorrect switch to sleep mode;
  3. Check whether the client is closed in wakeUp() : no need to wake up; can happen if the client was sleeping, then is closed, then authenticate() is called.
  4. Check whether the client is closed in maybeScheduleTokensRenewal(): no need to check and switch to sleep; avoids the case where the client is closed after some inactivity, then maybeScheduleTokensRenewal is called and incorrectly puts the client to sleep.
  5. In log() simplify the check if log is needed: the token future can only be cancelled iif closing is true.

@@ -158,6 +158,9 @@ public void close() {
}

private void wakeUp(Instant now) {
if (closing.get()) {
return;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe log a debug message in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me merge this as is and I'll add the debug message in #8338.

@adutra adutra merged commit b86d8de into projectnessie:main Apr 16, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants