#2 Shared issue

Відкрити
11 місяців тому відкрито david · 7 коментарів
David Thielemann відкоментовано 11 місяців тому

@bugz, could you post here the bug, and maybe the fix you used? :)

@bugz, could you post here the bug, and maybe the fix you used? :)
bugz відкоментовано 11 місяців тому

The issue:

//#[derive(Clone)]  // NO!  See fn clone() below!

pub struct SharedData<T> {
    pub data: Arc<Mutex<T>>,
}

When using the #[derive(Clone)] trait, rust wants everything, including T to implement the Clone trait. This is wrong!

Implement your own clone for this:

    /// Clone just the Arc data, not T.
    pub fn clone(&self) -> Self {
        Self {
            data: self.data.clone(),
        }
    }

This, does what the comments say. It doesn't clone T, (or worse yet, think that it needs to), it clones just the shared data part.

The issue: ```rust //#[derive(Clone)] // NO! See fn clone() below! pub struct SharedData<T> { pub data: Arc<Mutex<T>>, } ``` When using the #[derive(Clone)] trait, rust wants everything, including T to implement the Clone trait. This is wrong! Implement your own clone for this: ```rust /// Clone just the Arc data, not T. pub fn clone(&self) -> Self { Self { data: self.data.clone(), } } ``` This, does what the comments say. It doesn't clone T, (or worse yet, think that it needs to), it clones just the shared data part.
bugz відкоментовано 11 місяців тому

It did not allow my attachment.

data.rs:

use std::sync::Arc;
use tokio::sync::Mutex;
use tokio::sync::MutexGuard;
use tokio::time::{timeout, Duration};

const LOCK_TIMEOUT: u64 = 30;

//#[derive(Clone)]  // NO!  See fn clone() below!

pub struct SharedData<T> {
    pub data: Arc<Mutex<T>>,
}

impl<T> SharedData<T> {
    pub fn new(data: T) -> Self {
        SharedData {
            data: Arc::new(Mutex::new(data)),
        }
    }

    /// Clone just the Arc data, not T.
    pub fn clone(&self) -> Self {
        Self {
            data: self.data.clone(),
        }
    }

    /// Acquire lock
    ///
    /// Using:
    /// ```
    /// {
    ///     let data = shared.lock().await;
    ///     // make changes to ConnectionData (or T)
    ///     data.nick = "New".to_string();
    /// }
    /// ```
    pub async fn lock(&self) -> MutexGuard<'_, T> {
        let result = timeout(Duration::from_secs(LOCK_TIMEOUT), self.data.lock()).await;
        if let Ok(result) = result {
            result
        } else {
            panic!(
                "Failed to acquire lock in {} seconds.  Deadlock?",
                LOCK_TIMEOUT
            );
        }
    }
}
It did not allow my attachment. data.rs: ```rust use std::sync::Arc; use tokio::sync::Mutex; use tokio::sync::MutexGuard; use tokio::time::{timeout, Duration}; const LOCK_TIMEOUT: u64 = 30; //#[derive(Clone)] // NO! See fn clone() below! pub struct SharedData<T> { pub data: Arc<Mutex<T>>, } impl<T> SharedData<T> { pub fn new(data: T) -> Self { SharedData { data: Arc::new(Mutex::new(data)), } } /// Clone just the Arc data, not T. pub fn clone(&self) -> Self { Self { data: self.data.clone(), } } /// Acquire lock /// /// Using: /// ``` /// { /// let data = shared.lock().await; /// // make changes to ConnectionData (or T) /// data.nick = "New".to_string(); /// } /// ``` pub async fn lock(&self) -> MutexGuard<'_, T> { let result = timeout(Duration::from_secs(LOCK_TIMEOUT), self.data.lock()).await; if let Ok(result) = result { result } else { panic!( "Failed to acquire lock in {} seconds. Deadlock?", LOCK_TIMEOUT ); } } } ```
David Thielemann відкоментовано 11 місяців тому
Власник

Hmm... I wonder if:

impl<T> Clone for Shared<T> {
    fn clone(&self) -> Self {
        Self {
            data: self.data.clone(),
            lock_timeout: self.lock_timeout.clone(),
        }
    }
}

works or if it needs to not be bound to the Clone trait.

I'd suggest trying this crate...

cargo add --git https://git.red-green.com/david/shared

Hmm... I wonder if: ```rust impl<T> Clone for Shared<T> { fn clone(&self) -> Self { Self { data: self.data.clone(), lock_timeout: self.lock_timeout.clone(), } } } ``` works or if it needs to not be bound to the Clone trait. I'd suggest trying this crate... `cargo add --git https://git.red-green.com/david/shared`
David Thielemann відкоментовано 11 місяців тому
Власник

If that fixes the issue then we can close this issue.

If it doesn't, then I'll implement clone as a method, and not implement the Clone trait itself.

If that fixes the issue then we can close this issue. If it doesn't, then I'll implement clone as a method, and not implement the Clone trait itself.
bugz відкоментовано 11 місяців тому

I don't have lock_timeout in my SharedData.

pub struct SharedData<T> {
    pub data: Arc<Mutex<T>>,
}

You do, so yes, your clone would need to copy it. u64 does not need to be .cloned().

lock_timeout: self.lock_timeout,

The purpose of my code having a lock timeout was so I could detect deadlocks.

I don't have lock_timeout in my SharedData. ```rust pub struct SharedData<T> { pub data: Arc<Mutex<T>>, } ``` You do, so yes, your clone would need to copy it. u64 does not need to be __.cloned()__. ```rust lock_timeout: self.lock_timeout, ``` The purpose of my code having a lock timeout was so I could detect deadlocks.
David Thielemann відкоментовано 11 місяців тому
Власник

[fb7135e0cb]

impl<T> Clone for Shared<T> {
    fn clone(&self) -> Self {
        Self {
            data: self.data.clone(),
-           lock_timeout: self.lock_timeout.clone(),
+           lock_timeout: self.lock_timeout,
        }
    }
}

The lock_timeout for you're project is a CONST, I wanted it so if you wanted to customize the lock you could... hence the with_timeout(data, lock_timeout).

[fb7135e0cb] ```rust impl<T> Clone for Shared<T> { fn clone(&self) -> Self { Self { data: self.data.clone(), - lock_timeout: self.lock_timeout.clone(), + lock_timeout: self.lock_timeout, } } } ``` The lock_timeout for you're project is a `CONST`, I wanted it so if you wanted to customize the lock you could... hence the `with_timeout(data, lock_timeout)`.
David Thielemann відкоментовано 11 місяців тому
Власник

[v0.2.0]

I added a comment just so lock_timeout is clear to someone using intellisense.

I updated the documentation example on Shared::new

I updated the example doc_example (It got it's name because it merges all the doc example code into 1 file, so it hopefully is clearer)

\[[v0.2.0](https://git.red-green.com/david/shared/src/v0.2.0)\] I added a comment just so lock_timeout is clear to someone using intellisense. I updated the documentation example on Shared::new I updated the example doc_example (It got it's name because it merges all the doc example code into 1 file, so it hopefully is clearer)
Підпишіться щоб приєднатися до обговорення.
Етап відсутній
Немає відповідального
2 учасників
Завантажується...
Скасувати
Зберегти
Тут ще немає жодного вмісту.