Disposal and composing filesystems

[Rohansi]

Right now using any of the ComposeFileSystems will lead to missed Dispose calls because none of them call Dispose on the filesystems they have references to.

IMO they should (optionally) take ownership of the filesystem instances and dispose of them as necessary. Either when they get disposed or they are being removed from the composed filesystem.

[Rohansi]

It would also be good to make sure that an exception thrown in Dispose doesn't leave the composed filesystem in an undefined state.

[xoofx]

Yeah, ownership is always coming at some point and I don't like to have to decide this... because it is very annoying here, depending on your cases you might want to avoid entirely the auto-dispose pattern, but if you start to introduce the choice, you have to keep a boolean around (and it gets complicated if you have a list...)

IMO they should (optionally) take ownership of the filesystem instances and dispose of them as necessary. Either when they get disposed or they are being removed from the composed filesystem.

For a plain chaining FileSystem (having only a backend FileSystem), we could have something similar to StreamReader where you can specify ownership at constructor time.

For composite with multiple filesystems, not sure I would like to introduce such a behavior.

So, yep, that's not an easy feature, we will have to dig into how to properly handle this.

[Rohansi]

I feel that requiring manual disposal isn't intuitive. I am using AggregateFileSystem and MountFileSystem to construct a desired directory structure and then only accessing the "root" IFIleSystem. Cases like this might be common and reimplementing the bookkeeping so Dispose can be called on removal is quite annoying.

[xoofx]

I feel that requiring manual disposal isn't intuitive. I am using AggregateFileSystem and MountFileSystem to construct a desired directory structure and then only accessing the "root" IFIleSystem. Cases like this might be common and reimplementing the bookkeeping so Dispose can be called on removal is quite annoying.

You can try to bring this feature. What I really don't want is a dispose when removing an element, otherwise you can't transfer ownership after adding to an aggregate filesystem.

[Rohansi]

That's a good point. There could end up being many places where we would need a bool to control disposal.

[xoofx]

That's a good point. There could end up being many places where we would need a bool to control disposal.

If we can avoid this I would prefer. At worst, allow it for immutable backend fs (Chain). For aggregate, assume that once it is part of an aggregate, the aggregate is responsible for disposing it if the aggregate is disposed. but we will not provide a fine grain control over the disposing. Just that if you want to avoid the dispose, you need to remove it. The dispose pattern is really not great when it comes to resource sharing, as it would require a real AddReference/ReleaseReference to make it viable... but I don't want to introduce such a burden. So let's keep it simple, single ownership, auto-dispose (controllable with a bool for simple chain fs) and side free effect on remove from aggregate.

[Rohansi]

That sounds good. So pass true when constructing and then it will call Dispose only when that filesystem is being disposed?

[xoofx]

That sounds good. So pass true when constructing and then it will call Dispose only when that filesystem is being disposed?

Yep. Existing constructors taking a FS is redirecting to a new constructor accepting a owned (true for this case) Aggregates would dispose whatever they have in their lists, no bools there...