#2 Shared issue

Abierta
abierta hace 11 meses por david · 7 comentarios

@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 comentado hace 11 meses

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 comentado hace 11 meses

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 hace 11 meses
Propietario

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 hace 11 meses
Propietario

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 comentado hace 11 meses

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 hace 11 meses
Propietario

[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 hace 11 meses
Propietario

[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)
Inicie sesión para unirse a esta conversación.
Sin Milestone
Sin asignado
2 participantes
Cargando...
Cancelar
Guardar
Aún no existe contenido.