Consider Send/Sync-ness of RawConstPtr and friends
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 2.7k
- Forks
- 144
- Avg merge
- 12h 21m
- Merged PRs (30d)
- 146
Description
I see two potential issues with the status quo in this PR (#423):
One, when you impl `Send` on an aggregate type like this, it's easy for another developer to come along and add a non-`Send` field without revisiting the `Send` impl. So, I find it safer to impl `Send` on the primitive type and let the compiler infer `Send` on the aggregate. Of course, this could be done with an explicit newtype around the ptrs, but...
Two, if we don't require that the pointer types are mechanically `Send`, we at least need to require that they are _logically_ `Send` in the trait impl. E.g., with this code as is, it would be unsound to `impl RawConstPtr for Rc`. Now, that's already ruled out because of the `Copy` bound on `RawConstPtr`, but it seems very possible to have a `Copy + 'static` type that is also not "logically" `Send`, e.g., something representing an index into some array stored in TLS.
To address this, it seems like we at least need to document this requirement on `RawConstPtr` and friends, and we need to make the trait `unsafe` in order to enforce this requirement soundly.
But if we're going to do that, well, should we just make the raw pointer types `Send`?
Counter argument: no, because in general user pointers _shouldn't_ be sent between threads without additional thinking--what if the target thread is in a different address space (assuming we eventually implement fork for some platforms). So maybe forcing this code to use a newtype (which litebox could provide as a convenience) to document why this is OK would be more clear.
But either way, we need to do something to keep the code sound, I think. So, we should probably open an issue.
_Originally posted by @jstarks in https://github.com/microsoft/litebox/pull/423#discussion_r2436710190_
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reviewing PR #423 and the definitions of RawConstPtr and its related traits. Compare the proposed unsafe-trait, primitive-pointer, and newtype approaches, then resolve how logical Send/Sync requirements are enforced and documented so implementations remain sound.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- operating-systems
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100