Opened 13 months ago

Closed 13 months ago

Last modified 10 months ago

#314 closed defect (wontfix)

Fatal error class redeclaration after return statement

Reported by: afxdesign Owned by: moo
Priority: major Milestone: 3.0.4
Component: cacher Version: 3.0.0
Keywords: Cc: anthony@…
Application: PHP Version: 5.4.4
Other Exts: SAPI: Irrelevant
Probability: Blocked By:
Blocking:

Description

Hello, I have experienced a problem where xcache loads a class incorrectly before a validating if statement which would otherwise return/exit the script. Wrapping the class in else brackets resolves this issue, however I have seen this syntax in use in a number of Wordpress plugins which might mean its worth looking into this.

The code works as expected with xcache disabled and with APC. You can view a youtube video demonstrating this at: http://www.youtube.com/watch?v=VcNLoDvRLDc&feature=youtu.be

index.php

<?php
include('debug1.php');
include('debug2.php');

$test = new SomeClass();
echo $test->someVar;

debug1 & debug2 are duplicate files:

<?php
if(class_exists('SomeClass')) return;
class SomeClass {
    var $someVar = "test";
}

Change History (7)

comment:1 Changed 13 months ago by afxdesign

  • Cc anthony@… added

comment:2 Changed 13 months ago by moo

  • Resolution set to wontfix
  • Status changed from new to closed

This bug has been reported multiple times. The code reproduced is already deprecated. It is also reproduced with PHP 5.5 + builtin.

I haven't tried it with APC, but i guess you may get strange behavior if debug1.php is include before debug2.php while in another file in reverse order

comment:3 Changed 13 months ago by moo

It is also reproduced with PHP 5.5 + built in opcache

comment:4 Changed 13 months ago by afxdesign

Hi, I did notice that you had related issues which would have had the same root cause. However, I thought it worth flagging this up as it is potentially a valid use case. The if condition is not instantiating the class before it is parsed it is simply making a check on the classes loaded into the get_declared_classes() array - a perfectly valid call.

It seems the issue is to do with what scope the class is cached in and by it not being wrapped in brackets goes out of the scope of the if statement (which is not PHP functionality) and so is then presented to PHP to parse before the if statement which suddenly flags up an incorrect warning that we are trying to load the class a second/multiple times. Despite a lot of searching I cannot see where this is deprecated in php.

Using autoloading is the best way to avoid hitting this issue, simply wrapping with else in my mind adds needless code.

Anyway, hopefully this might flag up for any more specific searches people might make to this problem in the future. There is a stackoverflow issue related to this, if it is up-voted lots of times I will update this just to let you know that it may be causing a wider aggravation. Thanks.

comment:5 Changed 12 months ago by moo

The statement "The if condition is not instantiating the class before it is parsed" is false

The whole php file is parsed/compiled into opcode.

In both debug1.php, debug2.php same thing happen but there's a condition checking
They're compiled as

<?php
if(class_exists('SomeClass')) return;
install class "SomeClass";

and because SomeClass? is top level class, it is installed immediately in the middle of compiling.
in debug1.php case, the install success. it then clear the code "install class 'SomeClass?'" so it won't install again when the code is executed. this is early binding. early binding is "before" the code get executed, before your "if class_exists".
but for debug2.php, install will fail due to "class already exists", and it will give up, so the class will only be install after this php is executed (include=execute) exactly at that line of opcode, this is called late binding.
late binding happens after your "if class_exists"

in another word, in the original and some opcode cacher behavior, class in debug2.php turns into late binding. while XCache do both file in exactly same way which is more sane to me, all become early binding, leading class redeclaration error.

Even though your code seems to work, it's not actually what it looks like to you.
The reason no error is thrown, is because it's a PHP built-in behavior. not because your "if" checking
You'll see the following code output "true" without any opcode cacher, the original PHP behavior:
devel.php

<?php

include "a.php";

var_export(class_exists("A"));

a.php

<?php

return;

class A {}

However if you put "class A" under any {} (e.g.: inside "if" statement), the definition will always be late binding

<?php

return;

{
    class A {}
}
Last edited 12 months ago by moo (previous) (diff)

comment:6 Changed 12 months ago by afxdesign

Hi moo, thanks for taking the time to explain this and giving the demo - it was very useful as I spent hours just looking into it out of interest. You are absolutely right - which you knew!

I do wonder
As you have said if the issue it because PHP is executing late binding and you have decided not to (for valid reasons) unless PHP does deprecate late binding completely (I have not found anything to say they have) there would always be an inconsistency between PHP results with and without xcode which could be problematic for a person whom installs xcode on their perfectly working site and finds that it no longer works.

Anyway, as I said previously I placed this ticket as much to raise awareness for people experiencing the problem and save them time as to have it fixed.

comment:7 Changed 10 months ago by moo

  • Milestone changed from undecided to 3.0.4
Note: See TracTickets for help on using tickets.