IntrusiveQCellOwner?

[SoniEx2]

We don't suppose it'd be possible to have something along these lines?

struct IntrusiveQCellOwner<T: ?Sized> {
  inner: Arc<QCell<T>>,
}

// impl<T: ?Sized> !Clone for IntrusiveQCellOwner<T>;

impl<T> IntrusiveQCellOwner {
  fn new(value: T) -> Self {
      Arc::new_cyclic(|arc| {
        QCell { owner: QCellOwnerID(arc.as_ptr() as *const () as usize), value: UnsafeCell::new(value) }
      })
    }
  }
}

impl<T: ?Sized> IntrusiveQCellOwner {
  fn id(&self) -> QCellOwnerID {
    self.inner.owner
  }

  // the usual stuff, rw etc.

  fn clone_inner(&self) -> Arc<QCell<T>> {
    self.inner.clone()
  }

  // in fact, might aswell just use Deref and DerefMut for these.
  //fn mut_inner(&mut self) -> &mut T {
    // ...
  //}
  //fn get_inner(&self) -> &T {
    // ...
  //}
}

The main use-case would be the same as #16 except without a separate "owner".

pub struct ConfigManager {
    resources: Vec<Resource>,
}

pub struct AddConfigSource<'a, T: DataSourceBase + Send + Sync + 'static> {
    resource: &'a mut Resource,
    source: Arc<QCell<T>>,
}

struct Resource {
    // actual resource
    base: IntrusiveQCellOwner<dyn DataSourceBase + Send + Sync>,
    // views of the actual resource
    title: Option<Arc<QCell<dyn DataSource<InstanceTitle> + Send + Sync>>>,
    url: Option<Arc<QCell<dyn DataSource<InstanceBaseUrl> + Send + Sync>>>,
    repolist: Option<Arc<QCell<dyn DataSource<RepoListUrl> + Send + Sync>>>,
}

impl ConfigManager {
    pub fn add_source<T>(&mut self, source: T) -> AddConfigSource<'_, T>
    where
        T: DataSourceBase + Send + Sync + 'static,
    {
        let base = IntrusiveQCellOwner::new(source);
        let arc = base.clone_inner();
        self.resources.push(Resource::new(base));
        AddConfigSource {
            resource: self.resources.last_mut().unwrap(),
            source: arc,
        }
    }
}

impl<'a, T: DataSourceBase + Send + Sync + 'static> AddConfigSource<'a, T> {
    pub fn for_title(self) -> Self where T: DataSource<InstanceTitle> {
        let arc = &self.source;
        self.resource.title.get_or_insert_with(|| {
            arc.clone()
        });
        self
    }
    pub fn for_base_url(self) -> Self where T: DataSource<InstanceBaseUrl> {
        let arc = &self.source;
        self.resource.url.get_or_insert_with(|| {
            arc.clone()
        });
        self
    }
    pub fn for_repo_lists(self) -> Self where T: DataSource<RepoListUrl> {
        let arc = &self.source;
        self.resource.repolist.get_or_insert_with(|| {
            arc.clone()
        });
        self
    }
}

[uazu]

This isn't going to work. As you say, the owner is the cell itself. However to borrow mutably from the cell you need a &mut on the owner. But behind an Arc you can only get a & reference. So to be correct (i.e. sound), your cell-type would only be capable of ro type borrows, i.e. immutable borrows. To get a &mut behind an Arc, you need a Mutex or RwLock. So this intrusive-cell isn't helping at all. You might as well just have an Arc<Mutex<T>>.

The whole point of these cell types is to temporarily "transfer" mutability from one thing to something else that has only immutable access. In this case (plain Arc), you don't have anywhere with mutable access available. Does this make sense?

[SoniEx2]

This is sound. It's just kinda (read: very) weird.

The mutability would be held by the "original Arc". We manage the "original Arc" by encapsulating an Arc::new_cyclic inside an !Clone type - the lack of Clone prevents making new "owners" that hold the same Arc, such that no two IntrusiveQCellOwner ever point to the same object.

[uazu]

I'm still having trouble figuring out what you're trying to do.

So I think what you're saying is that the "original Arc" is the owner, and so accessing through the "original Arc" would allow modification of the cell contents. You say you're not going to allow cloning, but in your example code you have clone_inner. Those Arc clones are going to be useless, as you can't get access (not even immutable access) to the inside of the cell without having access to the owner (the "original Arc"). If you did allow immutable access through the Arc clones, then that would be a soundness failure, because somewhere else might be doing mutable access through the "original Arc".

However if the only Arc will be the "original Arc" (i.e. cloning is forbidden), then it doesn't need to be an Arc -- it could just be a Box. Still perhaps I have not really understood what you're trying to do.

(Also, if the "original Arc" needs to be the owner, then you either need to Box it or Pin it, so that its address is stable.)

