#23436 [NEW]: IBASE library does not free statement resources

From: pcisar at ibphoenix dot cz Operating system: All PHP version: 4.3.1 PHP Bug Type: InterBase related Bug description: IBASE library does not free statement resources This bug is related to IBASE library (ext/interbase) and is located in interbase.c file. Basically, author of IBASE library had wrong assumptions how InterBase/Firebird works with statement resources. He/she thought that statement resources are related to transaction and thus freed on commit/rollback. Actually, statements belong to connection and are freed on connection close. This leads to wrong implementation of _php_ibase_free_result and _php_ibase_free_query that in turn do not free statement resources when transaction is not active when either function is called. This may not cause many trouble when one uses connect, but with pconnect one would get disastrous results: a very memory hungry InterBase/Firebird server, and in turn a very slow application. Under heavy load, one can see how MB's of memory are vanishing in real-time. At the very end, everything would crash. I'd consider this bug as *very* serious, as it renders any serious use of InterBase/Firebird with PHP impossible (connection times are indispensable, and one would need to take an advantage from pconnect). The fix is very simple: This is the original _php_ibase_free_query from PHP 4.3.1 distribution: /* {{{ _php_ibase_free_query() */ static void _php_ibase_free_query(ibase_query *ib_query) { char tr_items[] = {isc_info_tra_id }; char tmp[32] ; /* ...should be enough as on the Api doc */ TSRMLS_FETCH(); IBDEBUG("Freeing query..."); if (ib_query) { if (ib_query->in_sqlda) { efree(ib_query->in_sqlda); } if (ib_query->out_sqlda) { efree(ib_query->out_sqlda); } //--->Here is the bug isc_transaction_info(IB_STATUS, &ib_query->trans,sizeof(tr_items), tr_items, sizeof(tmp), tmp ); /* we have the trans still open and a statement to drop? */ if ( !(IB_STATUS[0] && IB_STATUS[1]) && ib_query->stmt) { IBDEBUG("Dropping statement handle (free_query)..."); if (isc_dsql_free_statement(IB_STATUS, &ib_query->stmt, DSQL_drop)){ _php_ibase_error(TSRMLS_C); } } //<--- if (ib_query->in_array) { efree(ib_query->in_array); } if (ib_query->out_array) { efree(ib_query->out_array); } efree(ib_query); } } It should be: /* {{{ _php_ibase_free_query() */ static void _php_ibase_free_query(ibase_query *ib_query) { char tr_items[] = {isc_info_tra_id }; char tmp[32] ; /* ...should be enough as on the Api doc */ TSRMLS_FETCH(); IBDEBUG("Freeing query..."); if (ib_query) { if (ib_query->in_sqlda) { efree(ib_query->in_sqlda); } if (ib_query->out_sqlda) { efree(ib_query->out_sqlda); } // --> Here is the change if ( ib_query->stmt) { IBDEBUG("Dropping statement handle (free_query)..."); if (isc_dsql_free_statement(IB_STATUS, &ib_query->stmt, DSQL_drop)){ _php_ibase_error(TSRMLS_C); } } // <--- if (ib_query->in_array) { efree(ib_query->in_array); } if (ib_query->out_array) { efree(ib_query->out_array); } efree(ib_query); } } /* }}} */ The fix for _php_ibase_free_result is more complicated as another bug/wrong asumption came to play here. The call to isc_dsql_free_statement depends not only on transaction state, but also on value of ib_result->drop_stmt variable. This should switch the use of DSQL_drop/DSQL_close parameter in isc_dsql_free_statement. This variable is set to 1 (drop) in ibase_query (which is ok). While DSQL_drop frees resources allocated for statement, DSQL_close just closes a cursor. From comments in source, it's evident that author thought that cursor should be closed when the statement returns values (The ib_result->drop_stmt is set to zero (close) in _php_ibase_exec when statement returns values). Actually, a cursor need only be closed in this manner if it was previously opened and associated with stmt_handle by isc_dsql_set_cursor_name(). But IBASE library doesn't use or allow to use named cursors at all. Anyway, someone commented out all important code from this "close" branch :-) Together, this dug a very big hole in server resources. I'd like suggest next fix for this: 1) comment out the line in _php_ibase_exec where // IB_RESULT->drop_stmt = 0; /* when free result close but not drop!*/ 2) change the php_ibase_free_result to this one: /* {{{ _php_ibase_free_result() */ static void _php_ibase_free_result(zend_rsrc_list_entry *rsrc TSRMLS_DC) { char tr_items[] = {isc_info_tra_id }; char tmp[32]; /* should be enough as on the Api doc */ ibase_result *ib_result = (ibase_result *)rsrc->ptr; IBDEBUG("Freeing result..."); if (ib_result){ _php_ibase_free_xsqlda(ib_result->out_sqlda); //---> Here is the change if ( ib_result->stmt ) { if ( ib_result->drop_stmt ) { IBDEBUG("Dropping statement handle (free_result)..."); if (isc_dsql_free_statement(IB_STATUS, &ib_result->stmt, DSQL_drop)) { _php_ibase_error(TSRMLS_C); } } else { IBDEBUG("Closing statement handle..."); if (isc_dsql_free_statement(IB_STATUS, &ib_result->stmt, DSQL_close)) { _php_ibase_error(); } } } //<--- if (ib_result->out_array) { efree(ib_result->out_array); } efree(ib_result); } } /* }}} */ This way, there is still a possibility to implement named cursors in future and take advantage from existing ib_result->drop_stmt. You can contact me if you'd like additional information about this bug/suggested solution. -- Edit bug report at http://bugs.php.net/?id=23436&edit=1 -- Try a CVS snapshot: http://bugs.php.net/fix.php?id=23436&r=trysnapshot Fixed in CVS: http://bugs.php.net/fix.php?id=23436&r=fixedcvs Fixed in release: http://bugs.php.net/fix.php?id=23436&r=alreadyfixed Need backtrace: http://bugs.php.net/fix.php?id=23436&r=needtrace Try newer version: http://bugs.php.net/fix.php?id=23436&r=oldversion Not developer issue: http://bugs.php.net/fix.php?id=23436&r=support Expected behavior: http://bugs.php.net/fix.php?id=23436&r=notwrong Not enough info: http://bugs.php.net/fix.php?id=23436&r=notenoughinfo Submitted twice: http://bugs.php.net/fix.php?id=23436&r=submittedtwice register_globals: http://bugs.php.net/fix.php?id=23436&r=globals PHP 3 support discontinued: http://bugs.php.net/fix.php?id=23436&r=php3 Daylight Savings: http://bugs.php.net/fix.php?id=23436&r=dst IIS Stability: http://bugs.php.net/fix.php?id=23436&r=isapi Install GNU Sed: http://bugs.php.net/fix.php?id=23436&r=gnused