php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #64346 Function name resolution and eval
Submitted: 2013-03-04 10:11 UTC Modified: 2017-10-24 08:03 UTC
Votes:38
Avg. Score:4.4 ± 1.0
Reproduced:27 of 31 (87.1%)
Same Version:11 (40.7%)
Same OS:12 (44.4%)
From: gen dot work at gmail dot com Assigned:
Status: Open Package: *General Issues
PHP Version: 5.4.12 OS: Ubuntu 12.10
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: gen dot work at gmail dot com
New email:
PHP Version: OS:

 

 [2013-03-04 10:11 UTC] gen dot work at gmail dot com
Description:
------------
The code below works as expected on version 5.3.

Test script:
---------------
namespace Foo {
    function bar() {
        var_dump(is_callable('Foo\\time'));
        var_dump(time());
    }
}

namespace {
    \Foo\bar();
    eval('namespace Foo; function time() { return -1; }');
    \Foo\bar();
}

Expected result:
----------------
bool(false)
int(1362390237)
bool(true)
int(-1)


Actual result:
--------------
bool(false)
int(1362390237)
bool(true)
int(1362390237)

Patches

bug64346-2.diff (last revision 2013-04-12 06:50 UTC by laruence@php.net)
bug54346.diff (last revision 2013-03-04 13:25 UTC by dmitry at zend dot com)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2013-03-04 10:38 UTC] laruence@php.net
-Status: Open +Status: Verified -Assigned To: +Assigned To: dmitry
 [2013-03-04 10:38 UTC] laruence@php.net
confirm this change. this is due to the 5.4 performance improvement: "literal 
cache"

@dmitry, maybe we should not cache the function entry for INIT_NS_FCALL?

thanks
 [2013-03-04 13:43 UTC] laruence@php.net
or maybe we should document this behavior, since disable it will bring performance 
issue
 [2013-03-04 13:50 UTC] laruence@php.net
-Status: Verified +Status: Feedback
 [2013-03-04 13:50 UTC] laruence@php.net
@gen the main brief is, when you first call to \Foo\Bar,  the 'time' constant in 
the \Foo\Bar function, will bundle to "time function", in the first time , it 
obviously will be bundled to \time.

then when you sencond call to it. PHP will use that cache instead of look up in 
function table again for "time" function, to increase performance..

so, if we disable the cache, then performance slowdown...

what do you think?  a workaround is define a Foo\Bar2, after you eval, you call 
to it, then it will bundled to \Foo\Time..
 [2013-03-04 14:50 UTC] gen dot work at gmail dot com
The main issue I see is that is_callabe() is lying. It says that '\Foo\bar' is callable, but in fact it's not. So just document this behavior is not enough imo, is_callabe should be tweaked to reflect actual status.

And I don't quite understand suggested workaraund. Could you please give a simple example? In my usecase I try to mock time function to avoid sleep() calls:
https://github.com/rybakit/phive-queue/blob/master/tests/Phive/Tests/Queue/AbstractQueueTest.php#L59
https://github.com/rybakit/phive-queue/blob/master/src/Phive/Queue/InMemoryQueue.php#L40
 [2013-03-04 14:50 UTC] gen dot work at gmail dot com
-Status: Feedback +Status: Assigned
 [2013-03-04 14:53 UTC] gen dot work at gmail dot com
'\Foo\bar' -> '\Foo\time' in my prev comment
 [2013-03-04 15:59 UTC] dmitry@php.net
I suppose the bug has to be fixed.

The problem that the fix will slowdown each call to unqualified function from a namespace :(

I'm not sure if we like to do it...
 [2013-04-12 06:50 UTC] laruence@php.net
The following patch has been added/updated:

Patch Name: bug64346-2.diff
Revision:   1365749451
URL:        https://bugs.php.net/patch-display.php?bug=64346&patch=bug64346-2.diff&revision=1365749451
 [2013-12-02 16:57 UTC] Terry at ellisons dot org dot uk
Dmitry,

I came across this one myself (see http://3v4l.org/KSIDh) and found this bugrep when I came to report it.

I think that one-time caching as we've started to do with function look-up is an important performance feature, and something that we should look to extend -- though this is a wider discussion than this bugrep, probably a DL list discussion.

I haven't gone through the current implementation yet, but my immediate thought is that this really just a cache-coherence issue in a cache where the invalidation rates are very low, so even if cache flushing was a relatively expensive operation, then the expected (that is average) overhead would still be small.  I want to think about this some more and look at the performance issues around some approaches, before posting further -- Terry
 [2013-12-08 16:16 UTC] Terry at ellisons dot org dot uk
BTW, the title of this bug is misleading.  It can still occur for *any* INCLUDE_OR_EVAL of a namespaced override of a builtin function, eg.

namespace fred;
some_function_which_executes_builtin();
include "some_include_in_namespace_fred_which overrides_same_builtin();
some_function_which_executes_builtin();
 [2013-12-08 16:18 UTC] Terry at ellisons dot org dot uk
Damn, why can't you edit your own comments :-)  The include should read:

include "some_include_in_namespace_fred_which overrides_same_builtin.php";

of course.
 [2016-11-12 11:48 UTC] showerheadsuk at hotmail dot com
The same problem also applies to constants when accessing from a namespaced class - if the first the time the constant is accessed it is defined globally but not in the local namespace, then that global value will be used for all future accesses even if it is later defined in the local namespace.

Since overriding global functions / constants is mostly used just for testing, could this be added as a php.ini option, like 
literal_cache_disable = 1
 [2017-10-24 08:03 UTC] kalle@php.net
-Status: Assigned +Status: Open -Assigned To: dmitry +Assigned To:
 [2017-11-21 18:48 UTC] shiranai7 at hotmail dot com
Would it also be expensive to invalidate specific (not all) keys in the "literal cache" each time a function or a constant is defined?

I'd avoid a global INI setting if possible.
 [2018-08-24 12:27 UTC] syl dot fabre at gmail dot com
Still present in 7.3.0-beta2: https://3v4l.org/IMmoO
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Tue Mar 19 02:01:28 2024 UTC