PHP 4.4 Minor Gotcha

Tweet

In the days after the release of PHP 4.4, it was interesting to watch the discussions surrounding a possible backwards compatibility issue which had been introduced with the release. Commentors seemed to be divided on whether this did, or did not, actually constitute a break in backwards compatibility.

The ‘backwards-compatibility break’ is in fact a new notice generated by the PHP error-handling code, warning developers when trying to return a temporary variable by reference. The notice seems to have been added in response to a bug in PHP 4.3 in reference handling.

Today, when I upgraded my own development server at home to PHP 4.4, I was confronted by the problem. An application I’ve been working on runs in its own ‘debug’ mode, which makes notices really visible by echoing the notice to output.

The culprit:

Notice in D:htdocsaaaresourcesincludescontrols.inc.php, line 14: Only variable references should be returned by reference

The line in question falls in the following method:


function &getnodecontrol($objecttype)
{
require_once(RESOURCE_DIR . "nodecontrols/$objecttype.inc.php");
return new $objecttype($this->db);
}

The problem here is that “new $objecttype($this->db)” is not a variable – it is the result of a “new” statement – effectively a ‘temporary’ variable – which PHP can’t create a reference to. The only thing that can be returned, when the variable returns by reference, is a variable which has been initiated.

As the PHP manual puts it under returning references:

Note: If you try to return a reference from a function with the syntax: return ($found_var); this will not work as you now try to return the result of an expression, and not a variable, by reference. You can only return variables by reference from a function – nothing else.

The problem is solved, and the notice is surpressed, with the following code:


function &getnodecontrol($objecttype)
{
require_once(RESOURCE_DIR . "nodecontrols/$objecttype.inc.php");
$control = new $objecttype($this->db);
return $control;
}

Here, the result of “new $objecttype($this->db)” is stored into a variable, and a reference to that variable is returned.

This article from eZ Systems describes the change in a bit more detail.

The change in behaviour was criticised by some whose applications were affected by this change. One example is this message about a problem with Horde, the web email client.

In response, others have pointed out that notices should not be enabled in a production environment anyway, and therefore the change shouldn’t have caused any problem.

