Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Poison the debug writer on error #38

Merged
merged 2 commits into from
May 7, 2019
Merged

Poison the debug writer on error #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c885eb5
poison the debug writer on error
KodrAus May 7, 2019
ae01bd2
restore old layout
KodrAus May 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
282 changes: 200 additions & 82 deletions src/fmt/to_debug.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,18 @@ This stream is an alternative implementation of `std::fmt::DebugMap` and `std::f
It should be kept up to date with changes made upstream.
*/
struct Stream<'a, 'b: 'a> {
/**
The inner value is kept behind an `Option` and dropped in case it's poisoned.

This is a little convoluted, but protects the inner state from being re-used
after a panic or error is returned. It _shouldn't_ be observable unless the
caller implementation of `Value` is buggy, so long as it's not possible to
get a hold of this `Stream` type directly.
*/
inner: Option<Inner<'a, 'b>>,
}

struct Inner<'a, 'b: 'a> {
stack: Stack,
depth: usize,
delim: Option<&'static str>,
Expand All @@ -51,13 +63,33 @@ impl<'a, 'b: 'a> Stream<'a, 'b> {
#[inline]
fn new(fmt: &'a mut Formatter<'b>) -> Self {
Stream {
stack: Stack::new(),
depth: 0,
delim: None,
fmt,
inner: Some(Inner {
stack: Stack::new(),
depth: 0,
delim: None,
fmt,
}),
}
}

#[inline]
fn and_then(
&mut self,
f: impl FnOnce(&mut Inner<'a, 'b>) -> Result<(), stream::Error>,
) -> Result<(), stream::Error> {
self.inner = match self.inner.take() {
Some(mut inner) => match f(&mut inner) {
Ok(()) => Some(inner),
Err(e) => return Err(e),
},
None => return Err(stream::Error::msg("attempt to use poisoned writer")),
};

Ok(())
}
}

impl<'a, 'b: 'a> Inner<'a, 'b> {
#[inline]
fn next_delim(&self, pos: stack::Pos) -> Option<&'static str> {
if pos.is_value() || pos.is_elem() {
Expand All @@ -80,23 +112,27 @@ impl<'a, 'b: 'a> Stream<'a, 'b> {
impl<'a, 'b: 'a> stream::Stream for Stream<'a, 'b> {
#[inline]
fn begin(&mut self) -> Result<(), stream::Error> {
self.stack.begin()?;
self.and_then(|inner| {
inner.stack.begin()?;

Ok(())
Ok(())
})
}

#[inline]
fn fmt(&mut self, v: stream::Arguments) -> Result<(), stream::Error> {
let pos = self.stack.primitive()?;
self.and_then(|inner| {
let pos = inner.stack.primitive()?;

let next_delim = self.next_delim(pos);
if let Some(delim) = mem::replace(&mut self.delim, next_delim) {
self.fmt.write_str(delim)?;
}
let next_delim = inner.next_delim(pos);
if let Some(delim) = mem::replace(&mut inner.delim, next_delim) {
inner.fmt.write_str(delim)?;
}

write!(self.fmt, "{:?}", v)?;
v.fmt(inner.fmt)?;

Ok(())
Ok(())
})
}

#[inline]
Expand Down Expand Up @@ -136,134 +172,146 @@ impl<'a, 'b: 'a> stream::Stream for Stream<'a, 'b> {

#[inline]
fn seq_begin(&mut self, _: Option<usize>) -> Result<(), stream::Error> {
if self.is_pretty() {
self.depth += 1;
}
self.and_then(|inner| {
if inner.is_pretty() {
inner.depth += 1;
}

self.stack.seq_begin()?;
inner.stack.seq_begin()?;

if let Some(delim) = self.delim.take() {
self.fmt.write_str(delim)?;
}
if let Some(delim) = inner.delim.take() {
inner.fmt.write_str(delim)?;
}

self.fmt.write_char('[')?;
inner.fmt.write_char('[')?;

Ok(())
Ok(())
})
}

#[inline]
fn seq_elem(&mut self) -> Result<(), stream::Error> {
if self.is_pretty() {
if !self.stack.current().is_empty_seq() {
if let Some(delim) = self.delim.take() {
self.fmt.write_str(delim)?;
self.and_then(|inner| {
if inner.is_pretty() {
if !inner.stack.current().is_empty_seq() {
if let Some(delim) = inner.delim.take() {
inner.fmt.write_str(delim)?;
}
}
}

self.fmt.write_char('\n')?;
pad(&mut self.fmt, self.depth)?;
}
inner.fmt.write_char('\n')?;
pad(&mut inner.fmt, inner.depth)?;
}

self.stack.seq_elem()?;
inner.stack.seq_elem()?;

Ok(())
Ok(())
})
}

#[inline]
fn seq_end(&mut self) -> Result<(), stream::Error> {
if self.is_pretty() {
self.depth -= 1;
self.and_then(|inner| {
if inner.is_pretty() {
inner.depth -= 1;

if !self.stack.current().is_empty_seq() {
if let Some(delim) = self.delim.take() {
self.fmt.write_str(delim)?;
}
if !inner.stack.current().is_empty_seq() {
if let Some(delim) = inner.delim.take() {
inner.fmt.write_str(delim)?;
}

self.fmt.write_char('\n')?;
pad(&mut self.fmt, self.depth)?;
inner.fmt.write_char('\n')?;
pad(&mut inner.fmt, inner.depth)?;
}
}
}

let pos = self.stack.seq_end()?;
let pos = inner.stack.seq_end()?;

self.delim = self.next_delim(pos);
inner.delim = inner.next_delim(pos);

self.fmt.write_char(']')?;
inner.fmt.write_char(']')?;

Ok(())
Ok(())
})
}

#[inline]
fn map_begin(&mut self, _: Option<usize>) -> Result<(), stream::Error> {
if self.is_pretty() {
self.depth += 1;
}
self.and_then(|inner| {
if inner.is_pretty() {
inner.depth += 1;
}

self.stack.map_begin()?;
inner.stack.map_begin()?;

if let Some(delim) = self.delim.take() {
self.fmt.write_str(delim)?;
}
if let Some(delim) = inner.delim.take() {
inner.fmt.write_str(delim)?;
}

self.fmt.write_char('{')?;
inner.fmt.write_char('{')?;

Ok(())
Ok(())
})
}

#[inline]
fn map_key(&mut self) -> Result<(), stream::Error> {
if self.is_pretty() {
if !self.stack.current().is_empty_map() {
if let Some(delim) = self.delim.take() {
self.fmt.write_str(delim)?;
self.and_then(|inner| {
if inner.is_pretty() {
if !inner.stack.current().is_empty_map() {
if let Some(delim) = inner.delim.take() {
inner.fmt.write_str(delim)?;
}
}
}

self.fmt.write_char('\n')?;
pad(&mut self.fmt, self.depth)?;
}
inner.fmt.write_char('\n')?;
pad(&mut inner.fmt, inner.depth)?;
}

self.stack.map_key()?;
inner.stack.map_key()?;

Ok(())
Ok(())
})
}

#[inline]
fn map_value(&mut self) -> Result<(), stream::Error> {
self.stack.map_value()?;
self.and_then(|inner| {
inner.stack.map_value()?;

Ok(())
Ok(())
})
}

#[inline]
fn map_end(&mut self) -> Result<(), stream::Error> {
if self.is_pretty() {
self.depth -= 1;
self.and_then(|inner| {
if inner.is_pretty() {
inner.depth -= 1;

if !self.stack.current().is_empty_map() {
if let Some(delim) = self.delim.take() {
self.fmt.write_str(delim)?;
}
if !inner.stack.current().is_empty_map() {
if let Some(delim) = inner.delim.take() {
inner.fmt.write_str(delim)?;
}

self.fmt.write_char('\n')?;
pad(&mut self.fmt, self.depth)?;
inner.fmt.write_char('\n')?;
pad(&mut inner.fmt, inner.depth)?;
}
}
}

let pos = self.stack.map_end()?;
let pos = inner.stack.map_end()?;

self.delim = self.next_delim(pos);
inner.delim = inner.next_delim(pos);

self.fmt.write_char('}')?;
inner.fmt.write_char('}')?;

Ok(())
Ok(())
})
}

#[inline]
fn end(&mut self) -> Result<(), stream::Error> {
self.stack.end()?;

Ok(())
self.and_then(|inner| inner.stack.end())
}
}

Expand All @@ -274,3 +322,73 @@ fn pad(mut w: impl Write, amt: usize) -> fmt::Result {

Ok(())
}

#[cfg(all(test, feature = "std"))]
mod tests {
use super::*;

use crate::{
std::{
cell::RefCell,
rc::Rc,
},
value::{
self,
Value,
},
};

#[test]
fn stream_is_poisoned_after_err() {
struct Writer {
poisoned: Rc<RefCell<bool>>,
}

impl Write for Writer {
fn write_str(&mut self, _: &str) -> fmt::Result {
if *self.poisoned.borrow() {
Err(fmt::Error)
} else {
Ok(())
}
}
}

let poisoned = Rc::new(RefCell::new(false));

struct Check {
poisoned: Rc<RefCell<bool>>,
}

impl Value for Check {
fn stream(&self, stream: &mut value::Stream) -> Result<(), value::Error> {
stream.seq_begin(None)?;

// An initial write should succeed
assert!(stream.seq_elem(1).is_ok());

// A subsequent write should fail if the underlying stream fails
{
*self.poisoned.borrow_mut() = true;
}
assert!(stream.seq_elem(1).is_err());

// A subsequent write should fail even if the stream is ok again
{
*self.poisoned.borrow_mut() = false;
}
assert!(stream.seq_elem(1).is_err());

Ok(())
}
}

let _ = write!(
Writer {
poisoned: poisoned.clone()
},
"{:?}",
ToDebug(Check { poisoned })
);
}
}
Loading