File::set_len uses an immutable reference to self

[vitiral]

I am creating a wrapper for the File type and came across an issue that I thought was odd.

• File::set_len
• File::set_permisions

Both of these methods take an immutable reference (&self) to the File instead of a mutable reference (&mut self).

I'm not so much concerned about memory safety per se, as I assume that the operating system guarantees that operaions are memory safe, it is just that this is extremely unexpected. If I pass a function an immutable reference to a File, I wouldn't expect it to be able to truncate the file!

Obviously this is a breaking change so probably can't be changed -- however, I was thinking we could at least add docs explaining why the reference is immutable.

[vitiral]

I do wonder how much this would break people in the wild though. How many people are truncating or extending a file that they don't have a mutable reference to?

[ExpHP]

I do not think this is too odd considering the much more frequently-encountered gotcha! that &File implements Read and Write. It's all fallout from the fact that mut doesn't really mean "mutable". I agree that there isn't much that can be done here beyond documentation and education.

[vitiral]

I'm also not following this with my path_abs crate to prevent any gotchas. Would love feedback if that is a bad idea!

[ExpHP]

I'm also not following this with my path_abs crate to prevent any gotchas. Would love feedback if that is a bad idea!

I looked at the crate and didn't see any &mut self functions, so I'm not sure what you're referring to; but you should keep in mind multithreaded use cases. &File needs to implement the things it does to enable things like e.g. writing to a File from inside a rayon ParallelIterator.

In your case, though, even if there were &mut self functions that did not strictly have to be &mut, it would be little more than a speedbump, since all of the objects in your crate are easily cloned (since they are really paths, and not Files).

[vitiral]

@ExpHP thanks for looking at the crate! Sorry I should have been more clear. I am currently implementing a PathOpen object which looks like a File but has a .path() method to get a PathFile and its error messages include the path where the error origininated (a serious annoyance with std::fs::File).

It will be slightly more difficult to clone PathOpen so I am interested in what you are saying. What would be the behavior of having multiple threads writing to the same &File? Would there be any guarantee that the written data wouldn't be interleaved in some weird and interesting way?

If you had thread_a writing "foobar" and thread_b writing "bazbob" is there any guarantee that you wouldn't have "foobazbobbar" or any other permutation of the letters?

If there are no such guarantees then I would prefer to require the user to do a producer/consumer structure for sending data to be written. This is just personal preference to try and avoid gotchas.

Would love any criticism/feedback! Thanks!

[ExpHP]

If you had thread_a writing "foobar" and thread_b writing "bazbob" is there any guarantee that you wouldn't have "foobazbobbar" or any other permutation of the letters?

I doubt it. I guess I was thinking more along the lines of there are times where I simply don't mind jumbled output (e.g. println tracing) and from that I extrapolated that there must be times where jumbled output could even be considered neccessary or unavoidable due to the nature of what's being written.

...but on further thought, I can't think of any example that isn't terribly contrived. So for your library, I don't imagine anything of real value will be lost by eliminating this footgun.

[vitiral]

cool, thanks! 👍

[vitalyd]

The fact that &File can be mutated is a very common headscratcher for beginners, right after they’ve heard about Rust’s strict mutability rules. The standard answer seems to be: Rust prevents data races for memory safety purposes, and not for modeling any extra semantics on top. In other words, if the OS provides memory safe mutation over aliased file handles, Rust-the-language will happily allow it to occur in your code too. Of course someone writing the File type could’ve made the design decision to not allow this footgun but now it’s a “policy” decision they’re making, and not purely memory safety.

The good news is one can define their own type and add extra semantics (eg prevent mutation unless you have a unique borrow).

[vitiral]

@ExpHP @vitalyd I've just released path_abs v0.3.0 and here is the docs related to this issue:

https://docs.rs/path_abs/0.3.0/path_abs/#differing-method-signatures

Would appreciate a review!

[alercah]

Thought: how about a general warning on the type in general, plus smaller notes saying "Note that this method can change the file via a shared reference."

[steveklabnik]

@alercah I like it, but I'd remove "shared"; the proper terms are "reference" and "mutable reference", even though sometimes people do say "shared reference" for &T, I try to keep things consistent. Alternatively, just say &T straight-up so there's even less confusion 😄