thephpleague / flysystem

Abstraction for local and remote filesystems
https://flysystem.thephpleague.com
MIT License
13.23k stars 825 forks source link

deleteDirectory does not delete directories reliably in NFS environments #1755

Open Vringe opened 3 months ago

Vringe commented 3 months ago

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. https://github.com/nextcloud/updater/pull/515