Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:11583
Return-Path: <dlawson@masterytech.com>
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Delivered-To: mailing list internals@lists.php.net
Received: (qmail 18621 invoked by uid 1010); 27 Jul 2004 11:47:10 -0000
Delivered-To: ezmlm-scan-internals@lists.php.net
Delivered-To: ezmlm-internals@lists.php.net
Received: (qmail 16812 invoked from network); 27 Jul 2004 11:46:49 -0000
Received: from unknown (HELO mail07d.vwh1.net) (207.201.152.72)
  by pb1.pair.com with SMTP; 27 Jul 2004 11:46:49 -0000
Received: from www.masterytech.com (208.55.109.138)
	by mail07d.vwh1.net (RS ver 1.0.94vs) with SMTP id 2-1375308330;
	Tue, 27 Jul 2004 07:38:53 -0400 (EDT)
Message-ID: <41063ECA.5010903@masterytech.com>
Date: Tue, 27 Jul 2004 07:38:50 -0400
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5) Gecko/20031007
X-Accept-Language: en-us, en
MIME-Version: 1.0
To:  internals@lists.php.net
CC:  kalowsky@php.net
Content-Type: multipart/mixed;
 boundary="------------030302000606050708070307"
X-Loop-Detect:1
Subject: [PATCH] Three patches for odbc module
From: dlawson@masterytech.com (Dave Lawson)

--------------030302000606050708070307
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Hello,

I discovered these problems during a project I'm working on. There are
three separate patches involving different areas of the odbc module:

php_odbc.c.patch1.txt
=====================
Wherever an odbc_result is allocated, "emalloc" is used. This may cause 
random
behavior in certain php_odbc methods if some odbc_result members are not
initialized. I had a specific problem with the fetch_abs member of 
odbc_result.
This member would contain garbage at random when calling odbc_gettypeinfo().
Since fetch_abs was non-zero, all of the odbc_fetch_xxx() functions would
try to use SQLExtendedFetch(). The driver I'm using doesn't support this 
method
so the method would appear to fail. The solution was to replace all 
odbc_result
"emalloc" allocations with "ecalloc".


php_odbc.c.patch2.txt
=====================
There are resource leaks when using any of the following php_odbc functions:

odbc_tables()
odbc_columns()
odbc_columnprivileges()
odbc_foreignkeys()
odbc_gettypeinfo()
odbc_primarykeys()
odbc_procedures()
odbc_procedurecolumns()
odbc_specialcolumns()
odbc_statistics()
odbc_tableprivileges()

Each of these functions allocates a new odbc_result resource. When
odbc_free_result() is called to free a resource from one of these 
functions, it
tries to use the "id" member of the odbc_result struct to delete the 
resource
from the global resource list. Since none of these functions initialize 
the "id"
member, the delete silently fails and the resource stays allocated. After
further investigation, there doesn't seem to be a real purpose to the "id"
member. The only functions that initialize it are odbc_prepare() and
odbc_exec(). Rather than using this member as the resource index to 
delete in
odbc_free_result(), why not just use the value of the zval passed in to
odbc_free_result()? This patch removes references to the "id" member in all
affected functions.


php_odbc.c.patch3.txt
=====================
Reference counting for odbc_connection resources has issues. There are two
problems:

First, odbc_prepare() and odbc_exec() both add references to the connection
(presumably for the the conn_ptr member of odbc_result). The code that 
should
delete these references appears to be commented out in the 
_free_odbc_result()
function (the odbc_result destructor). No other php_odbc functions add these
references and they appear to be unnecessary. When an obdc_connection is 
freed
via _close_odbc_conn() (the odbc_connection destructor), the function
scans the resource list and deletes any existing odbc_result references
associated with that connection.

The second problem has to do with the fact the odbc_connections are cached.
Please see the attached patch3-example.php file for an explanation of the
problem and solution.





--------------030302000606050708070307
Content-Type: text/plain;
 name="patch3-example.php"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="patch3-example.php"

<?php

/**
 * EXPLANATION:
 *
 * The php_odbc module uses a connection cache for providing resources via
 * odbc_connect(). If two separate calls are made to odbc_connect() with the
 * same dsn/uid/pwd info, then two different zvals will be returned pointing
 * to the same connection resource. The problem has to do with reference
 * counting for the allocated resource once odbc_close() is called.
 *
 * In the example below, we'll use CR to refer to the connection resource
 * created by a call to odbc_connect().
 *
 * $conn = odbc_connect($dsn, null, null);
 *
 * This allocates CR with a refcount of one and assigns it to the $conn
 * variable. Subsequently, in the call to test(), a duplicate connect call
 * is generated which simply increments CR's refcount and assigns it to $conn2:
 *
 * $conn2 = odbc_connect($dsn, null, null);
 *
 * Now we have CR with a refcount of two, assigned to both $conn and $conn2.
 * Next, the connection is closed in test():
 *
 * odbc_close($conn2);
 *
 * The call to odbc_close() decrements CR (now back to one). The problem occurs
 * as we leave the scope of test(). When this happens, the zval destructor of
 * $conn2 is called. This destructor notices that $conn2 is still pointing at a
 * valid resource and so decrements the reference. This puts CR's refcount back
 * to zero, triggering the resource to be freed. Once we're back in global
 * scope, the $conn variable is now pointing at an invalid resource which it
 * can no longer use.
 *
 * SOLUTION:
 *
 * Change odbc_close() so that it sets $conn (the zval parm) to null before
 * returning. This would prevent the zval parm's destructor from trying to
 * decrement the connection resource's refcount.
 *
 */
$dsn = "DSN=MasteryNetDBSystem;";

function test($dsn)
{
    echo "test start\n";
    $conn2 = odbc_connect($dsn, null, null);
    odbc_close($conn2);
    echo "test end\n";
}

$conn = odbc_connect($dsn, null, null);
test($dsn);
$result = odbc_exec($conn, "SELECT COUNT(*) FROM products");
odbc_close($conn);
echo "done\n";

?>
--------------030302000606050708070307
Content-Type: text/plain;
 name="php_odbc.c.patch1.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="php_odbc.c.patch1.txt"

Index: php_odbc.c
===================================================================
RCS file: /repository/php-src/ext/odbc/php_odbc.c,v
retrieving revision 1.178
diff -u -r1.178 php_odbc.c
--- php_odbc.c	18 Jun 2004 00:44:35 -0000	1.178
+++ php_odbc.c	26 Jul 2004 20:10:26 -0000
@@ -846,7 +846,7 @@
 	convert_to_string_ex(pv_query);
 	query = Z_STRVAL_PP(pv_query);
 
-	result = (odbc_result *)emalloc(sizeof(odbc_result));
+	result = (odbc_result *)ecalloc(1, sizeof(odbc_result));
 	
 	result->numparams = 0;
 	
@@ -1269,7 +1269,7 @@
 	convert_to_string_ex(pv_query);
 	query = Z_STRVAL_PP(pv_query);
 	
