php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #79865 Requiring a file twice with `require` doesn't reload anonymous classes
Submitted: 2020-07-15 21:04 UTC Modified: 2020-07-17 12:56 UTC
From: d28b312d at opayq dot com Assigned: nikic (profile)
Status: Closed Package: *General Issues
PHP Version: Irrelevant OS: N/A
Private report: No CVE-ID: None
View Add Comment Developer Edit
 [2020-07-15 21:04 UTC] d28b312d at opayq dot com 
Description:
------------
Requiring a file twice (using `require` _not_ `require_once`), will reload a file and run all the code but won't redefine anonymous classes in that code.

The file is loaded and the code to define the object is run, but as the hash is the same it doesn't redefine it. The echo statement above the anonymous class _is_ run both times and is different both times, as expected.

There is _no_ warning and _no_ change, but the rest of the code still continues to run.

This seems like very unexpected behaviour.

This is the vld output gist: https://gist.github.com/ElvenSpellmaker/940fa8b4fd5523a81a42fb9686266d67

From that gist, the first time the class is loaded:
-------------------------------------------------------------------------------------
   3     0  E >   ECHO                                                     'Title%3A+foo'
         1        ECHO                                                     '%0A'
   5     2        DECLARE_ANON_CLASS                               9.88131e-324
         3        NEW                                              $1      0
         4        DO_FCALL                                      0
         5      > RETURN                                                   $1
   9     6*     > RETURN                                                   1

And the second time:
-------------------------------------------------------------------------------------
   3     0  E >   ECHO                                                     'Title%3A+bar'
         1        ECHO                                                     '%0A'
   5     2        DECLARE_ANON_CLASS                               9.88131e-324
         3        NEW                                              $1      0
         4        DO_FCALL                                      0
         5      > RETURN                                                   $1
   9     6*     > RETURN                                                   1

As you can see there doesn't appear to be a point where it skips the redefine, but I assume as the hash comes back as '9.88131e-324' both times it doesn't actually declare the class again.

Test script:
---------------
index.php:
<?php

$class = require 'test.php';
echo 'Get Name: ' . $class->getName();

# Replace foo with bar to try and test a class reload
$foo = file_get_contents('test.php');
$foo = str_replace('foo', 'bar', $foo);
file_put_contents('test.php', $foo);
echo "\n";

$class2 = require 'test.php';
echo 'Get Name: ' . $class2->getName();

# Reset file
$foo = str_replace('bar', 'foo', $foo);
file_put_contents('test.php', $foo);

---
test.php:
<?php

echo 'Title: foo', "\n";

return new class {
    private $name = 'foo';
    public function getName() { return $this->name; }
};

Expected result:
----------------
Title: foo
Get Name: foo
Title: bar
Get Name: bar

Actual result:
--------------
Title: foo
Get Name: foo
Title: bar
Get Name: foo

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2020-07-15 22:03 UTC] requinix@php.net
-Status: Open +Status: Feedback -Package: *Compile Issues +Package: *General Issues
 [2020-07-15 22:03 UTC] requinix@php.net 
This makes sense to me. It's a class definition, even if it doesn't have a name. And just like how anonymous functions don't get redefined, anonymous classes shouldn't either.

In other words, what do you believe should happen? Should test.php not be allowed to execute a second time? What is the bad code that PHP should warn about? And what is the correct alternative for the developer?
 [2020-07-17 10:45 UTC] d28b312d at opayq dot com 
> This makes sense to me. It's a class definition, even if it doesn't have a name. And just like how anonymous functions don't get redefined, anonymous classes shouldn't either.

When you re-include a file with a concrete class definition it will emit a warning saying the class can't be redefined.
With this you get no output at all, to me that's really bad for the developer who may think their function or class has been reloaded.
I'm not quite sure why they don't get reloaded as they're anonymous and so shouldn't get cached surely?
If you want a concrete class then make them one?

> In other words, what do you believe should happen? Should test.php not be allowed to execute a second time? What is the bad code that PHP should warn about? And what is the correct alternative for the developer?

I think that it should re-evaluate and reload the class and/or functions if they're anonymous.
But failing that at least emit a warning to developers of a re-definition that isn't going to happen.
Doing something silently makes for awful debugging.

The file clearly has different contents to before (as can be seen by the echo) but the class/functions don't get reloaded with the changes, which would be very unexpected to most developers I'd think.

If you want to cache stuff, then you might as well cache the whole file and not change the echo statements either, in which case what's the point of `require` vs `require_once` if sometimes it'll behave like a `require_once` for some statements and not others.
 [2020-07-17 10:49 UTC] nikic@php.net 
Pretty sure this has already been fixed (this falls under the "RTD key collision" problem internally). Are you sure that you're on the latest PHP 7.4 release?
 [2020-07-17 12:22 UTC] nikic@php.net 
Just tried your test case on current 7.4 HEAD and I get as expected:

Title: foo
Get Name: foo
Title: bar
Get Name: bar

So I think this is just a matter of an outdated PHP version.
 [2020-07-17 12:26 UTC] d28b312d at opayq dot com 
> Pretty sure this has already been fixed (this falls under the "RTD key collision" problem internally). Are you sure that you're on the latest PHP 7.4 release?

Yeah Cygwin's bundled PHP is 7.3, and I saw your fix for the RTD thing in the notes for PHP 7.4.7 but assumed as I don't have opcache on for the cli that that wouldn't fix it either way.

I've just compiled 7.4 locally in Cygwin and get the expected result too. Thanks Nikita.

/c/src/php-7-4 ((php-7.4.8)) [0] $ ./sapi/cli/php.exe /c/src/slack-test/index.php
Title: foo
Get Name: foo
Title: bar
Get Name: bar

I might continue to use this compiled 7.4 version as I have a bug with the bundled Cygwin version anyway whereby it has a hard 40MB memory limit regardless of any setting.
 [2020-07-17 12:56 UTC] nikic@php.net
-Status: Feedback +Status: Closed -Assigned To: +Assigned To: nikic
 [2020-07-17 12:56 UTC] nikic@php.net 
Thanks for checking. In that case I'm closing this issue. For reference, this was fixed as part of bug #78903 in PHP 7.4.2. In that particular case a complex sequence of events made this manifest in a crash rather than missing recompilation of code.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Thu Apr 25 09:01:29 2024 UTC