RecursiveDirectoryIterator misbehaves if the directory gets modified while iterating about doc-en HOT 14 OPEN

So i’d guess that PHP is not at fault here, it just behaves the same way as the operating/file system it’s dealing with does.

The "fault" of PHP is that it does not document the guarantees (not) given. At least I could not find any documentation on this topic.

from doc-en.

Dear all

Thanks for your valuable comments and your support on this!

I did more tests:

Original:

foreach (getRecursiveDirectoryIterator("src") as $path) {
[..]

-> 522: src/1000 -> dst/1000 -> bad

Intermediate variable:

$iter = getRecursiveDirectoryIterator("src");
foreach ($iter as $path) {
[..]

-> 522: src/1000 -> dst/1000 -> bad

File list:

$fileList = iterator_to_array(getRecursiveDirectoryIterator("src"), true);
foreach ($fileList as $path => $fileInfo) {
[..]

-> 1000: src/1000 -> dst/1000 -> ok

This answers my question, an intermediate variable ($iter) does not contain the filelist but the iteration object:

print_r($iter);
/*RecursiveIteratorIterator Object
()*/

where as the filelist is a pure array:

print_r($fileList);
/*Array
(
    [src/1] => SplFileInfo Object
        (
            [pathName:SplFileInfo:private] => src/1
            [fileName:SplFileInfo:private] => 1
        )

    [src/2] => SplFileInfo Object
        (
            [pathName:SplFileInfo:private] => src/2
            [fileName:SplFileInfo:private] => 2
        )
[..]*/

So the usage of iterator_to_array() is a must.

from doc-en.

I found an answer to a similar question that suggests that this behavior undefined and depends on the operating system and file system: https://stackoverflow.com/a/39017355/1031606

So i’d guess that PHP is not at fault here, it just behaves the same way as the operating/file system it’s dealing with does.

There is also an article from Apple that explicitly states that this is not supported on their HFS file system: https://web.archive.org/web/20220122122948/https://support.apple.com/kb/TA21420?locale=en_US

from doc-en.

I agree that it just is a lack of documentation. IMO a critical one.

If you're satisfied with this being a documentation problem, I can move it to the appropriate place:

from doc-en.

See also nextcloud/updater#509 (comment) for further information

from doc-en.

FYI: Fixed version is in the example code. Guess that should not be the case.

ps: Thanks a lot for your work on the Nextcloud/Hostpoint/update issue. I'm not a PHP dev, but pretty sure that using a directory iterator while manipulating the underlying folder is a bad idea.

from doc-en.

Iterating on a directory's contents while you are actively modifying its contents feels risky just on principle. It would seem alright to automatically "snapshot" one particular directory's contents before returning its data for iteration, but that also means not being able to pick up any changes to the directory - desirable or not. Then the recursive iteration is harder: if you were recursing on ./ and the first directory it found was src, after those files got moved, would you expect that the iterator did or did not subsequently discover them existing in dst?

All in all, the safest course of action would certainly be to make your own snapshot before performing any actions (like with iterator_to_array), then doing any necessary last-minute sanity checks before making each change, but I wouldn't yet discount there being something that could be done here.

from doc-en.

I: Fixed version is in the example code. Guess that should not be the case.

Thanks, I corrected the example.

ps: Thanks a lot for your work on the Nextcloud/Hostpoint/update issue. I'm not a PHP dev, but pretty sure that using a directory iterator while manipulating the underlying folder is a bad idea.

Thanks, hopefully they will accept my fix. Currently it looks like they do not want to accept it because they officially do not support BSD, see nextcloud/updater#510 (comment)

@damianwadley Thanks for your comment.

Oddly, therere are many examples on the web doing it without iterator_to_array(), and nowhere a note that it is dangerous.
Since I am not enough into iterators, is there a difference between

$iterator = new DirectoryIterator($dir);
foreach ($iterator as $fileinfo) { .. }

and

foreach (new DirectoryIterator($dir) as $fileinfo) { .. }

?
Is the 2nd one save (altough it is not using the iterator_to_array()?

from doc-en.

Thanks, hopefully they will accept my fix. Currently it looks like they do not want to accept it because they officially do not support BSD, see nextcloud/updater#510 (comment)

Read it and I think they worded it badly and/or got it wrong. But their motivation to understand and fix the underlying issue is sound. And in this case, once it can be shown that there is no (vanilla) documentation stating that their usage of the iterators is save, then I am sure they will accept a fix and everyone, including users on their supported platforms, wins.

from doc-en.

Oddly, therere are many examples on the web doing it without iterator_to_array(), and nowhere a note that it is dangerous.

Would flip the implied question: Where can one find authoritative documentation stating that it is save to keep using a DirectoryIterator instance once its content got manipulated?

Any chance a PHP developer can help us out here?

from doc-en.

@caco3 can you test if the same issue also happens with readdir()?

$i = 0;
mkdir("dst");
$handle = opendir("src");
while (false !== ($entry = readdir($handle))) {
    $i++;
    $path = "src/$entry";
    $newPath = "dst/$entry";
    echo("$i: $path -> $newPath\n");
    rename($path, $newPath);
}
closedir($handle);
echo("EOF");

from doc-en.

Yes, same issue:

[..]
95: src/93 -> dst/93
96: src/94 -> dst/94
97: src/189 -> dst/189
98: src/190 -> dst/190
[..]
524: src/1000 -> dst/1000
EOF

from doc-en.

I agree that it just is a lack of documentation. IMO a critical one.
I hope the devs will pick it up and document it for the RecursiveDirectoryIterator() as well as readdir().

from doc-en.

Yes, please go ahead

from doc-en.