alexcrichton
commented
7 years ago I feel like we should be pushing people towards the methods that correspond to the C API
Definitely agreed! Looks like you're already taking care of all that though :)
There are two quotes of interest for the write method:
Heh quite a lot of interesting discussion here! It's true yeah that writing a 100% correct Write implementaiton is actually pretty tricky. I tink you've hit most of the high points, though, and PRs to shore up the docs here in libstd would be most welcome!
aidanhs
Followup to #10 once it gets merged. This is a WIP.
The new design of brotli seems a little odd, but I've tried to represent it as reasonably as possible. Some notes/thoughts so far:
decompress_vec, since it looked like an internal method that got exposed in case others found it useful. Now it's not used internally, I feel like we should be pushing people towards the methods that correspond to the C API.After struggling for some time with it, I'd like to talk about the
Writetrait. As it stands, it's seems extremely difficult to implement it as described in the documentation. There are two quotes of interest for thewritemethod:There are two possible interpretations of 1:
a. 'You can only call
writeonce on each wrapped object' b. 'If a call to write fails, you're not allowed to retry'The implications of these are as follows
1a. Not allowed call
writemultiple times, sowrite_allis clearly forbidden. As we need to ship data to a writer which may be slower than data is coming in, you either need an unbounded buffer or the ability to useOk(0)to indicate you're flushing some data. 1b. Not allowed to usewrite_allas it masksInterrupted. But you can replace it with your ownwrite_allthat bubbles up any error, so this isn't too problematic.Now let's completely ignore 1, and just consider 2. The implication here is that writes are 'retry-safe', i.e. if you get an error you can try again with the buffer you just tried with and if it succeeds then all is well. This forbids
write_allas you can't keep track of where you were up to if an error occurs and you may have already written some of the buffer, making your code retry-unsafe.So,
write_allis basically forbidden by anyone implementingWritewho wants to follow these rules. It turns out that the current implementation in this library useswrite_all, but tries to be retry-safe - I am reasonably confident I can come up with a test that demonstrates this.Looking up the original IO RFC, the intention seems to have been that both 1a and 1b are the desired interpretations. I contend that adhering to these three rules is not possible in a moderately complex writer. Let's take a look a zlib. It mentions the issue I raise about
Ok(0), and of the three options for dealing with 1a (returningOk(0), using an unbounded buffer, ignoring the requirement) has opted for ignoring the requirement, happily attempting to write to a contained object more than once.It's also worth talking about the interaction between 1b and 2 here. A naive implementer following these two rules might take input, process it, and attempt to write all the bytes out in a loop, keeping track of progress. If one of the writes fails, the implementer must return the number of bytes written and so the error needs to be stashed away until the next attempted operation when it can be bubbled up. This is horrible, but can be alleviated by the One True Way displayed in zlib - an inversion of the naive logic I describe above, with fallible IO coming first, followed by consumption of the input buffer that will not fail on IO, effectively queuing the IO for the next call.
I'm just noting down my experiences (this is a terser version of a comment I lost) as I plan to:
Writetrait to make 1a 'best effort'Writetrait page