#2 Shared issue

Aberto
há 2 dias atrás aberto por david · 7 comentários

@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? :)

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.

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 comentado há 1 dia atrás
Proprietário

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 comentado há 1 dia atrás
Proprietário

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.

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 comentado há 1 dia atrás
Proprietário

[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 comentado há 1 dia atrás
Proprietário

[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)
Faça login para participar desta conversação.
Sem Objetivo
Sem utilizador atribuido
2 Participantes
A carregar...
Cancelar
Guardar
Ainda não existe conteúdo.