Discussion: make Json more flexible WRT sharing across threads

[abonander]

Json currently fails to satisfy slog's SendSyncUnwindSafeDrain because of the internal RefCell, necessitating wrapping it in a Mutex to use it as a drain in slog::Logger. This seems redundant, especially when using Json with io::Stdout or io::Stderr which also use mutexes internally and can be shared across threads.

The simplest solution might be to just replace RefCell with Mutex. Uncontended single-thread access of a mutex isn't much more expensive than RefCell, and since slog::Logger requires Send + Sync + UnwindSafe anyway, it seems more than reasonable just to use Mutex. Of course, there is the issue of poisoning; to be backwards-compatible we could either just ignore poisoning or wrap the poison error in io::Error.

However, I would prefer a more general solution that also allows us to avoid using Mutex for internally synchronized writers like Stdout/Stderr. I'm thinking it would look something like this:

pub struct RecordSerializer<'a> { /**/ }

impl<'a> RecordSerializer<'a> {
    /// Write out the log record contained in this serializer to `write`.
    pub fn write_to<W: Write>(self, write: W) -> io::Result<()> { /**/ }
}

pub trait SharedWrite {
    /// An error type that can wrap `io::Error` along with any locking error this impl may produce.
    type Error: From<io::Error>;

    /// Provide `serializer` with a `std::io::Write` impl via its `write_to()` method
    fn with_writer(&self, serializer: RecordSerializer) -> Result<(), Self::Error>;

    // I initially conceived of a `lock()` method that returned an impl of `io::Write` but it
    // would have required generic associated types to bind the lifetime to the `&self` param
}

pub enum MutexWriteError {
    Io(io::Error),
    Poisoned,
}

impl<T> From<PoisonError<T>> for MutexWriteError { /* */ }
impl From<io::Error> for MutexWriteError { /* */ }

impl<W: Write> SharedWrite for Mutex<W> {
    type Error = MutexWriteError;

    fn with_writer(&self, serializer: RecordSerializer) -> Result<(), Self::Error> {
        let writer = self.lock()?;
        serializer.write_to(writer).map_err(Into::into)
    }
}

impl SharedWrite for Stdout {
    type Error = io::Error;
 
    fn with_writer(&self, serializer: RecordSerializer) -> Result<(), Self::Error> {
        serializer.write_to(self.lock())
    }   
}

impl SharedWrite for Stderr {
    type Error = io::Error;
 
    fn with_writer(&self, serializer: RecordSerializer) -> Result<(), Self::Error> {
        serializer.write_to(self.lock())
    }   
}

The main issue is that this would be a breaking change to Json's API if we changed it to take SharedWrite instead of Write; to be backwards-compatible this would have to be introduced along with a new type that pretty much just duplicates Json's API, which isn't great either.

[abonander]

Note: stdio handles are currently not UnwindSafe anyway, but it's a trivial fix: rust-lang/rust#51863

[dpc]

Oh, wow. I need more time to think about it, but I see that you did your lesson.

I think it's OK to just introduce new major version.

I can't commit to do anything about it right now, but I encourage you to give it a try. :)