Make WordPress Core

Opened 12 years ago

Closed 12 years ago

Last modified 21 months ago

#11780 closed defect (bug) (invalid)

Memory Leak in plugins_api

Reported by: hakre Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: General Keywords: needs-patch
Focuses: Cc:


plugins_api() in plugin-install.php does not return by reference nor does it calls filters sothat they pass / return by reference.

This violates against the PHP 4 compability guidelines in WP core. The effect ist, that clones of the objects are created which will leak memory.

Related filters: 'plugins_api_args', 'plugins_api', 'plugins_api_result'.

(stumbeled over this while reviewing #10319)

Change History (10)

#1 @hakre
12 years ago

Referenced the wrong ticket, the correct one is #11640.

#2 @dd32
12 years ago

Are the local variables not destroyed upon function termination anyway? Which would free up the memory?

from memory, returning references can be pretty tricky between PHP4 and 5 compatibility?

#3 @hakre
12 years ago

It's common in WP, isn't?

function &get_role( $role )
function &get_manifest()
function &get_categories( $args = '' ) 
function &get_category( $category, $output = OBJECT, $filter = 'raw' ) 
function &get_tags( $args = '' ) 
function &get_tag( $tag, $output = OBJECT, $filter = 'raw' )
function &get_comment(&$comment, $output = OBJECT)
function &separate_comments(&$comments)
function &get_translations_for_domain( $domain )
function &get_children($args = '', $output = OBJECT)
function &get_post(&$post, $output = OBJECT, $filter = 'raw') 
function &get_page(&$page, $output = OBJECT, $filter = 'raw')

this list could be extended, to only name some. Please compare to those functions, I think they do not do that just for fun.

From the tests/docs I know of there are no negative impacts in PHP5 using PHP4-reference-styled code. PHP5 does not have any incompability changes on that one, it has others. It was in the beginning of PHP5 (consumed more memory) but from what I know that does not count any longer, we had run some tests some time ago (2 years or so). If I should be false, in other tickets from decisions have been made (stating this for correctness sake).

I asked for clarification by core developers in this ticket: #11663

#4 @miqrogroove
12 years ago

  • Milestone 3.0 deleted
  • Resolution set to invalid
  • Status changed from new to closed

In PHP there is no such thing as "return by reference" by the normal definition. PHP always uses references for return variables. The only exception is when there's a scope conflict, as in, return $GLOBALS['abc']; or return $this->var; when PHP implicitly returns by value.

Most of the examples cited above are probably returning cached objects, which would be maintained as separate copies unless explicitly referenced when returned.

plugins_api() returns a local variable and needs no special treatment.


#5 @hakre
12 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

Please define "normal definition". I'm using PHP documentation/manual speech here:

which actually is in there. No Idea what you want to blunt here miqrogroove, sorry. I think it's better to clarify stuff first. And if it's not possible to clarify, things are still unclarified even if you run down the hill and just press the close button.

Looks like you do this by intention. May I ask why?

#6 @dd32
12 years ago

hakre: That link also specifically mentions this:

Do not use return-by-reference to increase performance. The engine will automatically optimize this on its own. Only return references when you have a valid technical reason to do so.

What miqrogroove is saying, is that PHP doesnt "return by reference" in the C++ sense of the word.

http://au.php.net/features.gc.refcounting-basics is a good reference to how PHP stores variables, and why returning by reference is not needed to reduce memory footprint. It'll also explain how Objects can cause memory leaks in the Garbage collection unit (Note to self: Glass can break through thin bags).

PHP5 takes it a step furthur once again, Objects are not stored in the same bucket as variables, in addition to being passed as reference in all case and as such, returning them as reference under php5 is even more useless.

So, Returning a simple variable from the function will not cause memory leaks, nor can this create a memory leak due to serialization not handling circuar references (IIRC).

#7 @miqrogroove
12 years ago

Some empirical testing would satisfy a few points of curiosity about references that were raised in IRC.

Hyp 1: Passing a large string variable into a user-defined function will cause peak memory consumption to double, depending on argument referencing.

Hyp 2: String variables are implicitly referenced in functions that do not modify the variable, so argument referencing does nothing.

Hyp 3: Argument referencing is detrimental to performance.

#8 @nacin
12 years ago

  • Resolution set to invalid
  • Status changed from reopened to closed

#9 @miqrogroove
10 years ago

Follow up on this conversation.. would be happy to see your comments there. Enjoy!


This ticket was mentioned in Slack in #core-js by netweb. View the logs.

21 months ago

Note: See TracTickets for help on using tickets.