Testing memory usage of each function on the OpenCart extension page

Question

Somewhere in this code seems to be a memory leak. A couple of users were exceeding 512Mb and I'm not sure how.

Adapted the following SO code to try and test memory usage of each function on the (OpenCart extension) page:

public function mem_test($func_name){
 declare(ticks=1);
 $handle = fopen("/Users/mikekilmer/Sites/mzoo/dev/php/open_cart_memory.txt", "a+");
 $mem = memory_get_usage();
 $bt = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 2);
 fwrite($handle, $bt[0]["file"]."\t: ".$func_name." :".$bt[0]["line"]."\t".$mem."\n");
 $vars = get_defined_vars();
 foreach($vars as $var){
 if(is_array($var)){
 foreach($var as $v){
 fwrite($handle, implode(", ", $v)." (in array) \n");
 }
 }else{
 fwrite($handle, $var."\n");
 }
 }
}

All of which are returning less than 1MB (around 700000 - 900000). Added the get_defined_vars() to see if there were some large variables being toted around, which returns four small variables.

Here is the offending code:

 public function alert_stock($pid, $qtd) {
 $this->mem_test("alert_stock");
 $qry = $this->db->query("SELECT quantity, model FROM " . DB_PREFIX . "product WHERE product_id = '" . $pid . "'");
 if ($qry->row['quantity'] < 5) {
 $mail = new Mail($this->config->get('config_mail_protocol'), $this->config->get('config_smtp_host'), $this->config->get('config_smtp_username'), html_entity_decode($this->config->get('config_smtp_password'), ENT_QUOTES, 'UTF-8'), $this->config->get('config_smtp_port'), $this->config->get('config_smtp_timeout')); 
 $mail->setTo($this->config->get('config_email'));
 $mail->setFrom($this->config->get('config_email'));
 $mail->setSender($this->config->get('config_email'));
 $mail->setSubject('Low Stock on: ' . $qry->row['model']);
 $mail->setText('Low Stock on: ' . $qry->row['model'] . "\n" . $qry->row['quantity'] . " remaining.\nThank you for your great work and have a wonderful day.");
 $mail->send();
 }
 }
 public function sub_stock($pid, $qtd, $chave) {
 $this->mem_test("sub_stock");
 $this->db->query("UPDATE " . DB_PREFIX . "product_option_value SET quantity = (quantity - " . (int)$qtd . ") WHERE product_option_value_id = '" . (int)$chave . "' AND subtract = '1'"); 
 $this->db->query("UPDATE " . DB_PREFIX . "product SET quantity = (quantity - " . (int) $qtd . ") WHERE product_id = '" . (int) $pid . "' AND subtract = '1'");
 }
 public function upd_stock($key,$qty) {
 $this->mem_test("upd_stock");
 $products = $this->getProducts();
 $pid = $products[$key]['product_id'];
 $qtd = $products[$key]['quantity'];
 if ((int) $qty != (int) $qtd) {
 if ($products[$key]['option']) {
 $opt_id = $products[$key]['option'][0]['product_option_value_id'];
 if ($qty > $qtd) {
 $qtx = (int) $qty - (int) $qtd;
 $this->db->query("UPDATE " . DB_PREFIX . "product_option_value SET quantity = (quantity - " . (int) $qtx . ") WHERE product_option_value_id = '" . (int) $opt_id . "' AND subtract = '1'");
 } else {
 $qtx = (int) $qtd - (int) $qty;
 $this->db->query("UPDATE " . DB_PREFIX . "product_option_value SET quantity = (quantity + " . (int) $qtx . ") WHERE product_option_value_id = '" . (int) $opt_id . "' AND subtract = '1'");
 }
 }
 if ($qty > $qtd) {
 $qtx = (int) $qty - (int) $qtd;
 $this->db->query("UPDATE " . DB_PREFIX . "product SET quantity = (quantity - " . (int) $qtx . ") WHERE product_id = '" . (int) $pid . "' AND subtract = '1'");
 } else {
 $qtx = (int) $qtd - (int) $qty;
 $this->db->query("UPDATE " . DB_PREFIX . "product SET quantity = (quantity + " . (int) $qtx . ") WHERE product_id = '" . (int) $pid . "' AND subtract = '1'");
 }
 }
 }
 public function add_stock($key) {
 $this->mem_test("add_stock");
 $products = $this->getProducts();
 print_r($products);
 die();
 $pid = $products[$key]['product_id'];
 $qtd = $products[$key]['quantity'];
 if ($products[$key]['option']) {
 $opt_id = $products[$key]['option'][0]['product_option_value_id'];
 $this->db->query("UPDATE " . DB_PREFIX . "product_option_value SET quantity = (quantity + " . (int) $qtd . ") WHERE product_option_value_id = '" . (int) $opt_id . "' AND subtract = '1'");
 }
 $this->db->query("UPDATE " . DB_PREFIX . "product SET quantity = (quantity + " . (int) $qtd . ") WHERE product_id = '" . (int) $pid . "' AND subtract = '1'");
 }

