[Filesystem] Try to rename directories before deleting them #39984
Conversation
|
for 4.4? |
| @@ -170,6 +170,12 @@ public function remove($files) | |||
| throw new IOException(sprintf('Failed to remove symlink "%s": ', $file).self::$lastError); | |||
| } | |||
| } elseif (is_dir($file)) { | |||
| // Rename directory before removing it to mitigate the chance of | |||
| // another process writing to it during the operation. | |||
| // @see https://github.com/symfony/symfony/issues/27578 | |||
There was a problem hiding this comment.
should be removed: we don't link to github in the source
There was a problem hiding this comment.
Nice job for fabbot!?
cc @fabpot
| // another process writing to it during the operation. | ||
| // @see https://github.com/symfony/symfony/issues/27578 | ||
| $orig_file = $file; | ||
| $file = '.symfony-tmp.' . substr(sha1(rand()), 0, 7); |
There was a problem hiding this comment.
the temporary dir has to be in the same parent directory
also, we don't want to rename when removing nested folders of an already renamed one
about the tmp name, we should not use rand nor sha1 imho. Using strrev(strtr(base64_encode(random_bytes(2)), '/=', '-.')) would look better to me. We would delete the previously existing folder in case of collision, to have some very basic last-resort garbage collection...
| // @see https://github.com/symfony/symfony/issues/27578 | ||
| $orig_file = $file; | ||
| $file = '.symfony-tmp.' . substr(sha1(rand()), 0, 7); | ||
| $this->rename($orig_file, $file); |
There was a problem hiding this comment.
if the rename fails, we should ignore and still proceed with the removal to me
|
Thank you for the PR. |
This is a proof of concept solution for #27578. The bug is so old that I'm no longer in a position where I see it regularly, but I'm confident that it is still a bug. This is totally untested but hopefully points people on the right track.