Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

yas3fs deletes the /tmp directory #150

Open
ollietreend opened this issue May 3, 2017 · 3 comments
Open

yas3fs deletes the /tmp directory #150

ollietreend opened this issue May 3, 2017 · 3 comments

Comments

@ollietreend
Copy link

ollietreend commented May 3, 2017

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
Copy link
Collaborator

dacut commented May 3, 2017

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

@liath
Copy link
Collaborator

liath commented Jun 6, 2017

Would shutil.rmtree be the correct choice here?

jazzl0ver added a commit to jazzl0ver/yas3fs that referenced this issue Mar 22, 2018
@jazzl0ver
Copy link
Collaborator

it's fixed

ollietreend pushed a commit to ministryofjustice/wp-ppj that referenced this issue Jul 6, 2018
We should not empty the `/tmp` directory when the docker image is built. The base image `mojdigital/wordpress-base` includes a file called `/tmp/keeptmp` which is used to mitigate a bug whereby `yas3fs` self-destructs by deleting the entire `/tmp` directory. This is a known issue which has been reported to the developers of `yas3fs`: danilop/yas3fs#150

The bug here is that we were emptying the `/tmp` directory at build time, thus disabling the `yas3fs` bug workaround.
jsmcgd pushed a commit to ministryofjustice/wp-ppj that referenced this issue Jul 9, 2018
We should not empty the `/tmp` directory when the docker image is built. The base image `mojdigital/wordpress-base` includes a file called `/tmp/keeptmp` which is used to mitigate a bug whereby `yas3fs` self-destructs by deleting the entire `/tmp` directory. This is a known issue which has been reported to the developers of `yas3fs`: danilop/yas3fs#150

The bug here is that we were emptying the `/tmp` directory at build time, thus disabling the `yas3fs` bug workaround.
jazzl0ver added a commit that referenced this issue Oct 2, 2018
better implementation of cache_path variable access for fixing #150 
multiple other fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants