aparkhomenko / php-cassandra

PHP extension for original DataStax C/C++ Driver for Apache Cassandra
GNU General Public License v2.0
39 stars 6 forks source link

Memory leak, CqlQuery objects never get deleted #9

Open hw-dwalter opened 9 years ago

hw-dwalter commented 9 years ago

Problem

Instances of the CqlQuery class in PHP never gets deleted. While looking deeper at this bug, arouse suspicion this may be a bug in all classes exposed to PHP.

In my case this quickly exceeds all of the available memory. This PHP code shows up the problem:

while(true) {
    $cql = new CqlQuery('SomeMBLargeString');
}

In such a case this c++ code is evaluated in each round of the loop

obj->cql_query = boost::shared_ptr<cql::cql_query_t> (new cql::cql_query_t(query_string, cql::CQL_CONSISTENCY_DEFAULT, is_traced, false));

Proof

This new object of type cql::cql_query_t never gets deleted! I also did a valgrind memory check to find this problem. Here you can see the corresponding valgrind output:

==3968== 9,162,720 bytes in 1 blocks are possibly lost in loss record 273 of 273
==3968==    at 0x4C29180: operator new(unsigned long) (vg_replace_malloc.c:324)
==3968==    by 0x119A9E98: std::string::_Rep::_S_create(unsigned long, unsigned long, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.20)
==3968==    by 0x119AB814: char* std::string::_S_construct<char const*>(char const*, char const*, std::allocator<char> const&, std::forward_iterator_tag) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.20)
==3968==    by 0x119ABC45: std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.20)
==3968==    by 0x10F70FB4: zim_CqlQuery___construct(int, _zval_struct*, _zval_struct**, _zval_struct*, int) (cassandra.cpp:775)
==3968==    by 0x6C0768: dtrace_execute_internal (in /usr/bin/php5)
==3968==    by 0x77FD6A: ??? (in /usr/bin/php5)
==3968==    by 0x73FE77: execute_ex (in /usr/bin/php5)
==3968==    by 0x6C063C: dtrace_execute_ex (in /usr/bin/php5)
==3968==    by 0x7803D5: ??? (in /usr/bin/php5)
==3968==    by 0x73FE77: execute_ex (in /usr/bin/php5)
==3968==    by 0x6C063C: dtrace_execute_ex (in /usr/bin/php5)

Proposal for solution

I am not an expert in PHP extension development, but it seems to me that there is no dtor defined in this calls:

retval.handle = zend_objects_store_put(obj, NULL, NULL, NULL TSRMLS_CC);

The zend_objects_store_put function is defined as this:

zend_object_handle
 zend_objects_store_put (void *object, zend_objects_store_dtor_t dtor, zend_objects_free_object_storage_t free_storage, zend_objects_store_clone_t clone TSRMLS_DC)

My suggestion is to add appropriate dtor functions in all zend_objects_store_put calls. As far as I can see, this problem exists for all classes!

Workaround in PHP

If you avoid building new CqlQuery objects each step in the loop, the memory leak does not kills your memory too quick. Rearrange the PHP code like this works around this bug:

$cql = new CqlQuery('');
while(true) {
    $cql->setQueryString('SomeMBLargeString');
}
aparkhomenko commented 9 years ago

Hello @hw-dwalter

Thanks for your request. Yes, this is a wider problem if you create a long life script because more objects in original extension use store pointer as boost::shared_ptr and zend engine can't free them. But you're right if we create custom destructor on extension side it could be good decision. I'll try to do that as soon as possible.

stefanis commented 7 years ago

Any progress on this bug? I'm getting bit by it hard. Even with the workaround I'm still seeing a 1KB/query leak. It doesn't sound like much, but my script runs for hours with millions of queries. I end up leaking 4GB, at which point the script gets killed.