9

We have very strange errors occasionally popping up in our php logs: Trying to get property of non-object.

This exact error seems to be caused by the access to the member $shortName in the following if statement:

class MyLocaleWrapper extends SomeOtherClass {
    …
    protected static $system = NULL;
    public static function getSystemLocale() {
        if (self::$system === NULL) {
            self::$system = new self();
            debug(self::$system);
            self::$system->rfcName = SYSTEM_LOCALE_RFCNAME;
            self::$system->shortName = strtolower(Locale::getRegion(self::$system->rfcName));
            if (self::$system->shortName == '') {
                self::$system->shortName = strtolower(self::$system->rfcName);
            }
            …

# in another file:
class SomeOtherClass {
    …
    public function __construct() {
        # Some documentation about features that have been
        # removed from the constructor, but no real code in here.
        return NULL;
    }
    …

# in yet another file:
MyLocaleWrapper::getSystemLocale();

if I dump self::$system into a log file, I see that it is NULL - right after being constructed with the keyword new.

The most interesting part is that this file is included in each and every request to our page, so it gets executed ~ 10 times per second. But occasionally it just fails without anyone touching the code (or even the server).

Has anyone else ever experienced such behavior in PHP?

tereško
  • 58,060
  • 25
  • 98
  • 150
soulmerge
  • 73,842
  • 19
  • 118
  • 155
  • 2
    is there anything inside the code that can reset the static instance? – Gordon Oct 21 '10 at 16:47
  • Hm...why are you creating a new instance of the class from within the class? I'm probably wrong, but wouldn't it make more sense to create a single instance when you initially call it, and refer to it from within with the `$this` keyword? – treeface Oct 21 '10 at 16:47
  • @Gordon: The code I pasted is complete, I haven't left anything out - but I have hard times believing this myself. – soulmerge Oct 22 '10 at 07:43
  • @treeface This is not a singleton, but rather a built-in factory. There can be multiple locales in the application - but one of them is the system locale. – soulmerge Oct 22 '10 at 07:45
  • @soulmerge I guess you will have to use XDebug to get more information about what happens at runtime. – Gordon Oct 22 '10 at 07:55
  • @Gordon: I can't think of a feature in xdebug that could help me, so it seems, I don't know enough about it. How could I make use of that great tool? – soulmerge Oct 22 '10 at 08:03
  • @soulmerge just browse their [API](http://www.xdebug.org/docs/) to see if you can add some function calls that would give you more information about the system state when the error occurs. Maybe that sheds some light as to what went wrong, e.g. try to get the backtrace, used memory and stuff like that. It's a straw but it's better than nothing. – Gordon Oct 22 '10 at 08:18
  • Do you mean the error occurs when you try to access the shortName member, or after? It seems very odd that when you access the member "rfcName" everything is OK but on the next line the object is suddenly null. On which line # is the error ocurring? – Sean Oct 22 '10 at 11:51
  • I can't see how the error described could be occurring. Could you post a test case with a stack trace? Do any classes inherit from this class? Is there a constructor method that has not been included in the code sample? What version PHP? – Hamish Oct 31 '10 at 23:32
  • 1. please include in your example above, the exact line that you used to log the value of `self::$system`. 2. If the class has a constructor, please add it to the example above. 3. If the class extends another class, please let us know about it. 4. Please show an example of how you're calling this code. – Lee Nov 29 '10 at 01:59
  • @Lee: added requested information – soulmerge Nov 29 '10 at 19:05
  • @soulmerge: hmmm... **1** I don't see the line that is logging the value of `self::$system` . **2.** you've added a static constructor... but constructors can't be static - this will produce a php fatal error. **3** why are you returning NULL from your constructor? **4** What version of PHP are you using? (some versions of php4 allowed constructors to null the resulting object they were constructing, either by returning null or setting `$this=null` internally). – Lee Nov 29 '10 at 19:26
  • @soulmerge: also, what's up with `Locale::getRegion($rfcName)`? Shouldn't that be `Locale::getRegion(self::$system->rfcName)`? The same issue exists again two lines further down, where `$rfcName` is used again without using `self::$system->`. (unless there's a definition of `$rfcName`, somewhere else in this function that's not being shown here). – Lee Nov 29 '10 at 19:37
  • @Lee: Fixed the compilation errors. I can assure you, though, that these are not the problem. What I have provided here are the relevant lines in the source code (or at least what I thought was relevant, with some bugs in the adjustment), I am unable to publish all of it. The `NULL` is returned by code convention (I know it's utterly useless). PHP version is 5.3.3 – soulmerge Nov 30 '10 at 08:18
  • @Lee: btw: thanks for your efforts :-) – soulmerge Nov 30 '10 at 14:00
  • @soulmerge: no problem. Thanks for all the updates. I've posted an "answer" below (though I fear that it won't be much help). Good luck. Sorry I don't have more to offer. – Lee Nov 30 '10 at 17:14

6 Answers6

1

Thanks for all the updates. (see extensive comments above on original post).

Unfortunately, I'm as stumped as you are -- everything looks fine with that code.

Either

  1. you've found some rather obscure bug in php, or...
  2. the symptoms are tricking you into thinking the problem is in one place, when it's actually somewhere else, or...
  3. we're all missing something in that code that should be obvious to a bunch of seasoned php developers. ;-)

If it were my production system to deal with, I'd probably comment out the return NULL in the constructor, and let it run in production for a while. That return shouldn't cause any problems, but it's the only strange thing I can see here.

Sorry I can't help more than that. Please come back and let us know if you figure it out.

Lee
  • 13,462
  • 1
  • 32
  • 45
1

We finally found out we were running into php bug #50027. After setting the php.ini-variable zend.enable_gc to false, the error vanished.

soulmerge
  • 73,842
  • 19
  • 118
  • 155
0

Try the singleton approach:

class MyLocaleWrapper {
    …
    private static $system = NULL;//set it to private, and instead of accessing it as $this->system, access it with self::getInstance() or parent::getInstance() if your trying to get the parent instance
    public static function getInstance() {
        if (!(self::$system instanceof self)){
            self::$system = new self();
        }
        return self::$system;
    }
    final private function __construct() { }// Do not allow an explicit call of the constructor: $v = new Singleton();
    final private function __clone() { }// Do not allow the clone operation: $x = clone $v;
    public static function getSystemLocale() {
        $self = self::getInstance();
        log($self);//dump the value into a file with you function
…

and see what the value of $self is

Your problem seems to spawn from something overwriting or just because your not setting the $system variable

with the singleton approach you ensure that the instance has only been set once and that nothing is overwriting it.

p.s. what does the file actually do?

Timo Huovinen
  • 53,325
  • 33
  • 152
  • 143
  • I can hardly change the production server like that. But I have print_r()ed the value into a file and `$this` was `NULL`. – soulmerge Oct 22 '10 at 07:47
  • usually you have a development server and a production server, but I have experienced working with production code directly as well, so ill edit my answer. – Timo Huovinen Oct 22 '10 at 10:56
  • @soulmerge: you said you have "print_r()ed the value" of `$this`. There are 2 problems with that statement: 1. you're in a static function, so `$this` will never exist; 2. `print_r(NULL)` prints nothing -- not "NULL". `var_dump(NULL)` would print "NULL". I assume you meant something like: "I have var_dump()ed the value of `self::$system`? – Lee Nov 29 '10 at 01:49
  • @Lee: 1.) Sorry, I meant that `$system` was `NULL` 2.) I was using a custom debugging function that verified that the value was `NULL` (I'm using the function for several years now, I highly doubt it has a bug like that) – soulmerge Nov 29 '10 at 19:00
0

What is this line: self::$system->shortName = strtolower(Locale::getRegion($rfcName));? Where does the $rfcName come from? If it's not defined before you trying to use it, that would cause an error, causing the rest of the code to fail, giving you the problem you describing.

Since I don't see your entire class, I don't have all the information to answer the question, so this is just a guess

Dmitri
  • 34,780
  • 9
  • 39
  • 55
0

This will print the object once

class MyLocaleWrapper extends SomeOtherClass {

    protected static $system = NULL;
    public static function getSystemLocale() {
        if (self::$system === NULL) {
            self::$system = new MyLocaleWrapper();

            self::$system->rfcName = 'test';
            self::$system->shortName = strtolower('test');
            if (self::$system->shortName == '') {
                self::$system->shortName = strtolower('test');
            }
                print_r(self::$system);

        }
        return self::$system;
    }
}

# in another file:
class SomeOtherClass {

    public function __construct() {
        # Some documentation about features that have been
        # removed from the constructor, but no real code in here.
        return NULL;
    }
}

# in yet another file:
$l = MyLocaleWrapper::getSystemLocale();
$l = MyLocaleWrapper::getSystemLocale();
Manu
  • 4,101
  • 1
  • 17
  • 23
-1

Not sure if you figured this out but I would try assigning the new object directly to a variable, then assign it to self::$system later on. Something like the code below may help.

$newSystem = new self();
$newSystem->rfcName = SYSTEM_LOCALE_RFCNAME;
$newSystem->shortName = strtolower(Locale::getRegion($rfcName));
....
self::$system = $newSystem
BoltClock
  • 700,868
  • 160
  • 1,392
  • 1,356
oreX
  • 82
  • 3