Is `mioco::get_userdata` sound?

[dpc]

It seems to me that:

  pub fn get_userdata<'a, T: Any>() -> Option<&'a T> {
      let coroutine = unsafe { tl_current_coroutine() };
      match coroutine.user_data {
          Some(ref arc) => {
              let boxed_any: &Box<Any + Send + Sync> = arc.as_ref();
              let any: &Any = boxed_any.as_ref();
              any.downcast_ref::<T>()
          } 
          None => None,
      }
  }

Is really:

  pub fn get_userdata<T: Any>() -> Option<&'static T> {

The problem with it is that underlying storage is not really 'static, but Arc tied to a life of a coroutine. The returned &'static T could be stored in a real global, while coroutine could finish and drop the whole userdata pointer.

If I'm not wrong, we should probably change get_userdata functions to return something from Owning Ref that would store both the copy of Arc and reference to the downcasted &T.

[dpc]

Pinging @Drakulix . What do you think?

[Drakulix]

I guess this is a Problem, you are right. 'a would need to be the lifetime of the current coroutine, something which cannot be expressed. Right now you can insert 'static for 'a which would create a dangeling pointer, IF the reference gets moved between coroutines.

I think we have multiple options here:

1. Force T to be not Sync (T: Any + !Sync). I am not sure, if this is actually possible, but it would express, that the userdata shall not be moved between coroutines, which would fix the hole. This however could be limiting for the user.
2. Return a cloned Arc or a Weak of that Arc. We could cast the Arc<Any + Send + Sync> to Arc/Weak.
3. Use something like the OwningRef, which looks kinda weird to me.

[dpc]

With 3 it would be:

  pub fn get_userdata<T: Any>() -> Option<ArcRef<T>> {

With 2, we could do:

  pub fn get_userdata<T: Any>() -> Option<Arc<Box<Any + Send + Sync>>> {

and user would have to do the cast manually. The whole point of OwningRef is that you can both keep the copy of the original Arc to guarantee it's not freed prematurely while functioning as just Ref<T>.

I'm not sure if !Sync blocks all the problems. User can still stuff the &'static T into a global Mutex or something like that?

[Drakulix]

Mutexes do not grant Sync capabilities, only Send. But since it would be a reference, you could not do that as well.

We may cast the 2 Option inside the function using mem::transmute: pub fn get_userdata<T: Any>() -> Option<Arc<Box<T>>> {

[Drakulix]

But that being set OwningRef sounds to be exactly what we need, I just prefer fixing it with what Rust provides, instead of having another dependency. But I will leave that decision up to you. There is nothing wrong with OwningRef itself.

[dpc]

Are you sure about the Mutex?

https://doc.rust-lang.org/std/sync/struct.Mutex.html

pub struct Mutex<T: ?Sized> { // does not require Sized

impl<T: ?Sized + Send> Send for Mutex<T> // adds Send
impl<T: ?Sized + Send> Sync for Mutex<T> // adds Sync

[dpc]

I wish OwningRef was in standard library. The only drawback that I see is that user will have to deal with some new custom type.

[Drakulix]

Yes I am:

impl<T: ?Sized + Send> Send for Mutex<T> //T must be Send!
impl<T: ?Sized + Send> Sync for Mutex<T> //and again

It only implements Send and Sync for Types which are Send. Meaning you may use a Mutex, but it will not provide you with any benifits thread-wise. And since Sync refers to The precise definition is: a type T is Sync if &T is thread-safe. (https://doc.rust-lang.org/std/marker/trait.Sync.html), a reference of a Type, that is not Sync, cannot be Send and therefor cannot be shared between Coroutines.

[dpc]

I see. So !Sync would solve the threading issue. Let me think if there are other cases that could go wrong.

[dpc]

How about

mioco::set_userdata(...);
let x = mioco::get_userdata(...);
mioco::set_userdata(... something else ...);

During the second set_userdata the previous one will be deleted as Arc count goes to 0, and x will be dangling, no? So I guess !Sync won't help here.

[Drakulix]

yes...

I think I am in favour of pub fn get_userdata<T: Any>() -> Option<Weak<Box<T>>>. But OwningRef would be as good.

[dpc]

All right. Thanks for help!

[Drakulix]

always a pleasure

[burdges]

Appears one should note this on ye great list of things one could probably do better with higher-kinded types. :)

[dpc]

@burdges: Could you explain how could it work if HKT was implemented?

[dpc]

I went with OwningRef as transmutes etc. seem risky, while OwningRef kind of explains the whole reason of this complications. #130

One drawbacks that I've noticed in tests is that Eq is not implemented for OwningRef.

[burdges]

I meant simply that HTK might facilitate doing all of the above, and OwningRef looks like a typical comonad, but upon the light of day 1 and 2 do not appear to fit the mold.