Introduce generic `ChanPtr`
This introduces a generic ChanPtr type which adds reference counting to the channel.
This allows us to remove the Sender type and promote inbox::Receiver to a (future public) Mailbox type.
On most parts, the implementation is the same. Important differences are:
- The actual logic for reference counting is encapsulated on
Chan. This used to be #162 but I ended up doing it here because the merge conflicts would have been insane. RefCounterPolicydoes not have aninto_eitherfn. This is solved by special-casingimplblocks forChanPtr.- The new implementation is accompanied by a range of tests.
- We remove the
RefCountertrait from the publicref_countmodule. This is not actually necessary and allows us to avoid the sealing of the trait because users cannot name it anyway.
If we end up merging https://github.com/Restioson/xtra/pull/160, I am gonna iterate on this design again. I think it would be a step backwards to introduce a "ref count policy" again just so we can use this abstraction also for the receiver side.
One alternative idea to this would be to retain the Sender and Receiver newtypes but implement Deref on them so we don't need to forward all the methods.
I think this will need to be rebased or updated now that #160 has been merged
I think this will need to be rebased or updated now that #160 has been merged
Yep, I'll need to think about a new design here.
So after thinking about this more, I think it is overall better to actually re-introduce a ref-count policy with this PR. Here is my thinking:
I am currently trying to achieve symmetry between various modules. #159 will introduce a top-level recv_future module that is equivalent to the send_future module. Both of these futures need to have a reference-counted pointer to the inner channel, i.e. they can't just take Arc<ChanA>>.
Address wraps a Sender which is technically not necessary, all of the logic of Sender can go into Address except that we would be introducing circular references between the address and the send_future module because SendFuture needs to have reference-counted pointer to Chan which - after the removal of Sender would be Address.
The same situation applies to Receiver. With #133, Receiver will be renamed to Mailbox or at least there will be a public Mailbox type with a receive function on it that will return a ReceiveFuture. Mailbox will thus be the equivalent of Address. ReceiveFuture will have to also contain a reference-counted pointer to the Chan which again, shouldn't be Mailbox itself because of the circular-reference issue.
Thus, I think despite an Rx reference count type not being strictly necessary, the ChanPtr abstraction is quite clean and provides a safe way for us to manage the reference counts to the channel. It can easily embedded in other types, most of the code can be reused between Address and Mailbox and the symmetry makes it easy to understand.
Does the above sound reasonable to you @Restioson ?
except that we would be introducing circular references between the address and the send_future module
Is this a problem? I don't think this will be a compile fail, anyway.
Is this essentially what you are imagining?
type Address<A, Rc> = ChanPtr<A, Rc>; // maybe with newtypes
type Mailbox<A> = ChanPtr<A, RxStrong>;
Then, the difference between RxStrong and TxStrong is simply which counter is modified.
Another way to look at it: yes, receiver has a ref counter, but this also encapsulates the logic for ref counting into ChanPtr rather than the receiver.
except that we would be introducing circular references between the address and the send_future module
Is this a problem? I don't think this will be a compile fail, anyway.
It is not a technical problem but it is a design smell. With the right abstractions, you should be able to avoid circular dependencies and code is much easier understood if dependencies are uni-directional.
Is this essentially what you are imagining?
type Address<A, Rc> = ChanPtr<A, Rc>; // maybe with newtypes type Mailbox<A> = ChanPtr<A, RxStrong>;Then, the difference between RxStrong and TxStrong is simply which counter is modified.
This but with new-types so we have control over the public API of the library.
Okay, this is ready for another review @Restioson! I've integrated #162 into here because the merge conflicts would have been insane.
I also updated the PR description to be an up-to-date summary of what this patch set does.
All dependent PRs have been merged and feedback has been addressed :)
Would appreciate another review here @Restioson :)
I've left some comments - apologies for the long wait, I've been busy as term is coming to a close so we are writing tests and handing in assignments currently. It's vac soon so I should have more time then though :)
I've left some comments - apologies for the long wait, I've been busy as term is coming to a close so we are writing tests and handing in assignments currently. It's vac soon so I should have more time then though :)
No worries and thanks for taking the time :)
Good to merge once tests pass I think
Good to merge once tests pass I think
Needs your approval though :)