Consider changing io::Error to use Arc so it can implement Clone

[lilyball]

io::Error can't implement Clone right now because it uses a trait object internally for custom errors. With #24133 adding the Sync bound, it becomes possible to put io::Error in Arc. But it would still be nice to be able to Clone it again, which could be accomplished by changing the private Box<Custom> field to Arc<Custom> instead.

[lilyball]

/cc @alexcrichton

[sbstp]

@kballard @alexcrichton it seems like the change was done, but that io::Error still doesn't implement clone.Was it forgotten?

[alexcrichton]

The choice to use Arc was intentionally left out to take the more conservative route to start out. Some time has passed, though, and it could perhaps be re-evaluated again.

[flying-sheep]

damn, a clonable io::Error would be really nice for me right now 😢

[spinda]

Normally I would just leave a 👍 here, but it's been a year since the last comment and I've bumped up against this several times in the past few weeks. Would an RFC be appropriate here to move things along, or is further discussion required? (If so, in what venue?)

[sanmai-NL]

@alexcrichton: @spinda and others have brought this issue up again, but no action or explanation has followed. Moreover, this issue has only one tag. Are you the person to triage or otherwise take action on this, or perhaps someone else?

[dtolnay]

I would prefer for this to go through the RFC process.

Most importantly for the RFC, one thing that hasn't been discussed so far is what are some of the uses cases where cloning an io::Error is the best solution. The RFC would need to provide some concrete situations from real code where not being able to clone an io::Error causes problems, and justify why cloning an io::Error is the best solution to those problems.

Second, a discussion of what implications this would have for performance. In my opinion this is not critical but it may help avoid derailing the discussion with guesses about Arc performance.

[kornelski]

For the record, I needed cloneable io::Error in order to cache a Result:

fn get_foo(&self) -> Result<Foo, Err> {
   self.cached.clone()
}

I've worked around that by storing io::Error as a String, so I'm not annoyed enough by this to write an RFC :)

[NoraCodes]

I just wanted to chime in here. Many use cases can indeed work around this issue, but I'm currently hitting one that's pretty much insoluable as far as I can see. I have a stream of Result<Bytes, MyError> which uses a channel to communicate that it's finished to another async task - so, in other words, I need to return a MyError and send a duplicate MyError through the channel. MyError is an enum whose variants include io::Error, so this is a serious problem.

My current workaround is to simply not send MyError to the other async task; all it gets is TaskStatus::Error(). That's okay for now, but if the project is still receptive to this as an RFC, I'm tempted to write one.

[lilyball]

@NoraCodes Can you replace the io::Error with an Arc<io::Error> instead?

[simonbuchan]

Another use case: I was trying to reimplement a slice of tokio::process (for dumb reasons I won't get into here) with the same API. That has the method:

impl Child {
  pub async fn wait(&mut self) -> std::io::Result<ExitStatus>;
}

Notably, you should be able to call this multiple times, (though not in parallel) and it should return the same result - including any error.

Doing this relatively safely (given you need to call FFI at a minimum) is quite tricky: for clarity the issue is the same in the simplest option: to spawn_blocking(|| native_wait(self.handle.raw)).await, but that requires Self: 'static, since otherwise self could be dropped during native_wait. So really, you want to move self.handle into the spawn_blocking, but of course that means that it's not there for a second call to wait().

The ideal here is to model Child as:

pub struct Child {
  id: u32
  state: ChildState,
}

enum ChildState {
  Running(Handle),
  Exited(std::io::Result<ExitStatus>),
}

and simply return the content of ChildState::Exited in that alternative - but that leads to this issue.