Make API more idiomatic about fuse-rs HOT 14 OPEN

I've been thinking about how to make the API more idiomatic and have come up with some ideas.

We need to keep ensured that (a) the actual implementation can only reply once to each request and (b) an error reply is sent if the implementation forgets to reply at all.

A fs operation method currently looks like this:

fn lookup(&mut self, _req: &Request, _parent: u64, _name: &OsStr, reply: ReplyEntry);

This ensures (a) by passing the reply object which has self-consuming ok/error methods and (b) because the reply object sends an error if dropped without calling ok or error.

More idiomatic would be to return a result:

fn lookup(&mut self, _req: Request, _parent: u64, _name: &OsStr) -> Result<ReplyEntry, Error>;

Both (a) and (b) would be covered because the implementation must return exactly one result. Error would be a rust-fuse specific wrapper type for errno (similar to std::io::error::Repr::Os) probably with some conversion methods for convenience.

Since ReplyEntry needs request-specific information, it either needs to be created by calling a method on _req or by implementing From<Request> so that _req can be turned into a reply.

This however might allow people to mistakenly create a wrong reply type and mess things up if they manage to interchange replies between different requests to satisfy the return type.

ReplyEntry might better be named Entry or turned into generic type Reply<Entry> (I'd prefer Entry for less clutter)

To make it asynchronous, we can use futures instead of results. However since Future is a trait, we would require the "impl traits" feature, which is only available in nightly rustc yet.

fn lookup(&mut self, _req: Request, _parent: u64, _name: &OsStr) -> impl Future<Item=ReplyEntry, Error=Error>;

Without impl traits, we need to box the returned type. This works with stable rustc, but would require an allocation for every returned value (!)

fn lookup(&mut self, _req: Request, _parent: u64, _name: &OsStr) -> Box<Future<Item=ReplyEntry, Error=Error>>;

We could either implement asynchronous operations in a different trait (which would end up in duplicating the large Filesystem trait using slightly different method signatures. Or move to use futures exclusively (which would work fine even for synchronous implementations since every Result can be turned into a future that resolves immediately)

With futures, it's getting a little more complicated.. The Filesystem trait might be easy to switch to futures (with impl traits being the only blocker). To fully take advantage of asynchronicity, the session's run method needs to return a future that the developer can put into a reactor to run the session loop and dispatch filesystem operations.
Moreover, the kernel communication (channel) needs to be future-based. This is where it requires more than just future-rs. As far as I can see, this would require a dependency on tokio-core/tokio-io/mio for async RawFd communication which I wouldn't be very happy about.

Thoughts/ideas appreciated

from fuse-rs.

The reason why the Filesystem trait in rust-fuse uses a Reply argument instead of returning is to allow for asynchronous response. If I'm understanding this feature request correctly, this would not be an improvement, but a setback :/.

from fuse-rs.

Maybe then return a Future<Reply*>? See #80

Another problem is that you can call .error and such multiple times. I'm thinking something like that for readdir:

let mut reply = Reply::directory();
reply.add(...)
if ok {
    reply.ok()
} else {
    reply.error(ENOENT)
}

with Reply::directory and such static methods of the trait which returns the corresponding struct.

from fuse-rs.

error/ok take self by value, so you can't call them multiple times.

from fuse-rs.

Hum, right XD

from fuse-rs.

If rust-fuse moves to tokio/mio, then maybe it could make sense to return a Future<Reply>. But in the interim, I don't think it makes too much sense as it only imposes a specific kind of abstraction on the developer.

I have some WIP stuff on integrating tokio with rust-fuse and I think it's possible to do this without introducing breaking changes (by making a new FilesystemFutures trait and a wrapper struct). I still need to figure out how to allow the methods access the tokio event loop (need a Handle or something somewhere).

from fuse-rs.

I'd also really like to see a more idiomatic API. Back when I wrote the Filesystem trait, it wasn't really clear how cases like these should be handled idiomatically. I like the idea to use Future<Reply> as return types, but there are some problems with that:

• In general, I'd like to prevent dependencies that would enforce developers to use a specific I/O implementation, like tokio or mio. I think future-rs would be ok (mainly traits), but I wouldn't like to depend on tokio (don't get me wrong, I like tokio, but I don't want to force others to use it).

• The current trait was designed to enforce the user to always reply (to prevent leaking unreplied requests. If the reply object is dropped, a default reply is sent). Having a sendable reply object allows implementations to reply asynchronously. This could be done with return types as well (and would be more idiomatic). Since Future<T> is implemented by T, it would nicely accept immediate values being returned for synchronous operations and optionally some future value for asynchronous operations. However, the reply does need to know the request id when it's sent. Currently, the reply gets the request id when it's created by the request dispatcher. If the user creates the reply object himself, how would we know the proper request id when actually sending the reply?

• I think a Filesystem trait using future return values could work, but I'm unsure how we can keep the request/dispatch loop encapsuled from the user without depending on a concrete async I/O implementation like tokio.

from fuse-rs.

I still don't quite get why using return (without any explict async io crates [e.g. tokio] in the API) makes it impossible to perform async response...

Does the function that calls these functions waits until these functions to return?

• If yes, how does using parameter instead of return value makes it async?
• If not, why can't it spawn asynchronously this function and any following code?

Any pointing to relevant code is appreciated.

from fuse-rs.

The point is that if it's passed as a parameter, the driver can then push the request/response to a thread pool or an event queue. rust-fuse would still block on it returning, but the function could return immediately after pushing the request on a work-stealing task queue.

from fuse-rs.

The reply parameter doesn't make it async. The key thing is, that it doesn't prevent you from making it async. Passing the reply parameter gives the implementor freedom to either handle it asynchronously or synchronously.

• Synchronously: do processing, use reply parameter to send the result, done
• Asynchronously: spawn thread/task passing the reply parameter, return immediately. Within the thread/task do the processing and send the result later.

That's also the reason why the function gets ownership of the reply parameter (it's moved into the function), it can pass it on to other threads. Providing the reply as return values would effectively prevent any async processing since the function would need to provide the reply value to finish. The internal kernel communication and dispatching is single threaded, so yes, the next operation can't be dispatched until the function returns.

This design decision is quite old. I think there was no tokio or anything async back that time. There's some discussion about it in #1 :)
Nowadays, with Rust getting native futures and an ecosystem of async stuff is building up, I think this API should be redesigned and use futures (which provide the same functionality in a more obvious way). I'm working on redesigning the dispatch so that we get a fully async and more idiomatic interface (currently in the modernize branch. Hopefully I'll find some time in the next days to bring it into a working state and merge it)

