various soundness fixes and other improvements

[Freax13]

First of all I wanted to say that I really like what you've done here especially the concept of a reset function to reset objects once they've been used.

This pr fixes various soundness holes including:

1. require init and reset to implement Send and Sync:
2. fix bounds for Send and Sync: Previously you just implemented Send and Sync regardless of whether the underlying type actually implements those traits: https://github.com/EVaillant/lockfree-object-pool/blob/cba781c8e6d7eb9a15773149a29511cefafb665f/src/linear_object_pool.rs#L86-L87 https://github.com/EVaillant/lockfree-object-pool/blob/cba781c8e6d7eb9a15773149a29511cefafb665f/src/spin_lock_object_pool.rs#L94-L95 https://github.com/EVaillant/lockfree-object-pool/blob/cba781c8e6d7eb9a15773149a29511cefafb665f/src/mutex_object_pool.rs#L93-L94 https://github.com/EVaillant/lockfree-object-pool/blob/cba781c8e6d7eb9a15773149a29511cefafb665f/src/non_object_pool.rs#L55-L56
3. fix unsound casts to mutable references: simply casting to a mutable reference is UB https://github.com/EVaillant/lockfree-object-pool/blob/cba781c8e6d7eb9a15773149a29511cefafb665f/src/linear_reusable.rs#L50-L52 this pr uses UnsafeCells in Page for creating mutable references to the objects

Other improvements:

1. use pub(crate) instead of #[doc(hidden)] pub
2. add "safety" comments for unsafe blocks
3. change Page::alloc to use the first available bit instead of checking each bit individually
4. use fetch_update in Page::alloc
5. use fetch_or in Page::free instead of looping with compare_exchange_weak

I also wanted to point out that the tests in test_generic_02 and test_generic_03 don't completely work as intended for MutexObjectPool or SpinLockObjectPool as they compare the address of the objects with happen to be on the stack for MutexReusable and SpinLockReusable. https://github.com/EVaillant/lockfree-object-pool/blob/cba781c8e6d7eb9a15773149a29511cefafb665f/src/mutex_reusable.rs#L24-L27 I didn't fix this though

It might be worth considering to use generics instead of trait objects for init and reset. This introduces a lot more generics to keep track of, but also allows the Send and Sync requirements on init and reset to be relaxed (if the functions don't implement Send or Sync the pool then also doesn't implement Send or Sync which might not be needed in most situations).

[EVaillant]

First of all, i would like to thank you about your feedback. I'm a strong experience in C++ but i'm beginner in rust.

I also wanted to point out that the tests in test_generic_02 and test_generic_03 don't completely work as intended for MutexObjectPool or SpinLockObjectPool as they compare the address of the objects with happen to be on the stack for MutexReusable and SpinLockReusable.

After a long moment i understood my mistake. I will fix.

This introduces a lot more generics to keep track of, but also allows the Send and Sync requirements on init and reset to be relaxed (if the functions don't implement Send or Sync the pool then also doesn't implement Send or Sync which might not be needed in most situations).

what it is the problem to have Send or Sync requirements on init and reset ?

It might be worth considering to use generics instead of trait objects for init and reset.

My basic idea is : I don't want a trait on data to create & clear data because it is not flexible (two 2 on same data must init & reset way.

The other solution is have a trait but not on data:

 trait DataManagement<D> {
   fn init() -> D;
   fn reset(&mut D);
 }

struct ObjectPool<T, D : DataManagement<T>> {
 ...
 phantom_data: PhantomData<D>,
}

what do you think ?

[Freax13]

what it is the problem to have Send or Sync requirements on init and reset?

Since those datastructures are supposed to be used by multiple threads, all fields always have to be Send and Sync regardless of whether it's used in multiple threads. If you store the init and reset functions with generics instead of trait objects (dyn Fn...), you don't always require the Send and Sync bounds.

The other solution is have a trait but not on data:

 trait DataManagement<D> {
   fn init() -> D;
   fn reset(&mut D);
 }

struct ObjectPool<T, D : DataManagement<T>> {
 ...
 phantom_data: PhantomData<D>,
}

what do you think ?

This approach isn't quite as good imho because closures are actually well suited for this and also allow for captures. They way you implemented this with PhantomData<D> doesn't allow for captures or state in general.

[EVaillant]

more like ?

pub struct ObjectPool<D, I, R>
where
    I: Fn() -> D,
    R: Fn(&mut D),
{
    init  : I,
    reset : R,
}

impl<D, I, R> ObjectPool<D, I, R>
where
    I: Fn() -> D,
    R: Fn(&mut D),
{
    pub fn new(init : I, reset : R) -> Self {
        Self {
            init,
            reset,
        }
    }
}

but it is not easy to use

pub struct Data {
    pool : ObjectPool<u8, ?, ?>,
}

impl Data {
    pub fn new() -> Self {
        Self {
            pool : ObjectPool::new(|| Vec::<u8>::with_capacity(16 * 1024), |_v| {})
        }
    }
}

what do I replace the ? with ?

[Freax13]

more like ?

pub struct ObjectPool<D, I, R>
where
    I: Fn() -> D,
    R: Fn(&mut D),
{
    init  : I,
    reset : R,
}

impl<D, I, R> ObjectPool<D, I, R>
where
    I: Fn() -> D,
    R: Fn(&mut D),
{
    pub fn new(init : I, reset : R) -> Self {
        Self {
            init,
            reset,
        }
    }
}

Yeah I had something like that in mind.

but it is not easy to use

what do I replace the ? with ?

I don't know. It's possible but it'll be a lot more complicated. It's probably better just to keep it the way it is