On 04/25/2013 12:09 PM, Graydon Hoare wrote:
On 25/04/2013 7:18 AM, Gábor Lehel wrote:
I'm an outsider, so apologies if I'm off the right track. But this
sounds wrong to me. Isn't the point of condition handlers to /handle the
condition/? Meaning resolve the problem, so that processing can
continue? Here as far as I can tell, they're only being used to say
"fail differently", which then leads to failing again the next time you
try to do something. Opening a file was actually one of the examples
used to illustrate the condition system way back when[1].
Yeah. I read this design a few times and felt uneasy about the
combination of files, options and conditions. I couldn't put my finger
on exactly how to arrange it, but it felt off. I thought about it some
more and I think I can now put my concern plainly.
When a condition is handled -- not-in-failure -- there are _many_
possible ways the user might want to handle it. All these are plausible:
- This error is ok but subsequent errors on the same filehandle
should cause retriggering of the condition (so we can count
errors and fail after too many).
- This error is ok and all subsequent errors on the same filehandle
should be absorbed silently; turn it into /dev/null.
- This error should cause a state-change to the existing file,
such as closing it, truncating it, creating it, reopening it,
or similar. If it's a socket, maybe we want to try reestablishing
it. Maybe redirect it to an in-memory pseudo-file.
Because of this variety of possible solutions, the approach in the
current design is inadequate: the Some(existing_file) and None
strategies are only two of the possible strategies, and both are
statically dispatched due to the implementation of file methods on
Option<File>. The condition handler isn't really involved in picking
the strategy, only choosing between a fixed menu. It should be able to
_provide_ a strategy, which means providing an implementation of File
(or more likely: ~Stream).
In other words (very much thinking-out-loud here) I suspect that any
concrete stream type in our IO library that has serious error modes
like this ought to be able to "replace itself" after a failed
operation with a ~Stream that points to some other implementation. So
it should be something like:
enum FileStream {
FileDescriptor(libc::fd_t),
FileSimulation(~Stream)
}
Having your stream type be called `FileStream` instead of
`Option<FileStream>` is nice. I would still want a way here to recover
without allocating, making this data type more like
enum FileStream {
RtFileStream(rtio::FileStream), // Not an fd_t, a boxed wrapper
around uv/rt handles
FileSimulation(~Stream),
NullFileStream
}
With this factoring though you are going to be duplicating a lot of
logic for FileStream, TcpStream, etc. - they all have to define this
enum, handle the pass-through, and null cases.
and so forth on other types. Then your file conditions would look like:
condition! {
no_such_file : Path -> FileStream;
}
and
condition! {
write_error : (libc::fd_t, libc::errno_t) -> FileStream;
}
I think the idea here is that you could take your open file handle and
possibly put it into a new FileStream. In that case presumably it would
be the condition handler's responsibility to close the file handle, so
it would need to be wrapped in some RAII.
The main I/O implementation will not be dealing in file descriptors
though, it will be using boxed types defined in the `rtio` module that
wrap Rust uv handles that wrap native uv handles, none of which should
be exposed in the public interface. A condition that exposes C types is
not appropriate there. Furthermore, if we intend for the 'native' and uv
implementations to be interchangeable then they must raise the same
conditions. A generic write error condition could be more like:
condition! {
file_write_error: (IoError) -> FileStream;
}
This though doesn't give you any access at all to the original I/O
stream or handle. You could instead define it more like
condition! {
file_write_error: (IoError, &mut FileStream) -> ();
}
Which would let you both inspect the original or replace it (though I
don't think you can put borrowed pointers in conditions today).
and we'd assume that any handler for that condition will either
provide you a "real" FileDescriptor or else a "fake" simulation of a
file that directs to some other Stream type. And the 'write' operation
would look like:
fn write(&mut self, bytes: &[u8]) {
match self {
FileSimulation(s) => s.write(bytes),
FileDescriptor(fd) => {
let e = libc::write(bytes.to_ptr, bytes.len());
if e != 0 {
*self = no_such_file.cond.raise(fd, e);
}
}
}
}
Apologies for the sketchines; I realize real code looks gnarlier than
this, just trying to convey a possible strategy. I say all this,
again, because the alternatives (option<File> or a single "bad" flag,
as in many IO libraries) seems to provide very few recovery options to
a user.
(Also aware that getting the abstraction layer right might be
challenging; maybe FileStream is the wrong place to put in this
redirection, and it should be one layer further up, or down..)
It's not clear to me if MemStream or other such things have quite as
wide a repertoire of conditions. I think if there are no operations
that raise conditions of this sort, then there'd be no reason to use
such an enum.
-Graydon
Thanks for the feedback.
-Brian
_______________________________________________
Rust-dev mailing list
[email protected]
https://mail.mozilla.org/listinfo/rust-dev
