#23436 [Fbk->Csd]: IBASE library does not free statement resources
ID: 23436
Updated by: sniper@php.net
Reported By: pcisar at ibphoenix dot cz
-Status: Feedback
+Status: Closed
Bug Type: InterBase related
Operating System: All
PHP Version: 4.3.1
New Comment:
No feedback, assuming this is fixed.
Previous Comments:
------------------------------------------------------------------------
[2003-05-05 18:26:10] daniela@php.net
Many thanks for your report.
I've just committed in current CVS and php_4_3
Please try using this CVS snapshot:
http://snaps.php.net/php4-latest.tar.gz
For Windows:
http://snaps.php.net/win32/php4-win32-latest.zip
------------------------------------------------------------------------
[2003-05-01 06:22:42] pcisar at ibphoenix dot cz
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 this bug report at http://bugs.php.net/?id=23436&edit=1
Thread (3 messages)