-	result = (odbc_result *)emalloc(sizeof(odbc_result));
+	result = (odbc_result *)ecalloc(1, sizeof(odbc_result));
 
 	rc = SQLAllocStmt(conn->hdbc, &(result->stmt));
 	if (rc == SQL_INVALID_HANDLE) {
@@ -2797,7 +2797,7 @@
 
 	ZEND_FETCH_RESOURCE2(conn, odbc_connection *, pv_conn, -1, "ODBC-Link", le_conn, le_pconn);
 
-	result = (odbc_result *)emalloc(sizeof(odbc_result));
+	result = (odbc_result *)ecalloc(1, sizeof(odbc_result));
 	
 	rc = SQLAllocStmt(conn->hdbc, &(result->stmt));
 	if (rc == SQL_INVALID_HANDLE) {
@@ -2881,7 +2881,7 @@
 
 	ZEND_FETCH_RESOURCE2(conn, odbc_connection *, pv_conn, -1, "ODBC-Link", le_conn, le_pconn);
 
-	result = (odbc_result *)emalloc(sizeof(odbc_result));
+	result = (odbc_result *)ecalloc(1, sizeof(odbc_result));
 	
 	rc = SQLAllocStmt(conn->hdbc, &(result->stmt));
 	if (rc == SQL_INVALID_HANDLE) {
@@ -2961,7 +2961,7 @@
 
 	ZEND_FETCH_RESOURCE2(conn, odbc_connection *, pv_conn, -1, "ODBC-Link", le_conn, le_pconn);
 
-	result = (odbc_result *)emalloc(sizeof(odbc_result));
+	result = (odbc_result *)ecalloc(1, sizeof(odbc_result));
 	
 	rc = SQLAllocStmt(conn->hdbc, &(result->stmt));
 	if (rc == SQL_INVALID_HANDLE) {
@@ -3055,7 +3055,7 @@
 
 	ZEND_FETCH_RESOURCE2(conn, odbc_connection *, pv_conn, -1, "ODBC-Link", le_conn, le_pconn);
 
-	result = (odbc_result *)emalloc(sizeof(odbc_result));
+	result = (odbc_result *)ecalloc(1, sizeof(odbc_result));
 	
 	rc = SQLAllocStmt(conn->hdbc, &(result->stmt));
 	if (rc == SQL_INVALID_HANDLE) {
@@ -3130,7 +3130,7 @@
 
 	ZEND_FETCH_RESOURCE2(conn, odbc_connection *, pv_conn, -1, "ODBC-Link", le_conn, le_pconn);
 
-	result = (odbc_result *)emalloc(sizeof(odbc_result));
+	result = (odbc_result *)ecalloc(1, sizeof(odbc_result));
 	
 	rc = SQLAllocStmt(conn->hdbc, &(result->stmt));
 	if (rc == SQL_INVALID_HANDLE) {
@@ -3198,7 +3198,7 @@
 
 	ZEND_FETCH_RESOURCE2(conn, odbc_connection *, pv_conn, -1, "ODBC-Link", le_conn, le_pconn);
 
-	result = (odbc_result *)emalloc(sizeof(odbc_result));
+	result = (odbc_result *)ecalloc(1, sizeof(odbc_result));
 	
 	rc = SQLAllocStmt(conn->hdbc, &(result->stmt));
 	if (rc == SQL_INVALID_HANDLE) {
@@ -3276,7 +3276,7 @@
 
 	ZEND_FETCH_RESOURCE2(conn, odbc_connection *, pv_conn, -1, "ODBC-Link", le_conn, le_pconn);
 
-	result = (odbc_result *)emalloc(sizeof(odbc_result));
+	result = (odbc_result *)ecalloc(1, sizeof(odbc_result));
 	
 	rc = SQLAllocStmt(conn->hdbc, &(result->stmt));
 	if (rc == SQL_INVALID_HANDLE) {
@@ -3354,7 +3354,7 @@
 
 	ZEND_FETCH_RESOURCE2(conn, odbc_connection *, pv_conn, -1, "ODBC-Link", le_conn, le_pconn);
 
-	result = (odbc_result *)emalloc(sizeof(odbc_result));
+	result = (odbc_result *)ecalloc(1, sizeof(odbc_result));
 	
 	rc = SQLAllocStmt(conn->hdbc, &(result->stmt));
 	if (rc == SQL_INVALID_HANDLE) {
@@ -3436,7 +3436,7 @@
 
 	ZEND_FETCH_RESOURCE2(conn, odbc_connection *, pv_conn, -1, "ODBC-Link", le_conn, le_pconn);
 
-	result = (odbc_result *)emalloc(sizeof(odbc_result));
+	result = (odbc_result *)ecalloc(1, sizeof(odbc_result));
 	
 	rc = SQLAllocStmt(conn->hdbc, &(result->stmt));
 	if (rc == SQL_INVALID_HANDLE) {
@@ -3517,7 +3517,7 @@
 
 	ZEND_FETCH_RESOURCE2(conn, odbc_connection *, pv_conn, -1, "ODBC-Link", le_conn, le_pconn);
 
-	result = (odbc_result *)emalloc(sizeof(odbc_result));
+	result = (odbc_result *)ecalloc(1, sizeof(odbc_result));
 	
 	rc = SQLAllocStmt(conn->hdbc, &(result->stmt));
 	if (rc == SQL_INVALID_HANDLE) {
@@ -3591,7 +3591,7 @@
 
 	ZEND_FETCH_RESOURCE2(conn, odbc_connection *, pv_conn, -1, "ODBC-Link", le_conn, le_pconn);
 
-	result = (odbc_result *)emalloc(sizeof(odbc_result));
+	result = (odbc_result *)ecalloc(1, sizeof(odbc_result));
 	
 	rc = SQLAllocStmt(conn->hdbc, &(result->stmt));
 	if (rc == SQL_INVALID_HANDLE) {

--------------030302000606050708070307
Content-Type: text/plain;
 name="php_odbc.c.patch2.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="php_odbc.c.patch2.txt"

Index: php_odbc.c
===================================================================
RCS file: /repository/php-src/ext/odbc/php_odbc.c,v
retrieving revision 1.178
diff -u -r1.178 php_odbc.c
--- php_odbc.c	18 Jun 2004 00:44:35 -0000	1.178
+++ php_odbc.c	26 Jul 2004 21:50:15 -0000
@@ -908,11 +908,10 @@
 	} else {
 		result->values = NULL;
 	}
-	result->id = zend_list_insert(result, le_result);	
 	zend_list_addref(conn->id);
 	result->conn_ptr = conn;
 	result->fetched = 0;
-	RETURN_RESOURCE(result->id);
+	ZEND_REGISTER_RESOURCE(return_value, result, le_result);
 }
 /* }}} */
 
@@ -1328,12 +1327,11 @@
 	} else {
 		result->values = NULL;
 	}
-	result->id = zend_list_insert(result, le_result);
 	zend_list_addref(conn->id);
 	result->conn_ptr = conn;
 	result->fetched = 0;
 	
-	RETURN_RESOURCE(result->id);
+    ZEND_REGISTER_RESOURCE(return_value, result, le_result);    
 }
 /* }}} */
 
@@ -2037,7 +2035,7 @@
 		result->values = NULL;
 	}
 			
-	zend_list_delete(result->id);
+    zend_list_delete(Z_LVAL_PP(pv_res));
 	
 	RETURN_TRUE;
 }

--------------030302000606050708070307
Content-Type: text/plain;
 name="php_odbc.c.patch3.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="php_odbc.c.patch3.txt"

Index: php_odbc.c
===================================================================
RCS file: /repository/php-src/ext/odbc/php_odbc.c,v
retrieving revision 1.178
diff -u -r1.178 php_odbc.c
--- php_odbc.c	18 Jun 2004 00:44:35 -0000	1.178
+++ php_odbc.c	26 Jul 2004 22:19:42 -0000
@@ -909,7 +909,6 @@
 		result->values = NULL;
 	}
 	result->id = zend_list_insert(result, le_result);	
-	zend_list_addref(conn->id);
 	result->conn_ptr = conn;
 	result->fetched = 0;
 	RETURN_RESOURCE(result->id);
@@ -1329,7 +1328,6 @@
 		result->values = NULL;
 	}
 	result->id = zend_list_insert(result, le_result);
-	zend_list_addref(conn->id);
 	result->conn_ptr = conn;
 	result->fetched = 0;
 	
@@ -2373,6 +2371,7 @@
 	}
 	
 	zend_list_delete(Z_LVAL_PP(pv_conn));
+	ZVAL_NULL(*pv_conn);
 	
 	if(is_pconn){
 		zend_hash_apply_with_argument(&EG(persistent_list),

--------------030302000606050708070307
Content-Type: text/plain;
 name="php_odbc_includes.h.patch2.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="php_odbc_includes.h.patch2.txt"

Index: php_odbc_includes.h
===================================================================
RCS file: /repository/php-src/ext/odbc/php_odbc_includes.h,v
retrieving revision 1.9
diff -u -r1.9 php_odbc_includes.h
--- php_odbc_includes.h	8 Jan 2004 17:32:34 -0000	1.9
+++ php_odbc_includes.h	27 Jul 2004 11:17:57 -0000
@@ -222,7 +222,6 @@
 
 typedef struct odbc_result {
 	ODBC_SQL_STMT_T stmt;
-	int id;
 	odbc_result_value *values;
 	SWORD numcols;
 	SWORD numparams;


--------------030302000606050708070307--