However, using async in Rust still has a few drawbacks, e.g. async fns can't be used in traits yet and it still needs a nightly Rust to be compiled, so rust-fuse 0.4.0 will probably stay alpha/beta until futures in Rust become available in stable.

from fuse-rs.

(This is just for the period before async is fully integrated into rust. I agree using more native and explicit support is always better.)
(I'll try to catch #1, which is a bit too long for me to read now...)

So, I mean, I see waiting for functions to return is a sequential operation. But the caller can always spawn a thread (or whatever async methods) and call this function in that thread (as well as performing any further work there, after the function call returns).

I didn't dig into the detail of rust-fuse's implementation, but for whatever way you make it possible to allow the user to decide either to go async or not requires an async way of handing the value change (or consumption) of reply, don't you? So why not also put calling the function there?

from fuse-rs.

But the caller can always spawn a thread ...

The caller of the Filesystem methods is the session loop of rust-fuse that dispatches requests from the kernel driver to the user code. Spawning a thread there would be possible, but that'd part of the rust-fuse code and the idea is to give downstream implementations the freedom to choose whatever concurrency technology and strategy they like, therefore it needs to be done in the function implementation.

... but for whatever way you make it possible to allow the user to decide ... requires an async way of handing the value change (or consumption) of reply, don't you? So why not also put calling the function there?

Not exactly. The reply type is crafted to be able to send the reply back to the kernel driver on its own. It doesn't get processed by an outer loop.

from fuse-rs.

I have some thoughts on an approach that could help make everyone happy.

Firstly, keep the current interface. It's flexible for the reasons you stated, and works. The Filesystem trait isn't quite low level in FUSE terms, but it certainly feels lower than FilesystemMT. You could call it a mid level API if you want!

Second, let folks who want a fully synchronous API use fuseMT. You could mention it in the docs, or even merge it into the crate.

Finally, handle async io by not handling it, and instead offering ways to serialize / deserialize the messages from a byte buffer. This "network protocols sans i/o" philosophy is the most flexible way to do this, in my opinion. And if you want to get fancy for async's sake, you could have it work with N byte buffers for vectored io.

This is the design I'm going for with nine, which occupies a similar niche as rust-fuse. (I'd love to help with this, by the way. Especially since 9pfuse isn't packaged in homebrew anymore, so I want to make a replacement using rust-fuse!)

from fuse-rs.

I did some cleaning and improvements on the code in the modernize branch, which now also has a new version of the Filesystem trait that's using return values for replying: filesystem.rs. Comments / feedback welcome (MR is #126).
It can't be used yet since it's still missing the request dispatch mechanism, but I'm working to get that done as well.

I'm in favor of making the API async (which can easily be done now by making every method async), however since Rust doesn't support async methods in traits yet, that'd mean to either use the async-trait crate or use boxed future types as return values (very inconvenient).

Unfortunately, it would be hard to have both versions, a sync one and an async one at the same time. Making the trait async only if a certain feature is turned on seems a bit cumbersome. Easiest would be to change the return type to a boxed future of a reply, but that'd incur the cost of boxing the return value for each method call. Maintaining two filesystems traits, one with sync methods and one with async methods, seems too inconvenient as well.

from fuse-rs.