danilop / yas3fs

YAS3FS (Yet Another S3-backed File System) is a Filesystem in Userspace (FUSE) interface to Amazon S3. It was inspired by s3fs but rewritten from scratch to implement a distributed cache synchronized by Amazon SNS notifications. A web console is provided to easily monitor the nodes of a cluster.
http://danilop.github.io/yas3fs
MIT License
643 stars 98 forks source link

yas3fs deletes the /tmp directory #150

Open ollietreend opened 7 years ago

ollietreend commented 7 years ago

Hi,

I've discovered a specific set of conditions under which the entire /tmp directory is deleted when yas3fs cleans its cache.

This causes all sorts of problems in Linux since /tmp is a system directory that should always exist.

How to recreate

  1. Start with an empty /tmp directory.
  2. Run yas3fs and add a new file to the mounted directory.
    • The local filesystem cache is created at /tmp/yas3fs/...
  3. Delete the file you just added to the mounted directory.
    • yas3fs removes that file from its cache
    • empty cache directories are recursively deleted using python's os.removedirs method
    • because that was the only file in the cache, the entire /tmp/yas3fs directory is deleted
    • /tmp is now empty so it gets deleted

Why it happens

This happens because FSCache.delete() calls FSData.delete(), which calls remove_empty_dirs_for_file(), which calls remove_empty_dirs(), which uses os.removedirs().

The docs for os.removedirs() say that:

removedirs() tries to successively remove every parent directory mentioned in path until an error is raised (which is ignored, because it generally means that a parent directory is not empty).

Therefore if /tmp is empty after deleting /tmp/yas3fs, then it will be deleted too.

What should happen

yas3fs shouldn't recursively delete beyond its own cache path (i.e. the --cache-path option, in this case /tmp/yas3fs)

It might be necessary to implement a custom recursive delete function rather than use os.removedirs(), since it doesn't look like it's possible to limit that to a specific root directory.

dacut commented 7 years ago

<sigh> This isn't the first time I've seen os.removedirs() misused. It's an unintuitive API.

liath commented 7 years ago

Would shutil.rmtree be the correct choice here?

jazzl0ver commented 6 years ago

it's fixed