rust-lang / rust-lang/libs-team

Implement Read and Write for Arc<T> for types where &T implements Read and Write, *and* it is safe to do so.

Open
#504 6 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

api-change-proposal
Dominant language
Rust
Stars
178
Forks
28
Avg merge
15m
Merged PRs (30d)
1

Description

Proposal

Problem statement

There's an inconsistency in the standard library at present: Arc<File> implements Read and Write, whereas Arc<T> for several other Read+Write types do not. In some cases (such as for [u8]), it isn't correct to do so (see links below). But for several others, such as TcpStream and UnixStream, it would be correct.

This proposal stands for the general proposition that, when &T implements Read and Write, and when it would be correct for Arc<T> to implement Read and Write, we should implement Read and Write on Arc<T>. It also stands for the specific proposal to implement Read and Write on Arc<TcpStream> and Arc<UnixStream>.

(See also prior discussions at https://github.com/rust-lang/rust/issues/53835 and https://github.com/rust-lang/rust/pull/94744. I'm opening this ACP because I was asked to at #134190.)

(This is my first ACP; please let me know if I've done it wrong.)

Motivating examples or use cases

This is roughly the use case I have, though others exist:

fn split_stream<T>(stream: T) -> (Box<dyn Read>, Box<dyn Write>)
where T: 'static,
     Arc<T>: Read + Write
{
    let arc = Arc::new(stream);
    (Box::new(arc.clone()),  Box::new(arc))
}

// This will work fine:
fn split_file(f:File) -> (Box<dyn Read>, Box<dyn Write>) {
    split_stream(f)

// This will fail:
fn split_tcp(f:TcpStream) -> (Box<dyn Read>, Box<dyn Write>) {
    split_stream(f)
}

(This is only one possible motivation. Prior discussion suggests that implementing Read and Write on Arc<T>, where it is correct to do so, would be useful in general for reasons of consistency.)

Solution sketch

See PR at https://github.com/rust-lang/rust/pull/134190.

At first glance, this could possibly be extended to these types, though I don't know if there is a reason to do so:

  • Arc<Stdout>
  • Arc<Stderr>
  • Arc<PipeWriter>
  • Arc<PipeReader>
  • Arc<ChildStdin>
  • Arc<ChildStdout>
  • Arc<Empty>
  • Arc<Sink>

Alternatives

There are numerous other workarounds that work for the motivating example, without changes to the library.

  1. We can tell the user to use try_clone(), at the cost of using an extra fd, and creating a fallible API.
  2. We can tell the user to define a wrapper type that implements Read + Write on NewType(Arc<T>), specifically for the cases where it is safe to do so. This requires a certain amount of boilerplate.
  3. We can tell the user to define a wrapper type that implements Read + Write on Newtype(Arc<Mutex<T>>) for arbitrary T implementing Read + Write. This requires a certain amount of boilerplate, and prevents simultaneous reads and writes.

Within the standard library, there are other approaches for solving the motivating problem.

  1. The standard library could implement its own version of Tokio's split_owned, returning a separate ReadHalf and WriteHalf for each current duplex Read+Write type it provides. This might be desirable for other reasons (such as performance), but is orthogonal to this proposal.
  2. The standard library could deprecate and eventually remove the Read and Write implementations for &T whenever it is not correct for them to apply to Arc<T>. If it did so, it would then be safe to provide a blanket impl<T> Read for Arc<T> where &T:Read .
  3. The standard library could add a ReadByRef trait (and by analogy a WriteByRef trait) for those types where &T implements Read and it is correct to implement Read for Arc<T>. It could then provide a blanket impl<T:ReadByRef> Read for Arc<T>.

Nonetheless, by analogy to the fact that Arc<File> currently implements Read and Write, I think that this approach would probably be the simplest and least controversial approach.

Links and related work

Prior discussions of impl {Read,Write} for Arc<T> where &T: {Read,Write}:

Existing implementation of impl Read,Write for Arc<File>:

Proposed implementation for Arc<TcpStream>, Arc<UnixStream>

Contributor guide

No contributing guide indexed for this repository

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 reading the existing Read and Write implementations for Arc, then review prior issue 53835, PR 94744, and the proposed PR 134190. Compare the safety and scope of supporting Arc and Arc with the additional listed types. Done means reaching and implementing a clear standard-library decision for the general and specific proposals.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
networking, operating-systems
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.