wvwwvwwv / scalable-concurrent-containers

High performance containers and utilities for concurrent and asynchronous programming
Apache License 2.0
285 stars 14 forks source link

Request: Remove 'static from Bag, Stack, Queue #128

Closed beckend closed 4 months ago

beckend commented 5 months ago

This will improve when used internally by for example a Channel implementation so users can push T without 'static.

wvwwvwwv commented 5 months ago

Hi again :-)

Note that Bag doesn't require such a lifetime bound because instances of T are guaranteed to be dropped before the Bag is gone, and references of T cannot escape the lifetime of the Bag.

On the other hand, T: 'static is necessary for Stack and Queue, because those instances of T are behind ebr where the lifetime of them can be indefinitely extended; e.g., even when T: 'a, ebr may keep T beyond 'a. Furthermore, Queue and Stack even allow users to keep references of contained instances as long as they want using ebr::Guard.

However, it is still possible to use T: 'a where 'a < 'static with your own discretion. Stack and Queue additionally provide unsafe methods that allow pushing non static instances.

For instance, the following code just works.

let a = [1, 2, 3];
let stack = Stack::default();
for v in a.iter() {
    unsafe {
        stack.push_unchecked(v);
    }
}
assert_eq!(**stack.pop().unwrap(), &3);
assert_eq!(**stack.pop().unwrap(), &2);
assert_eq!(**stack.pop().unwrap(), &1);

Would those unsafe methods be enough for your project? -> I'm afraid to say that I won't remove unsafe from those methods, since they are really unsafe.

beckend commented 5 months ago

Thanks.

beckend commented 5 months ago

Another question, even if it might be kept beyond 'a, won't it be fully cleaned up eventually?

wvwwvwwv commented 5 months ago

Yes, all will be eventually cleaned up though the exact time point when it happens is quite non-deterministic.

The problem here is (assuming that no references leak), T::drop can be invoked after ‘a if ‘a is not ‘static; if T::drop somehow accesses memory that has been freed before ‘a, and it is use-after-free.

beckend commented 5 months ago

Then, how about giving us a .clear() for Queue and Stack, we then have the ability to make it behave like Bag if we call clear manually?

wvwwvwwv commented 5 months ago

Indeed, it will make sense to implement such a method that drops all the instances in place without passing to ebr. Let me think about this a little bit further.

beckend commented 5 months ago

I also notice there are no async functions like HashMap and HashSet, why not?

beckend commented 5 months ago

This also seem not possible, it's a scc::ebr::shared

  if let Some(x) = self.queue.pop() {
      return Some(x);
    }
beckend commented 5 months ago

Bag is less useful since it's not Ordered, using it to stream IOT sensor data.

wvwwvwwv commented 5 months ago

Async methods are neither provided nor necessary since there is no blocking code.

Yes, unfortunately the return type is not plain T; I understand this may complicate the usage in many cases, so I’ll also think about providing methods returning T (this won’t be trivial though..).

wvwwvwwv commented 5 months ago

Sorry to tell you that I’m pretty busy this and next week. I’ll provide something by the end of Feb!

wvwwvwwv commented 4 months ago

Entry::take_inner is now public that allows you to get T out of Entry in combination with Shared::get_mut.