microsoft / microsoft/litebox

Consider Send/Sync-ness of RawConstPtr and friends

Open
#431 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

discussion layer-litebox
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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.