[MEMLEAK] Memory Leak in RequestTest

[ghost]

Testing RequestTest [OK] (262144) (66048) (1.6658)
[Mon Sep 23 12:06:42 2013]  Script:  '/home/vladimir/workspace/cphalcon/unit-tests/manual-unit.php'
/home/vladimir/workspace/cphalcon/ext/kernel/array.c(702) :  Freeing 0x7FF238E74B48 (32 bytes), script=/home/vladimir/workspace/cphalcon/unit-tests/manual-unit.php
Last leak repeated 1 time
[Mon Sep 23 12:06:42 2013]  Script:  '/home/vladimir/workspace/cphalcon/unit-tests/manual-unit.php'
/home/vladimir/workspace/cphalcon/ext/kernel/array.c(703) :  Freeing 0x7FF238E7DE50 (7 bytes), script=/home/vladimir/workspace/cphalcon/unit-tests/manual-unit.php
Last leak repeated 1 time
[Mon Sep 23 12:06:42 2013]  Script:  '/home/vladimir/workspace/cphalcon/unit-tests/manual-unit.php'
/tmp/php-build/source/5.4.10/ext/filter/sanitizing_filters.c(162) :  Freeing 0x7FF238EF0AD8 (4 bytes), script=/home/vladimir/workspace/cphalcon/unit-tests/manual-unit.php
/tmp/php-build/source/5.4.10/Zend/zend_alloc.c(2529) : Actual location (location was relayed)
Last leak repeated 5 times
=== Total 10 memory leaks detected ===

[ghost]

@dreamsxin

One of the leaks I have fixed:

        PHALCON_INIT_VAR(key);
        ZVAL_STRING(key, "PHP_AUTH_USER", 1);

        value = phalcon_hash_get(Z_ARRVAL_P(_SERVER), key, BP_VAR_NA);
        if (value && Z_TYPE_PP(value) == IS_STRING) {
            auth_user = Z_STRVAL_PP(value);
        }

+       zval_dtor(key);
        ZVAL_STRING(key, "PHP_AUTH_PW", 1);

You were setting a new value to kety without freeing the old one.

The other change was

        char *auth_user = SG(request_info).auth_user;
-       char *auth_password = SG(request_info).auth_user;
+       char *auth_password = SG(request_info).auth_password;

[ghost]

Two other leaks:

-       PHALCON_INIT_VAR(auth);
-       array_init(auth);       
+       array_init_size(return_value, 2);

-       phalcon_array_update_string_string(&auth, SL("username"), auth_user, strlen(auth_user), PH_COPY | PH_SEPARATE);
-       phalcon_array_update_string_string(&auth, SL("password"), auth_password, strlen(auth_password), PH_COPY | PH_SEPARATE);
+       phalcon_array_update_string_string(&return_value, SL("username"), auth_user, strlen(auth_user), 0);
+       phalcon_array_update_string_string(&return_value, SL("password"), auth_password, strlen(auth_password), 0);

auth => return_value change is not important here, the bug was in PH_COPY.

There is no sense to use PH_COPY if you pass something other than zval* (this is the case for phalcon_array_update_string_long, phalcon_array_update_string_bool, phalcon_array_update_string_string functions).

PH_COPY will call Z_ADDREF_P() on the temporary value created by phalcon_array_update_string_XXX function and as a result, that temporary value will have refcount = 2 and will never be freed.

[dreamsxin]

Thank you sjinks,
There is question:
SG(request_info).request_uri and $_SERVER["REQUEST_URI"] are different from the value of it?

Help me see function Request::getURI

PHP_METHOD(Phalcon_Http_Request, getURI){

    zval **value;
    const char *uri = SG(request_info).request_uri;
    if (unlikely(!uri)) {
        zval *_SERVER, key;

        INIT_ZVAL(key);
        ZVAL_STRING(&key, "REQUEST_URI", 0);

        phalcon_get_global(&_SERVER, SS("_SERVER") TSRMLS_CC);
        value = phalcon_hash_get(Z_ARRVAL_P(_SERVER), &key, BP_VAR_NA);
        if (value && Z_TYPE_PP(value) == IS_STRING) {
            uri = Z_STRVAL_PP(value);
        }
    }

    if (uri) {
        RETURN_STRING(uri, 1);
    }

    RETURN_EMPTY_STRING();
}

[ghost]

More leaks (could be my fault):

@@ -191,6 +191,7 @@ PHP_METHOD(Phalcon_Http_Request, get){
                                phalcon_call_method_p3(return_value, filter, "sanitize", value, filters, norecursive);

                                if (PHALCON_IS_EMPTY(return_value) && !zend_is_true(allow_empty)) {
+                                       zval_dtor(return_value);
                                        RETURN_CCTOR(default_value);
                                } else {
                                        RETURN_MM();

PHALCON_IS_EMPTY() will evaluate to true if an empty string is passed; however, empty strings do take some memory and therefore we need to free it before trying to assign a different value.

[phalcon]

Fixed by @sjinks