rename fails on Windows PHP 8.1 if the target file is being executed

[johnstevenson]

Description

The following code:

<?php
// file: bug.php

$content = file_get_contents(__FILE__);
file_put_contents('newfile.php', $content."\n//");
var_dump(rename('newfile.php', __FILE__));

Resulted in this output:

PHP Warning:  rename(newfile.php,C:\Users\John\projects\test\bug.php): Access is denied (code: 5) in C:\Users\John\projects\test\bug.php on line 6

Warning: rename(newfile.php,C:\Users\John\projects\test\bug.php): Access is denied (code: 5) in C:\Users\John\projects\test\bug.php on line 6
bool(false)

But I expected this output instead:

bool(true)

This code mimics how Composer self updates (https://getcomposer.org/doc/03-cli.md#self-update-selfupdate-) and has obviously worked for many years, but is broken on Windows PHP 8.1: https://github.com/composer/composer/issues/10444

Note that on Composer 2 we now use copy on Windows (to get around potential permission issues in UAC protected locations) and this still works on PHP 8.1, but Composer 1 uses rename (as do all non-Windows platforms on all Composer versions).

Fortunately, this appears to be a Windows only thing, as per this demo: https://github.com/johnstevenson/php-rename-bug/actions/runs/1669358275

PHP Version

PHP 8.1.0

Operating System

Windows 10

[cmb69]

The regression is apparently caused by https://github.com/php/php-src/commit/c732ab400af92c54eee47c487a56009f1d79dd5d. I shall have a look.

[johnstevenson]

Hey thanks. It seems like the file is locked, because cmd.exe move also fails when we call it as part of our UAC elevation stuff.

[cmb69]

The file is not locked in a strict sense, but the handle is kept open after compilation, and that prevents the file to be renamed on Windows.

[johnstevenson]

Ah, so presumably the file has not been opened with FILE_SHARE_DELETE access.

[cmb69]

The file is opened in FILE_SHARE_DELETE mode, but apparently, moving an opened source file is possible, while moving to an opened destination file is not:

C:\php-sdk\phpdev\vs16\x64
$ echo foo>foo

C:\php-sdk\phpdev\vs16\x64
$ echo bar>bar

C:\php-sdk\phpdev\vs16\x64
$ type movefile.c
#include <Windows.h>

int main(void)
{
    HANDLE file = CreateFileA(
        "bar",
        GENERIC_READ | GENERIC_WRITE,
        FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
        NULL,
        OPEN_EXISTING,
        FILE_ATTRIBUTE_NORMAL,
        NULL);

    BOOL res = MoveFileExA(
        "foo",
        "bar",
        MOVEFILE_REPLACE_EXISTING | MOVEFILE_COPY_ALLOWED);
    printf("%d", res);
    return 0;
}

C:\php-sdk\phpdev\vs16\x64
$ cl movefile.c
Microsoft (R) C/C++-Optimierungscompiler Version 19.29.30138 für x64
Copyright (C) Microsoft Corporation. Alle Rechte vorbehalten.

movefile.c
Microsoft (R) Incremental Linker Version 14.29.30138.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:movefile.exe
movefile.obj

C:\php-sdk\phpdev\vs16\x64
$ movefile.exe
0

C:\php-sdk\phpdev\vs16\x64
$ type foo
foo

C:\php-sdk\phpdev\vs16\x64
$ type bar
bar

I think we either need to close the script file right after compilation, or accept that this is just not supported on Windows. I'd prefer to do the former, unless there is a good reason to keep the script file open.

[johnstevenson]

Thanks for the demo. That makes sense (I should have thought about it a bit more).

I'd prefer to do the former, unless there is a good reason to keep the script file open.

Let's hope there isn't. I appreciate your time and involvement in this. Moving to Github issues has made PHP bugs a lot more interesting and compelling - for me anyway.

[johnstevenson]

@cmb69 Apologies for the BUMP, but it would be good if this could be fixed.

[Voltra]

This also happens (sometimes) when files are being cached using Symfony or Laravel's default cache and filesystem adapters

(Happens on 7.4.26 too, from Wamp 3.2.6)

[Voltra]

In the meantime, here's the rector config I use to circumvent the issue:

// rector.php
$rectorConfig->paths([
    __DIR__ . '/src',
    __DIR__ . '/vendor/symfony/',
    __DIR__ . '/vendor/doctrine/',
   // Add any dependency that causes issues, adding the whole vendor directory might be too much on your PC
]);

$rectorConfig->ruleWithConfiguration(RenameFunctionRector::class, [
    'rename' => 'windows_rename_fix',
]);

// monkey_patch.php
function windows_rename_fix(string $oldName, string $newName, $context = null): bool {  
    $didCopy = copy($oldName, $newName, $context);

    if (!$didCopy) {
        return false;
    }

    @unlink($oldName, $context);

    return true;
}

if (!function_exists('rename')) {
    function rename(string $oldName, string $newName, $context = null): bool {
        return windows_rename_fix($oldName, $newName, $context);
    }
}

And I add the monkey_patch.php in the autoload.files array in my composer.json

[lufog]

Does this relate to the fact that in PHP 8 (8.1.27, 8.2.16, 8.3.3), on Windows, unlink() cannot delete a currently running script? Since PHP 7 (7.4.33) doesn't have this problem, I guess it's a regression?

[nicolas-grekas]

Is this issue still relevant? Didn't @cmb69 fix it in c7f24d9?

[nicolas-grekas]

Ah, yes, #9351 is still opened as draft. Would be could is someone could take over!

[Voltra]

I'm not sure if I could reproduce it as it was very "random" as to when the problem would occur, and I don't currently have access to the repo that had the issue