Nov 18, 2011

TYPO3 developers, say "no" to memory leaks!

There is one small mistake I often see in TYPO3 extensions that I review. It is related to freeing resources after you have done with them. Many people forget to do that. PHP is a language where you do not have to free memory. That's true. But resources (such as file handles, recordset, etc) are different. You have to close them explicitly! PHP may close file for you sometimes but even that is not guaranteed.
Let's see the problematic code:
function memoryLeakingGetDatabaseRow() {
    $resource = $GLOBALS['TYPO3_DB']->exec_SELECTquery(...);
    return $GLOBALS['TYPO3_DB']->sql_fetch_assoc($resource);
}
What is wrong here? This code fetches a recordset from the database but does not free it. This means that all structures MySQL had to prepare live in memory. This may include portions of the data retrieved from the database. That data lives for the whole request. If you make multiple calls you accumulate more and more such data in memory. That leaves less memory for other processes on the server. This data is tied to the database connection. If you have persistent database connections, this data is not released when your script terminates because the connection is not released. Thus you have data accumulating all the time. The result is slow down of the server after a certain period of time (low memory means more swapping and that is slow).
How this can be prevented? Code:
function memoryLeakingGetDatabaseRow() {
    $resource = $GLOBALS['TYPO3_DB']->exec_SELECTquery(...);
    $row = $GLOBALS['TYPO3_DB']->sql_fetch_assoc($resource);
    $GLOBALS['TYPO3_DB']->sql_free_result($resource);
    return $row;
}
This releases the recordset properly and frees all allocated memory.
[Note: of course, for a single result you can use exec_SELECTgetSingle but the point of this article was to show the problem and the solution]

7 comments:

  1. Sebastiaan de JongeNovember 18, 2011 at 2:26 PM

    Hi Dmitry,



    Nice post, this is indeed something that many developers neglect. I have to admit that often I also do not free results (only with bigger amounts of data etc. I do free results). It's a nice thing to keep in mind though, also for the sake of writing proper code.



    Cheers,

    Sebastiaan

    ReplyDelete
  2. I general I think such code with custom sql writing is source of problems. You always will forget about leak. So i think all such low-level operations have to be hidden by DAO objects and accessed via methods like getAllNews(), getAllNewsByCategoryId() etc.

    ReplyDelete
  3. This is not always a perfect approach. You should call memory_get_usage() at the end of the script to check if You need to free the memory in such funny way.



    Ref. : http://www.php.net/manual/en/function.mysql-free-result.php

    ReplyDelete
  4. Fedir, you point to the comment :) I believe it is not correct and memory was allocated by something else in the script. I never saw proof that using this function increases the memory but I always saw that Apache memory usage stops growing if this function is used.

    ReplyDelete
  5. Honestly, Dmitry, I think it's very good approach to clean the memory during development, especially, if requests brings a lot of data, when You know, what You will get huge amount of results.



    In the same time, the memory anyway will be freed at the end of the script execution. So, for regular requests, where You will receive only page Id or single label, there no need to do such stuff.



    It's the same, like variables, if we will unset each variable after it's use without to wait auto-unset, the code could became difficult to read and maintain.



    So, only for huge data.

    ReplyDelete
  6. Yes, there is.



    Firsts, if you do not do it once, how you can be sure you will do it next time? It must become a habit to clean up after yourself.



    Secondly, on a high load web site with persistent connections those memory pieces will not be cleaned up and will accumulate to a large chunk of allocated but unused memory.



    And I am not saying about unset. There is no a word about it in the article. It is a bout freeing recordsets, which belong to MySQL. This is entirely different thing from PHP variables.

    ReplyDelete
  7. I love this post. Typo3 developpers should free the resources used. Typo3 consumes a lot of resources (memory, disk space). Freeing resource increases drastically the performance of your typo3 extension and add more scalability to the extension.



    Imagine a high traffic website with 12000 tt_news for example. Without freeing resource the website could not support high charge.

    ReplyDelete