php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #23436 IBASE library does not free statement resources
Submitted: 2003-05-01 06:22 UTC Modified: 2003-05-13 18:29 UTC
From: pcisar at ibphoenix dot cz Assigned:
Status: Closed Package: InterBase related
PHP Version: 4.3.1 OS: All
Private report: No CVE-ID: None
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: pcisar at ibphoenix dot cz
New email:
PHP Version: OS:

 

 [2003-05-01 06:22 UTC] 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.

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2003-05-05 18:26 UTC] 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-13 18:29 UTC] sniper@php.net
No feedback, assuming this is fixed. 

 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Dec 21 15:01:29 2024 UTC