durch / rust-s3

Rust library for interfacing with S3 API compatible services
MIT License
498 stars 195 forks source link

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

Closed a-nickol closed 1 week ago

a-nickol commented 1 year ago

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

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

a-nickol commented 1 year ago

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

muhamadazmy commented 8 months ago

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 commented 7 months ago

@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 commented 3 months ago

https://github.com/durch/rust-s3/commit/729791a30b63ae72890d43d4e4ed4f6ca6a28e4b sees fix this