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

deleteDirectory does not delete directories reliably in NFS environments #1755

Open
Vringe opened this issue Feb 9, 2024 · 0 comments
Open

Comments

@Vringe
Copy link

Vringe commented Feb 9, 2024

Bug Report

Q A
Flysystem Version 3.24.0
Adapter Name flysystem-local
Adapter version 3.23.1
OS FreeBSD 13.2

Summary

I would like to bring up again the discussions from #1135 and #1136 . I think this only affects BSD-based systems.
This is especially a problem in environments, where NFS is used (NAS and also some webhosting providers)

There are old discussions about that:

In the past, we had several customers with different applications complaining about folders, that are not getting properly deleted. This often breaks built-in updaters, but also addons/modules or even core functions of an application.
For example, Shopware uses flysystem to delete folders when installing an update.


If a file is removed from or added to the directory after the most recent call to opendir() or rewinddir(), whether a subsequent call to readdir() returns an entry for that file is unspecified.

This combined with NFS leads leads to some very unpredictable problems as explained above.

I made a PoC to reproduce it. I took the relevant parts from flysystem-local/LocalFilesystemAdapter.php

function listDirectoryRecursively(
    string $path,
    int $mode = RecursiveIteratorIterator::SELF_FIRST
)  {
    return new RecursiveIteratorIterator(
        new RecursiveDirectoryIterator($path, FilesystemIterator::SKIP_DOTS),
        $mode
    );
}

function deleteFileInfoObject(SplFileInfo $file): bool
{
    switch ($file->getType()) {
        case 'dir':
            return @rmdir((string) $file->getRealPath());
        case 'link':
            return @unlink((string) $file->getPathname());
        default:
            return @unlink((string) $file->getRealPath());
    }
}

$folder = __DIR__ . '/poc';

$contents = listDirectoryRecursively($folder, RecursiveIteratorIterator::CHILD_FIRST);

foreach ($contents as $file) {
    if ( ! deleteFileInfoObject($file)) {
        throw new Exception("Could not delete file: " . $file->getPathname());
    }
}

unset($contents);

if ( ! @rmdir($folder)) {
    throw new Exception("Could not delete directory: " . $folder);
}

For testing, I created a folder with 400 files in it

for i in {1..400}; do dd if=/dev/zero of=poc/file_$i bs=1K count=1; done

After executing it, there are still a lot of files left in the directory and an exception is thrown because the poc folder is not empty.

Now, If I wrap iterator_to_array() around the return value of listDirectoryRecursively, the problems disappear, because all the files in the directory are first discovered and then deleted. This may not be the best solution to work around this problem. I think it would be better to sort of read for example 20 entries in that directory, delete them and then read the next 20 entries until it is empty.

I would be very grateful if we can implement a solution for BSD-based systems as this library is widely used in many applications. Of course, I'm happy to help where I can if you need to test something.

Nextcloud also had some recent changes related to this, if you want to get some inspiration.
nextcloud/updater#515

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

1 participant