PHP Notices and probably more [#1750476]

Using dev version I received tons of PHP notices like this:
Notice: Trying to get property of non-object in MemCacheDrupal->getMultiple() (line 30 of /sites/all/modules/memcache/memcache.inc).
Notice: Trying to get property of non-object in MemCacheDrupal->valid() (line 65 of /sites/all/modules/memcache/memcache.inc).

Sometimes there is no $result object, so I added a check before the result object is used.

Patch follows.

Comment	File	Size	Author
#6	memcache-notices-1750476-6.patch	952 bytes	markpavlitski
#3	notices-1750476-3.patch	981 bytes	mgifford
#1	notices-1750476-2.patch	583 bytes	valderama

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

valderama CreditAttribution: valderama commented 24 August 2012 at 10:30

Status:

Active

» Needs review

File	Size
notices-1750476-2.patch	583 bytes

Here is the patch. Please review.

Thanks,
walter

Comment #2

helmo CreditAttribution: helmo commented 26 October 2012 at 11:44

I saw the same notices while updating a memcache installation...
Your patch seems sound, although I haven't been able to reproduce the problem.

Comment #3

mgifford

he/him

English

CreditAttribution: mgifford commented 2 November 2012 at 18:06

File	Size
notices-1750476-3.patch	981 bytes

We got these with 7.x-1.0. I don't think it would have been fixed in dev, as I found that the checks weren't being done in git.

We got these errors (when clearing cache). Could be that it should be set up with something else. However, just having a check for null values is a good practice.

The patch above only addressed the issue at line 30 not at line 65, so I updated this patch to take them both out.

Comment #4

nerdcore CreditAttribution: nerdcore commented 2 November 2012 at 19:05

Proper variable checking is always good practice, although I am concerned about why these notices cropped up after running memcache for a week (or more) without any notices. A major change I made to the site where I started getting these was enabling APC as an additional cache mechanism.

Should I be concerned about this, or were there always just some bad variable tests in place and this patch fixes the problem once and for all?

Comment #5

mgifford

he/him

English

CreditAttribution: mgifford commented 2 November 2012 at 19:29

Comment #6

markpavlitski CreditAttribution: markpavlitski commented 14 March 2013 at 14:09

File	Size
memcache-notices-1750476-6.patch	952 bytes

The patch in #3 does not account for when $cache is a valid object but $cache->created does not exist.

The attached patch takes a slightly different approach.

Comment #7

sd46 CreditAttribution: sd46 commented 17 April 2013 at 11:06

Status:

Needs review

» Reviewed & tested by the community

Tested patch in comment #6, the patch prevents both notices from being generated.

I don't know how the notices were originally caused, but for testing purposes an easy way to recreate them is to change between
$conf['memcache_extension'] = 'Memcached';
and
$conf['memcache_extension'] = 'Memcache';
in the settings.php file, which will cause both notices to be generated on the next page load.