The Zend weekly summary (#245) provided an amusingly concise summary of the Horde issue.

Interestingly that discussion links to a newer bug in PHP 4.4 which involves passing by reference. It hints that a future update to PHP 4.4 might be needed in order to fix it, and that update might introduce a new warning and/or break backward compatibility.

Free JavaScript: Novice to Ninja Sample

Get a free 32-page chapter of JavaScript: Novice to Ninja and receive updates on exclusive offers from SitePoint.

  • Nico Edtinger

    I guess it should also be possible to write something like (sorry, no PHP 4.4 to test)

    function &getFoobar() {
    return $o = new Foobar();
    }

    making it 3-5 chars longer. Of course the problems is solved in PHP5, at least for objects.

    b4n

  • http://www.lopsica.com BerislavLopac

    I have a feeling — and it’s getting stronger with each new issue like this — is that PHP is starting to feel the strain of its own popularity and growth. Those are all symptomps of its growing pains, and as those keep piling up PHP will start to lose some of its followers (or it has already started, if we take a look at some recent threads on the “PHP Appliaction Design” here on Sitepoint.

    I believe that they should have break the backward-compatibility gospel with version 5 (which they did in a way with that php.ini setting) and go more completely in the OOP direction.

  • http://www.usebb.net/ PC_Freak

    I had this issue in my forum package about a week ago. I have an object for the DB class and one for functions. Inside a method of the function class, where I access a method of the DB class by making ‘$db’ global, the following failed:

    $query = end($db->get_used_queries());

    The method returns an array so it should work, however 4.4 gave an error. The following worked:

    $queries = $db->get_used_queries();
    $query = end($queries);

  • http://aplosmedia.com/ Eric.Coleman

    BC break is here to stay for now…

    Hopefully… when it’s time for 6… they just say screw it and break it all…

    - Eric

  • http://www.lastcraft.com/ lastcraft

    Hi.

    The issue is not so small. I had to add 50+ lines of useless code to SimpleTest to work around it I estimate that a 40K line commercial application that I contribute to would need several thousand. That effectively blocks us from “upgrading” to 4.4. Especially as their is a compound effect with some Zend acclerator bugs that caused to put all of theose &’s in the code in the first place.

    It affects the OO coders with their “&” returns worst of all. I don’t see why thye could not have fixed the fix. Or for that matter why they have taken so long to fix fundamental issues in the langauge.

    yours, Marcus

  • Etnu

    People complaining about the notice are kind of amusing to me. The entire reason for it is that it can cause memory leaks. It’s not stopping your application from working, it’s just a gentle nudge that says “fix it now”. It SHOULD be a fatal error, if you ask me, but I’m anal about that sort of thing.

  • Derick

    @Etnu: I agree with you, but that would simply piss off too many people ;-)

    @Thomas, the correct way to fix this function is NOT TO RETURN by reference at all! There is simply no reason for doing this.

  • http://www.divbyzero.org Div By Zero

    That’s exactly why big enterprise is staying away from PHP. There’s no real planning for future release and they’re scared to invest big money on something that won’t work in one or two year.

  • Harry Fuecks

    Somehow find myself thinking of old Hammer Horror films where the serfs, tired of giving up their daughters, rise up to oust evil Count, in the form of Christopher Lee.

    The point on the release notice has been made. Given the PHP 4.2.0 experience with register globals, find it a little surprising to see lightning strike twice.

    others have pointed out that notices should not be enabled in a production environment anyway

    Perhaps I’ve missed something but don’t see where that’s documented. Good practice for production environments might be disabling display_errors but I see no valid reason why E_NOTICE should be disabled. Otherwise think E_NOTICE parallels the Perl pragmas “use strict” and “use warnings” which good practice suggests should always be on. Having E_NOTICE on helps you catch an issue which only show up once you have real data pumping round your code – think about SimpleXML or PEAR::XML_Serializer and you have examples. Otherwise, I’m highly suspicious of anyone who claims to to know what someone else’s good practice is.

    The entire reason for it is that it can cause memory leaks.

    Perhaps my perspective is flawed but had viewed PHP as a platform where memory is managed and where I, as a user, am protected from being able to assign memory in a manner which can’t be recovered. I can accept circular references as a potential issue due to the garbage collection implementation but that’s not what we’re talking about here.

    To the end user, this is saying that a difference in syntax between two pieces of code, which you would probably expect to have the same semantic meaning if you’ve used pretty much any other languages with support for the notion of objects, is distinguishable by whether it “leaks” memory or not. And the impact on people’s code is as silly as this diff – it’s a step away from having less code.

    In a perfect world, perhaps this could have been solved at parse time, by recognizing the semantic equivalence. Reality dictates otherwise, which is fair enough, but then it’s a question of how to handle it so the serfs aren’t going to rise up.

    People complaining about the notice are kind of amusing to me.

    Christopher Lee would also be amused.

  • dedlfix

    @Nico Edtinger:
    function &getFoobar() { return $o =& new Foobar(); }

    Don’t forget to make a reference (watch the second &) otherwise your return by reference doesn’t make much sense.

  • Carl

    This is all going to come down to a management decision. The older php4 bugs are being solved with php5 code whenever possible. Probably because extra effort of maintaining a “with” and “without” version of php is proving to be a strain. One version or the other is being broken by a common fix. This makes for having to diecide which is better and which will take the lead. In my opinion php5 should be the flagship version now not later.

    Someone is going to have to breakdown and say php4 has reached the end of its cycle and place it in with php/fi and php3. The reason php5 came into being was because of the php4 limitations. Kill php4 now before it infects php5 with bad bug fixes and decisions.

  • mpigulla

    function &getFoobar() { return $o =& new Foobar(); }

    This 4.4 “fix” is really silly. I agree with Harry: I don’t want to care about memory management when coding in PHP. The platform claims to take care about that. When $b =& new Foobar(); is valid and I can assign the “return” value of a new as a reference, why shouldn’t it be possible to just return the same thing (an object instance) as a reference from a function?

    Of course we want to have memory leaks fixed in the platform – but not in a way that forces us to touch (in my case: a lot of) “legacy code”. Especially if it requires such silly “workarounds”. To me it seems that this workaround does – seen at the PHP language level – not make any sense. It expresses nothing. It forces me to write unneccessary constructs just to somewhere allocate some temporary
    memory under the hood so that the interpreter can pass a reference around. IMHO the language should do that for me.

    Please note I only refer to the case of “return new …”; something like

    function foo() { return "blah"; }
    function bar(&$arg) { $arg = 1; }
    bar(foo());
    is really pathologic.

    Now Derick says:

    The correct way to fix this function is NOT TO RETURN by reference at all! There is simply no reason for doing this.

    PHP5 finally changed the way objects are passed around – with their “handles” (avoiding the word “reference” here). But the point is that the broken PHP4 way of treating objects forces you to do exactly that!

    Just consider the commonly used factory pattern, which basically means you have exactly the above function returning a new something();

    This function has to return a reference (and you have to assign by reference at the caller’s level). Otherwise, you will lose object identity. Consider the following code, where A might be part of a central publish/subscribe pattern where objects register themselves when being constructed.


    m =& $whom;
    }
    }

    class B {
    var $foo = 0;
    function B(&$registerAt) {
    // $registerAt->register($this);
    }
    }

    class Factory {
    function &create(&$registerAt) {
    return new B($registerAt);
    }
    }

    $a = new A();
    $x =& Factory::create($a);

    /* $a->m and $x need to refer to the *same* object */
    $x->foo = 1;
    print $a->m->foo; /* should print "1" if referencing the *same* object (and works only if B registers itself, of course) */
    ?>

    Now that gives the notice in question and the problem is on the “return new B(…)” line.

    Things come even worse: Uncomment the register… line in B’s constructor. At that moment it’s all fine, that is, code in Factory::create() will produce a warning or not depending on the implementation of B::B()! X-(

    Again, the PHP4 way of treating objects requires you to pass around references almost everywhere when working with objects.

    If you’re writing OO code and postponed the migration to PHP5 due to the changes in OOP language features – do it now. It should be pretty the same to walk through all your “legacy” code and to add workarounds for the 4.4 bug, or just remove all the reference stuff altogether because you won’t need it in PHP5.

    Sadly enough Derick remained very vague in his announcement of 4.4.0. The announcement mail did not mention the implications with a single word.

  • Lux

    If you

  • mpigulla

    First, we do agree that “return new somewhat()” in a function and returning that as a reference *was* necessary and the only correct way in PHP4 to achieve the “correct” behaviour.

    Nobody complains about the fix of the memory leak problem, and it seems as if they found a way to work around it so you can still return new… Only it prints the stupid notice in a case where it really should not (again, I just refer to that single case).

    Insisting on the point that “you shouldn’t let notices surface on a production system anyway” or “just set the error reporting level to … & ~E_NOTICE” is simply ignorant.

    Now we have a large codebase and target installation systems where it is out of our control wheter 4.3.11 will remain installed (probably nobody will do if they hear “memory leak”), when the switch to 4.4 has been or will be done and what will be with future versions. We also have no influence on the error reporting level (and as to me, I want my app to be coded so well it runs without any notice at all :).

    Furthermore, I could hope the PHP developers will fix the fix and release a 4.4.1 pretty soon – which I think is far from being realistic regarding the unconcerned comments I’ve read as to this issue. Fix the fix myself? Not possible, again as I have no control of the target systems.

    Maybe an automated search-and-replace can “fix” my code to work around the problem. Let alone it would mean to add meaningless code – I am still unsure if there aren’t any cases where you would want to “return Factory::build()” to further indirect the creation of an instance; probably that needs a fix, too, and cannot be found that easily.

    As to PHP5 – the version your customers run determines the versions you need to support. Either abandon PHP altogether and rewrite in another language. Or go for PHP5 – but you should not hope that you can live with PHP4 forever. Better act now than being urged in a few years. (Too bad that 4.4 is urgent *now* and came so unannounced that it is a real problem to get your priorities right.)

    Indeed the migration is a major problem if you wrote OOP code (like we did) right from the start, and the difference between passing references or working with copies requires developers with a sound understanding of the issue. The errors arising from that kind of mistakes are so subtle that often things break not until a lot of lines of code later or you hardly notice that things break at all.

    Face it. :)

  • Lux

    Actually Mpigulla, I do hope to live with PHP4 for as long as possible, and so do many people. This follows the sound advise found in the following article on joelonsoftware.com:

    http://www.joelonsoftware.com/articles/APIWar.html

    “And personally I still haven’t had time to learn .NET very deeply, and we haven’t ported Fog Creek’s two applications from classic ASP and Visual Basic 6.0 to .NET because there’s no return on investment for us. None. It’s just Fire and Motion as far as I’m concerned: Microsoft would love for me to stop adding new features to our bug tracking software and content management software and instead waste a few months porting it to another programming environment, something which will not benefit a single customer and therefore will not gain us one additional sale, and therefore which is a complete waste of several months, which is great for Microsoft, because they have content management software and bug tracking software, too, so they’d like nothing better than for me to waste time spinning cycles catching up with the flavor du jour, and then waste another year or two doing an Avalon version, too, while they add features to their own competitive software. Riiiight.

    Same issue at hand here. As you said yourself, upgrading to PHP5 is so troublesome because “errors arising from that kind of mistakes are so subtle that often things break not until a lot of lines of code later or you hardly notice that things break at all”. If I move to PHP5, I simply cannot introduce that level of instability on my users. That would be a support nightmare.

    PHP4 has enough OOP features for most uses, the rest is just icing (and Java envy). PHP4 is also well known and well tested. Why introduce an unnecessary unknown into the equation?