Perhaps I would understand better if you could explain what advantage this approach has compared to the original code which used a QCellOwner.

[SoniEx2]

The clone_inner is -> Arc<QCell<...>>, not -> IntrusiveQCellOwner. The advantage is that this avoids an unnecessary allocation that's only used for the QCellOwnerID. Also note that Arc::as_ptr is usize-aligned.

[uazu]

Arc::as_ptr gives the address of the data inside the Arc (i.e. the data pointed to), not the address of the Arc itself (the pointer). The code as above is not going to work, but I'm trying to understand what you're trying to do to see if there is a way it could work.

If you try to code up the IntrusiveQCellOwner using safe code and QCellOwner maybe that would make more sense of things. It would mean that you couldn't access the T from an Arc<QCell<T>> without also having a reference to the IntrusiveQCellOwner. If this isn't what you expect, then I suspect that you've overlooked some problems with this arrangement.

[SoniEx2]

The code as above is going to work. And the data inside the Arc is a QCell - which has a QCellOwnerID in it so it's never zero-sized anyway.

In fact, the get_inner just needs to be:

  fn get_inner(&self) -> &T {
    self.ro(&self.inner)
  }

The mut_inner can't just be:

  fn mut_inner(&mut self) -> &mut T {
    self.rw(&mut self.inner)
  }

because self.rw borrows &mut self. Except this isn't unsound, it just requires copying the fn rw into mut_inner, as rw doesn't actually touch the QCell contents. Note that you can't just clone the Arc, as in:

  fn mut_inner(&mut self) -> &mut T {
    self.rw(&mut self.inner.clone())
  }

because the cloned Arc would get dropped at the end of the scope - there's no lifetime that satisfies this.

[steffahn]

The idea seems (probably) sound to me. But it might be a bit much new API for fairly little benefit (saving a single allocation). It’s also quite Arc-specific (without declaring so in the naming).

[uazu]

I can see that you can put an owner in IntrusiveQCellOwner and make access through the IntrusiveQCellOwner work. The problem is the Arc<QCell<T>> returned by clone_inner(). Those QCells can't be accessed without accessing them through the IntrusiveQCellOwner. But in Resource, you're keeping them right next to one another. So you do have the owner available. So I can kind of see how this would work now.

So this would be equivalent to putting a QCellOwner or a QCellOwnerPinned in the Resource. The optimisation will save a usize in memory (compared to QCellOwnerPinned).

If this was going to be made into a general-purpose owner type, it's not necessary to use QCell. It could be its own type. So you'd have ArcOwner<T> and ArcCell<T>. The T in the ArcCell<T> can't be accessed without having a &mut or & on the ArcOwner<T>. This checks that the as_ptr() on the two Arcs points to the same data. So there is no ID overhead. This is mostly useless (if you have access to the ArcOwner then why do you need to access through the ArcCell?) but in your case you're getting different vtables, which is the only thing you're really interested in.

Really the ideal situation would be to keep a Box<T> and a bunch of vtable pointers, and reconstruct the dyn pointers. But there is no safe or approved way to do this yet, although deconstructing and reconstructing a fat pointer is commonly done in various crates. For example I use it in Stakker to store a FnOnce in a flat list.

Anyway, I think ArcOwner and ArcCell as I've described are too special-purpose to add to this crate. I can't think of any use apart from this work-around for not being able to store multiple vtables in any other way. Or can you see a more general-purpose way of implementing this that might be useful for other things?

[SoniEx2]

The original IntrusiveQCellOwner fully supports being used with "unrelated" QCells. Yes, our use-case is purely to carry multiple VTables around, and clone_inner is pretty much limited/designed for that use-case, but the rest of the API isn't.

We do understand if this would be a better fit for a separate crate.

[SoniEx2]

It’s also quite Arc-specific (without declaring so in the naming).

It's hard to make APIs like these generic. Something that could support Arc, Rc and Box would be nice, but IntrusiveQCellOwner<X<QCell<T>>> is... verbose... and you don't want to support arbitrary Deref. you'd want some sort of unsafe trait DerefPinned: Deref or something.

[uazu]

Personally I'd skip straight to the desired end-point, i.e. try to store vtable pointers directly. I had a quick look to see if there had been any progress, and actually yes there has! Simon Sapin's RFC proposal was accepted last year. Discussion about the implementation is progressing here and there seems to be a preliminary implementation here. So maybe you can implement something using this crate. If it's of interest, my own code in Stakker that deconstructs and reconstructs fat pointers is here.

[SoniEx2]

We can't think of a way to carry Option<VTable>'s with a BaseObject that isn't inherently unsound. Arc<QCell> works. the thing is that regardless of how you do it, you still need to decouple the underlying mutability from each of the VTables, like QCell.

[SoniEx2]

honestly closing this as we believe it's handled by #37 / selfref