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

Bucket - write and read lock for credentials does not work well with tokio #337

Open
a-nickol opened this issue Apr 18, 2023 · 4 comments
Open
Assignees
Labels

Comments

@a-nickol
Copy link

a-nickol commented Apr 18, 2023

Describe the bug

When rust-s3 is used with tokio and in a concurrent setup, refreshing the credentials will result in a read or write race condition for the corresponding lock.

The credentials just use the system RwLock.

#[derive(Clone, Debug)]
pub struct Bucket {
    pub name: String,
    pub region: Region,
    pub credentials: Arc<RwLock<Credentials>>,
    pub extra_headers: HeaderMap,
    pub extra_query: Query,
    pub request_timeout: Option<Duration>,
    path_style: bool,
    listobjects_v2: bool,
}

impl Bucket {
    pub fn credentials_refresh(&self) -> Result<(), S3Error> {
        Ok(self
            .credentials
            .try_write()
            .map_err(|_| S3Error::WLCredentials)?
            .refresh()?)
    }
}

To Reproduce
Just make some concurrent requests read requests:

// cargo run --example tokio

use awscreds::Credentials;
use s3::error::S3Error;
use s3::{Bucket, Region};

#[tokio::main]
async fn main() -> Result<(), S3Error> {
    let bucket = Bucket::new(
        "rust-s3-test",
        "eu-central-1".parse()?,
        // Credentials are collected from environment, config, profile or instance metadata
        Credentials::default()?,
    )?;

    let s3_path = "test.file";
    let test = b"I'm going to S3!";

    let response_data = bucket.put_object(s3_path, test).await?;
    assert_eq!(response_data.status_code(), 200);

    for _ in 0..200 {
        let b = bucket.clone();
        tokio::spawn(async move {
            let response_data = b.get_object(s3_path).await;
            if let Err(e) = response_data {
                eprintln!("{}", e);
            }
        });
    }

    Ok(())
}

About 1 % of the requests result in a write error

Could not get Write lock on Credentials

About 0,5 % of the requests result in a read error

Could not get Read lock on Credentials

Expected behavior
No error should occur.

Environment

  • Rust version: rustc 1.67.0 (fc594f156 2023-01-24)
  • lib version master (bea733a) or 0.33

Possible Solution
The method should be async and bucket should use tokio::sync::RwLock when tokio is used.

@a-nickol a-nickol added the bug label Apr 18, 2023
@a-nickol a-nickol changed the title Write and Read Lock of Credentials does not use tokio api Bucket - Write and Read Lock for Credentials does not work well with tokio Apr 18, 2023
@a-nickol a-nickol changed the title Bucket - Write and Read Lock for Credentials does not work well with tokio Bucket - write and read lock for credentials does not work well with tokio Apr 18, 2023
@a-nickol
Copy link
Author

a-nickol commented Apr 19, 2023

A workaround is probably to create a bucket for every every task, tokio worker or a pool of buckets.

@muhamadazmy
Copy link

This is a very old issue! but is still there. I keep hitting the same issue when try to asynchronously put/get different objects

@pintoflager
Copy link

@a-nickol 's workaround above confirmed valid.

Ended up with a struct holding Credentials, Region and the bucket name + impl bucket() method.

I first encountered this with

.get_object_tagging()

But seems to be the same for gets and puts and such.

@msly
Copy link

msly commented Mar 8, 2024

729791a sees fix this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants