Skip to content

Commit

Permalink
Fix some deadlocks and examples
Browse files Browse the repository at this point in the history
  • Loading branch information
marioortizmanero committed Sep 25, 2021
1 parent 2b2411b commit df5f9f1
Show file tree
Hide file tree
Showing 7 changed files with 17 additions and 10 deletions.
2 changes: 1 addition & 1 deletion examples/client_creds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ async fn main() {
// ```
let creds = Credentials::from_env().unwrap();

let mut spotify = ClientCredsSpotify::new(creds);
let spotify = ClientCredsSpotify::new(creds);

// Obtaining the access token. Requires to be mutable because the internal
// token will be modified. We don't need OAuth for this specific endpoint,
Expand Down
2 changes: 1 addition & 1 deletion examples/oauth_tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ async fn main() {
let url = spotify.get_authorize_url(false).unwrap();
spotify.prompt_for_token(&url).await.unwrap();

let token = spotify.get_token().await;
let token = spotify.token.lock().await.unwrap();
println!("Access token: {}", token.as_ref().unwrap().access_token);
println!(
"Refresh token: {}",
Expand Down
2 changes: 2 additions & 0 deletions examples/with_auto_reauth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ async fn main() {
.await
.lock()
.await
.unwrap()
.as_mut()
.map(|x| x.expires_at = Some(now.clone()));
println!(">>> Session two, the token should expire, then re-auth automatically");
Expand All @@ -104,6 +105,7 @@ async fn main() {
.await
.lock()
.await
.unwrap()
.as_mut()
.map(|x| x.expires_at.replace(now));
println!(">>> New Session two from ClientCredsSpotify, expiring the token and then re-auth automatically");
Expand Down
3 changes: 1 addition & 2 deletions src/auth_code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,7 @@ impl BaseClient for AuthCodeSpotify {
async fn refetch_token(&self) -> ClientResult<Option<Token>> {
// NOTE: this can't use `get_token` because `get_token` itself might
// call this function when automatic reauthentication is enabled.

match self.get_token().await.lock().await.unwrap().as_ref() {
match self.token.lock().await.unwrap().as_ref() {
Some(Token {
refresh_token: Some(refresh_token),
..
Expand Down
6 changes: 3 additions & 3 deletions src/auth_code_pkce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl BaseClient for AuthCodePkceSpotify {
// NOTE: this can't use `get_token` because `get_token` itself might
// call this function when automatic reauthentication is enabled.

match self.get_token().await.lock().await.unwrap().as_ref() {
match self.token.lock().await.unwrap().as_ref() {
Some(Token {
refresh_token: Some(refresh_token),
..
Expand All @@ -86,7 +86,6 @@ impl OAuthClient for AuthCodePkceSpotify {
}

async fn request_token(&self, code: &str) -> ClientResult<()> {
// TODO
let mut data = Form::new();
let oauth = self.get_oauth();
let scopes = oauth
Expand All @@ -103,7 +102,8 @@ impl OAuthClient for AuthCodePkceSpotify {

let token = self.fetch_access_token(&data).await?;

*self.token.lock().await.unwrap() = Some(token);
// NOTE: get_token can be used safely here
*self.get_token().await.lock().await.unwrap() = Some(token);

self.write_token_cache().await
}
Expand Down
10 changes: 8 additions & 2 deletions src/client_creds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ impl ClientCredsSpotify {
/// saved internally.
#[maybe_async]
pub async fn request_token(&self) -> ClientResult<()> {
*self.token.lock().await.unwrap() = Some(self.fetch_token().await?);
// NOTE: `get_token` can be used safely here
*self.get_token().await.lock().await.unwrap() = Some(self.fetch_token().await?);

self.write_token_cache().await
}
Expand All @@ -137,10 +138,13 @@ impl ClientCredsSpotify {
if !self.config.token_refreshing {
return Ok(());
}
// NOTE: this can't use `get_token` because `get_token` itself might
// call this function when automatic reauthentication is enabled.
//
// You could not have read lock and write lock at the same time, which
// will result in deadlock, so obtain the write lock and use it in the
// whole process.
if let Some(token) = self.get_token().await.lock().await.unwrap().as_ref() {
if let Some(token) = self.token.lock().await.unwrap().as_ref() {
if !token.is_expired() {
return Ok(());
}
Expand All @@ -154,6 +158,8 @@ impl ClientCredsSpotify {
async fn refresh_token(&self) -> ClientResult<()> {
let token = self.refetch_token().await?;
if let Some(token) = token {
// NOTE: this can't use `get_token` because `get_token` itself might
// call this function when automatic reauthentication is enabled.
self.token.lock().await.unwrap().replace(token);
}

Expand Down
2 changes: 1 addition & 1 deletion tests/test_with_oauth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use rspotify::{
TrackPositions,
},
prelude::*,
scopes, AuthCodeSpotify, ClientResult, Credentials, OAuth, Token,
scopes, AuthCodeSpotify, ClientResult, Token,
};

use std::env;
Expand Down

0 comments on commit df5f9f1

Please sign in to comment.