Does anyone see anything in the code that might be causing a memory leak, or can offer suggestions for further testing/investigating?

Answer 1

PHP is a way to high-level programming language to fully, and comprehensively manage the memory. Memory leaks are really relatively common. They are, however, not as problematic as you seem to think.

PHP is, essentially, stateless. After the script has been executed, all resources associated with the execution are released, so don't worry too much about it.
Note that this is not entirely true, PHP's memory management allocates memory using either emalloc or pemalloc, where the p stands for persistent. Some resources are kept alive (DB connections for example), or FastCGIBut keeping PHP alive, but that's besides the point here.

Some things, however, are still your responsibility. A common source of leaks in most languages are streams, handlers, or other types of resources that weren't closed correctly, or were not closed at all. Your code contains a textbook example of leaks caused by file pointers being left open:

//in mem_test method
$handle = fopen("/Users/mikekilmer/Sites/mzoo/dev/php/open_cart_memory.txt", "a+");

You open this file, but never call fclose. The file is opened, and the resource is assigned to $handle, a local variable that goes out of scope when the method is returned. call fclose before returning.

You are also using a DB connection, in which case: make sure you close any cursor, clear your prepared statements, and finally: disconnect when you need to.

Other than that, there's not enough code to go on here, to determine what is hogging all the memory.

Some background, and basics on PHP memory management

Bottom line is: most of the time PHP's Zend engine, and the extensions you use are expected to manage the memory for you. And, again, most of the time: they will. There are situations where this is not possible, however.
PHP manages the memory using refcounts. All variables are passed around as zval * internally, and each of these zval * are pointers to a struct that has a member called refcount, which counts how many variables are associated with this value. If a variable is re-assigned, or goes out of scope, the refcount is updated (something like ((zval *) internal_rep_of_val)->refcount -=1;). If the refcount member reaches zero, the memory is freed.

I could go into more details here, but the bottom line is: refcounting has some advantages over other management tactics (like tracing), but it has a couple of weak-spots, too.
If you keep these issues in mind, and avoid writing code that is difficult to manage using refcounts, you're doing your job just fine. If an extension leaks extensively, just file a bug-report.

class A
{
 private $myself = null;
 public function __construct()
 {
 $this->myself = $this;
 }
}
$x = new A();//MEMORY LEAK!

Think about it: A new instance of A is created, and assigned to $x. So the instance's zval refcount is set to 1. But in the constructor, the instance references itself, by assigning $this to a private property. So the refcount is incremented again. But $this->myself can't be accessed outside of the class. So if $x goes out of scope, or is reassigned, you have no way to access the instance again. Yet, the zval's refcount will never reach zero.
A destructor won't help, because the instance can't ever be truly destroyed.

This is just a simple example, but consider passing instances around to other objects, that are "aware" of each other:

class Foo
{
 protected $owner = null;
 public function register(Bar $owner)
 {
 $this->owner = $owner->register($this);
 return $this;
 }
}
class Bar
{
 protected $owns = null;
 public function register(Foo $owns)
 {
 $this->owns = $owns;
 return $this;
 }
}
$x = new Foo;
$y = new Bar;
$x->register($y);//LEAK

In this situation (which is really quite common), neither Foo or Bar can be garbage collected: Each instance references the other, so both instances will never reach refcount 0.

Things that are difficult to GC(Garbage Collect) include:

• Closures are hard to GC, especially if you add function() use ($var), or worse, still: function() use (&$var). Increasingly difficult to go without, though
• Statics are never GC'ed, they remain in memory until the script exits. I've used 1 static in the past 3 years, so it's quite easy to do without them
• avoid, if at all possible, any form of circular referencing (impossible to avoid in reality)

Sorry if this doesn't actually review your code, but your question seems to be more one of "why am I (seemingly) leaking memory". Things that will further help you to track down (and possibly fix) your issues include:

