#2 Shared issue

開啟中
david11 月之前創建 · 7 條評論

@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 commented 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 commented 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 commented 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 commented 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 commented 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 commented 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 commented 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)
Sign in to join this conversation.
未選擇里程碑
未指派成員
2 參與者
正在加載...
取消
保存
尚未有任何內容