• Code analysis tools like this
• PHP-Valgrind to see the opcodes, and what memory is being used where
• Install Xdebug, Xhprof and the like, to step through your code, and see when the memory usage spikes

---

Late update:

To clarify what kind of "leaks" I'm actually talking about and why they're not really something you should be too concerned about.

Throughout this answer, I gave a couple of examples of "memory leaks". Again, this sounds like a big problem, but when I'm talking about memory leaks in PHP, I'm mainly talking about memory being allocated throughout the request cycle. The PHP runtime has 4 distinct phases: PHP_MINIT, PHP_RINIT, PHP_RSHUTDOWN, and finally PHP_MSHUTDOWN. This M in MINIT and MSHUTDOWN stands for Module. During these phases, a module can initialize certain resources, and configure itself based on ini settings and such.

Similarly, the R in RINIT and RSHUTDOWN stands for Request. Your PHP code essentially executes between RINIT and RSHUTDOWN. The overwhelming majority of leaks in PHP that you create in PHP code mean that your resources will stay in memory throughout the request lifecycle. During RSHUTDOWN, however, the memory is freed.

Examples I gave (eg circular referencing, failing to close a db connection, and not closing file pointers) will prevent the associated resources to be freed the moment you stop using them, but the resources will be freed during/after RSHUTDOWN.
In the same way that, in C memory you allocate on heap (malloc & co) will remain allocated unitl your program exits. Technically, this is a memory leak, but most systems today will reclaim the memory on termination.
Even though you can, for the most part, rely on PHP cleaning up after you, it's still worth getting in to the habit of closing file pointers, and DB connections yourself. It will help you greatly when you decide to pick up a different, more low-level language further down the line (like C, Go, or C++).

True, problematic memory leaks occur when a PHP module allocates resources in MINIT, and doesn't free them during MSHUTDOWN, or allocates the same resources during RINIT. This is clearly a bug in the extension source code, and cannot be avoided by writing your PHP code differently. Sometimes, modules contain edge-case bugs that cause resources allocated during MINIT to grow, without them being released properly during RSHUTDOWN. In these very rare cases, writing your PHP code differently might solve the issue, but you're better of looking for an alternative extension.

Lastly: the impact these bugs have heavily depends on the runtime: if you're still using the old mod_php, you'll never know the problems are there. If you're using php-fpm, however, the memory consumption can grow over time. Again: this is indicative of a bug in the C code for an extension, and needs to be fixed there. Thankfully, these bugs are rare.

• MikeiLL: Thank you for the VERY insightful reply, man. Are emalloc and pemalloc functions of c? That's the language php is built on, right? When you say that $this->myself = $this; cannot be accessed outside the class, do you mean there is a struct attached to it, that is not pointed to by a zval *? Or maybe that the struct does not contain the refcount? Or that the struct and zval * are there but something it preventing them from accessing it due to the rules of php's OOP private variables?
• Elias Van Ootegem: @MikeiLL: [p]emalloc are the custom memory management functions in the PHP core. Every PHP variable is a zval *. This zval is a struct. $this is a zval * that points to the PHP object instance, and by doing $this->myself = $this The refcount of the zval * that holds $this is set to 2: 1 because it is referenced by $x, and 1 because the property myself references $this. If you reassing $x, the refcount of $this->myself remains, but you can't access it, and so you leak memory
• Elias Van Ootegem: @MikeiLL: If you're interested in the inner workings of the Zend Engine (PHP core), here's the place to look, along with the php-src github repo
• MikeiLL: Is the difference between zval * and zval sort of like the difference between an instance and a class? And when you say you can't access the refcount of $this->myself, who is you? The script?
• Elias Van Ootegem: @MikeiLL: zval * is a pointer to a zval, and zval is a typedef (~=alias)of the struct _zval_struct struct. It refers to the PHP source code, so it's C, nothing to do with instance or classes. Yes, I'm sure you cannot acces $this->myself once $x is no longer exists or is reassigned: $this->myself is non-static, non-private, and inherent to the instance. You have no variable referencing the instance, outside of itself. You can't cast the instance, because you have no variable to cast, and adding code to the class won't help, because you have no means of calling methods

Answer 2

What is under the covers of your db->query()? If it's MySQLi, then that is your memory leak right there. MySQLi returns a result set that has to be explicitly freed. The result_set from mysqli_query($sql) needs to be freed with mysqli_free_result($result_set).

• MikeiLL: If I recall it was MySQLi.So would $qry then need to be deleted to "free" the memory?