]> granicus.if.org Git - php/commitdiff
Fixed bug #49560 (oci8: using LOBs causes slow PHP shutdown)
authorChristopher Jones <sixd@php.net>
Wed, 6 Jan 2010 18:58:16 +0000 (18:58 +0000)
committerChristopher Jones <sixd@php.net>
Wed, 6 Jan 2010 18:58:16 +0000 (18:58 +0000)
 - Improved descriptor refcounting to remove unneeded items sooner
 - Replaced n^2 list traversal during descriptor list destruction

NEWS
ext/oci8/oci8.c
ext/oci8/oci8_interface.c
ext/oci8/oci8_lob.c
ext/oci8/oci8_statement.c
ext/oci8/php_oci8_int.h
ext/oci8/tests/bug43497.phpt
ext/oci8/tests/lob_043.phpt [new file with mode: 0644]

diff --git a/NEWS b/NEWS
index 8566496367c1b9a52bf09614f09e5ac3eb855893..3d6d5f56eeb6a461d6002bccdc17d91a74396e0a 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,7 @@ PHP                                                                        NEWS
 - Fixed bug #50632 (filter_input() does not return default value if the
   variable does not exist). (Ilia)
 - Fixed bug #50576 (XML_OPTION_SKIP_TAGSTART option has no effect). (Pierrick)
+- Fixed bug #49560 (oci8: using LOBs causes slow PHP shutdown). (Oracle Corp.)
 - Fixed bug #48590 (SoapClient does not honor max_redirects). (Sriram)
 - Fixed bug #48190 (Content-type parameter "boundary" is not case-insensitive
   in HTTP uploads). (Ilia)
@@ -23,7 +24,6 @@ PHP                                                                        NEWS
   (Ilia, chrisstocktonaz at gmail dot com)
 - Fixed bug #44827 (define() allows :: in constant names). (Ilia)
 
-
 ?? ??? 20??, PHP 5.3.2
 - Upgraded bundled sqlite to version 3.6.21. (Ilia)
 - Upgraded bundled PCRE to version 8.00. (Scott)
index 45beb32b4a7949edfcd76a7b8ba53977a9f80b5e..4a47ecae95af2e9cd63bcf631a28b81759c58f3e 100644 (file)
@@ -1530,23 +1530,21 @@ void php_oci_descriptor_flush_hash_dtor(void *data)
 }
 /* }}} */
 
-/* {{{ php_oci_descriptor_delete_from_hash()
+/* }}} */
+
+/* {{{ php_oci_connection_descriptors_free()
  *
- * Delete descriptor from the hash
+ * Free descriptors for a connection
  */
-int php_oci_descriptor_delete_from_hash(void *data, void *id TSRMLS_DC)
+void php_oci_connection_descriptors_free(php_oci_connection *connection TSRMLS_DC)
 {
-       php_oci_descriptor *descriptor = *(php_oci_descriptor **)data;
-       int *desc_id = (int *) id;
-
-       if (descriptor && desc_id && descriptor->id == *desc_id) {
-               return 1;
-       }
-       return 0;
+       zend_hash_destroy(connection->descriptors);
+       efree(connection->descriptors);
+       connection->descriptors = NULL;
+       connection->descriptor_count = 0;
 }
 /* }}} */
 
-/* }}} */
 
 /* {{{ php_oci_error()
  *
@@ -2271,9 +2269,7 @@ int php_oci_connection_release(php_oci_connection *connection TSRMLS_DC)
        }
 
        if (connection->descriptors) {
-               zend_hash_destroy(connection->descriptors);
-               efree(connection->descriptors);
-               connection->descriptors = NULL;
+               php_oci_connection_descriptors_free(connection TSRMLS_CC);
        }
 
        if (connection->svc) {
index 979ac0368c6bca8f56409eeab95e3289f241d718..b12c8dc3bc1b3f2271f5d5a9dc3a16960ee79852 100644 (file)
@@ -1066,9 +1066,7 @@ PHP_FUNCTION(oci_rollback)
        PHP_OCI_ZVAL_TO_CONNECTION(z_connection, connection);
 
        if (connection->descriptors) {
-               zend_hash_destroy(connection->descriptors);
-               efree(connection->descriptors);
-               connection->descriptors = NULL;
+               php_oci_connection_descriptors_free(connection TSRMLS_CC);
        }
 
        if (php_oci_connection_rollback(connection TSRMLS_CC)) {
@@ -1092,9 +1090,7 @@ PHP_FUNCTION(oci_commit)
        PHP_OCI_ZVAL_TO_CONNECTION(z_connection, connection);
 
        if (connection->descriptors) {
-               zend_hash_destroy(connection->descriptors);
-               efree(connection->descriptors);
-               connection->descriptors = NULL;
+               php_oci_connection_descriptors_free(connection TSRMLS_CC);
        }
        
        if (php_oci_connection_commit(connection TSRMLS_CC)) {
index f232963a201397a4f3c6fe775a894a0b92a9ce98..2b87dbb3d2282a15c0b2ca35a97aee1e406b2417 100644 (file)
@@ -95,9 +95,17 @@ php_oci_descriptor *php_oci_lob_create (php_oci_connection *connection, long typ
                if (!connection->descriptors) {
                        ALLOC_HASHTABLE(connection->descriptors);
                        zend_hash_init(connection->descriptors, 0, NULL, php_oci_descriptor_flush_hash_dtor, 0);
+                       connection->descriptor_count = 0;
+               }
+               
+               descriptor->index = (connection->descriptor_count)++;
+               if (connection->descriptor_count == LONG_MAX) {
+                       php_error_docref(NULL TSRMLS_CC, E_WARNING, "Internal descriptor counter has reached limit");
+                       php_oci_connection_descriptors_free(connection TSRMLS_CC);
+                       return NULL;
                }
 
-               zend_hash_next_index_insert(connection->descriptors,&descriptor,sizeof(php_oci_descriptor *),NULL);
+               zend_hash_index_update(connection->descriptors,descriptor->index,&descriptor,sizeof(php_oci_descriptor *),NULL);
        }
        return descriptor;
 
@@ -669,7 +677,25 @@ void php_oci_lob_free (php_oci_descriptor *descriptor TSRMLS_DC)
 
        if (descriptor->connection->descriptors) {
                /* delete descriptor from the hash */
-               zend_hash_apply_with_argument(descriptor->connection->descriptors, php_oci_descriptor_delete_from_hash, (void *)&descriptor->id TSRMLS_CC);
+               zend_hash_index_del(descriptor->connection->descriptors, descriptor->index);
+               if (zend_hash_num_elements(descriptor->connection->descriptors) == 0) {
+                       descriptor->connection->descriptor_count = 0;
+               } else {
+                       if (descriptor->index + 1 == descriptor->connection->descriptor_count) {
+                               /* If the descriptor being freed is the end-most one
+                                * allocated, then the descriptor_count is reduced so
+                                * a future descriptor can reuse the hash table index.
+                                * This can prevent the hash index range increasing in
+                                * the common case that each descriptor is
+                                * allocated/used/freed before another descriptor is
+                                * needed.  However it is possible that a script frees
+                                * descriptors in arbitrary order which would prevent
+                                * descriptor_count ever being reduced to zero until
+                                * zend_hash_num_elements() returns 0.
+                                */
+                               descriptor->connection->descriptor_count--;
+                       }
+               }
        }
        
        /* flushing Lobs & Files with buffering enabled */
index 3ca43b60bfa2a182f6780154a13f7de9e12c5837..2cbc2847733390b6de63ca57c91536f5cc6da78e 100644 (file)
@@ -93,6 +93,7 @@ php_oci_statement *php_oci_statement_create (php_oci_connection *connection, cha
 
        statement->connection = connection;
        statement->has_data = 0;
+       statement->has_descr = 0;
        statement->parent_stmtid = 0;
        zend_list_addref(statement->connection->rsrc_id);
 
@@ -131,6 +132,40 @@ int php_oci_statement_set_prefetch(php_oci_statement *statement, long size TSRML
 }
 /* }}} */
 
+/* {{{ php_oci_cleanup_pre_fetch()
+   Helper function to cleanup ref-cursors and descriptors from the previous row */
+int php_oci_cleanup_pre_fetch(void *data TSRMLS_DC)
+{
+       php_oci_out_column *outcol = data;
+
+       if (!outcol->is_descr && !outcol->is_cursor)
+               return ZEND_HASH_APPLY_KEEP;
+
+       switch(outcol->data_type) {
+               case SQLT_CLOB:
+               case SQLT_BLOB:
+               case SQLT_RDD:
+               case SQLT_BFILE:
+                       if (outcol->descid) {
+                               zend_list_delete(outcol->descid);
+                               outcol->descid = 0;
+                       }
+                       break;
+               case SQLT_RSET:
+                       if (outcol->stmtid) {
+                               zend_list_delete(outcol->stmtid);
+                               outcol->stmtid = 0;
+                               outcol->nested_statement = NULL;
+                       }
+                       break;
+               default:
+                       break;
+       }
+       return ZEND_HASH_APPLY_KEEP;
+
+} /* }}} */
+
+
 /* {{{ php_oci_statement_fetch()
  Fetch a row from the statement */
 int php_oci_statement_fetch(php_oci_statement *statement, ub4 nrows TSRMLS_DC)
@@ -143,6 +178,10 @@ int php_oci_statement_fetch(php_oci_statement *statement, ub4 nrows TSRMLS_DC)
 
        php_oci_out_column *column;
 
+       if (statement->has_descr && statement->columns) {
+               zend_hash_apply(statement->columns, (apply_func_t) php_oci_cleanup_pre_fetch TSRMLS_CC);
+    }
+
        PHP_OCI_CALL_RETURN(statement->errcode, OCIStmtFetch, (statement->stmt, statement->err, nrows, OCI_FETCH_NEXT, OCI_DEFAULT));
 
        if ( statement->errcode == OCI_NO_DATA || nrows == 0 ) {
@@ -570,6 +609,7 @@ int php_oci_statement_execute(php_oci_statement *statement, ub4 mode TSRMLS_DC)
 
                                        define_type = SQLT_RSET;
                                        outcol->is_cursor = 1;
+                                       outcol->statement->has_descr = 1;
                                        outcol->storage_size4 = -1;
                                        outcol->retlen = -1;
                                        dynamic = OCI_DYNAMIC_FETCH;
@@ -583,6 +623,7 @@ int php_oci_statement_execute(php_oci_statement *statement, ub4 mode TSRMLS_DC)
 
                                        define_type = outcol->data_type;
                                        outcol->is_descr = 1;
+                                       outcol->statement->has_descr = 1;
                                        outcol->storage_size4 = -1;
                                        dynamic = OCI_DYNAMIC_FETCH;
                                        break;
index 01a5d22b8f921f97822a363d40d4f57bbf947191..c1f8cb8b03b57aea84545dcc1c9e575e0b65fc83 100644 (file)
@@ -130,6 +130,7 @@ typedef struct { /* php_oci_connection {{{ */
        sword                   errcode;                                        /* last errcode */
 
        HashTable          *descriptors;                                /* descriptors hash, used to flush all the LOBs using this connection on commit */
+       ulong                   descriptor_count;                       /* used to index the descriptors hash table.  Not an accurate count */
        unsigned                is_open:1;                                      /* hels to determine if the connection is dead or not */
        unsigned                is_attached:1;                          /* hels to determine if we should detach from the server when closing/freeing the connection */
        unsigned                is_persistent:1;                        /* self-descriptive */
@@ -146,6 +147,7 @@ typedef struct { /* php_oci_connection {{{ */
 
 typedef struct { /* php_oci_descriptor {{{ */
        int                                      id;
+       ulong                            index;                     /* descriptors hash table index */
        php_oci_connection      *connection;                    /* parent connection handle */
        dvoid                           *descriptor;                    /* OCI descriptor handle */
        ub4                                      type;                                  /* descriptor type (FILE/LOB) */
@@ -197,6 +199,7 @@ typedef struct { /* php_oci_statement {{{ */
        int                                      ncolumns;                              /* number of columns in the result */
        unsigned                         executed:1;                    /* statement executed flag */
        unsigned                         has_data:1;                    /* statement has more data flag */
+       unsigned                         has_descr:1;                   /* statement has at least one descriptor or cursor column */
        ub2                                      stmttype;                              /* statement type */
 } php_oci_statement; /* }}} */
 
@@ -367,7 +370,8 @@ void php_oci_column_hash_dtor (void *data);
 void php_oci_define_hash_dtor (void *data);
 void php_oci_bind_hash_dtor (void *data);
 void php_oci_descriptor_flush_hash_dtor (void *data);
-int php_oci_descriptor_delete_from_hash(void *data, void *id TSRMLS_DC);
+
+void php_oci_connection_descriptors_free(php_oci_connection *connection TSRMLS_DC);
 
 sb4 php_oci_error (OCIError *, sword TSRMLS_DC);
 sb4 php_oci_fetch_errmsg(OCIError *, text ** TSRMLS_DC);
@@ -452,6 +456,7 @@ int php_oci_bind_by_name(php_oci_statement *, char *, int, zval*, long, ub2 TSRM
 sb4 php_oci_bind_in_callback(dvoid *, OCIBind *, ub4, ub4, dvoid **, ub4 *, ub1 *, dvoid **);
 sb4 php_oci_bind_out_callback(dvoid *, OCIBind *, ub4, ub4, dvoid **, ub4 **, ub1 *, dvoid **, ub2 **);
 php_oci_out_column *php_oci_statement_get_column_helper(INTERNAL_FUNCTION_PARAMETERS, int need_data);
+int php_oci_cleanup_pre_fetch(void *data TSRMLS_DC);
 
 int php_oci_statement_get_type(php_oci_statement *, ub2 * TSRMLS_DC);
 int php_oci_statement_get_numrows(php_oci_statement *, ub4 * TSRMLS_DC);
index 0fc6a97b3531470fc3559ca7bc1454c4941afc7e..1ea46b40d7b0713e8b06d89dfde824339bc9d328 100644 (file)
@@ -157,7 +157,7 @@ for ($i = 1; $i <= 10; $i++) {
     readxmltab_im($c);
 }
 
-echo "\nExplicit LOB with no free (i.e. a temp lob leak)\n";
+echo "\nExplicit LOB with no free\n";
 for ($i = 1; $i <= 10; $i++) {
     echo "\nRun              = " . $i . "\n";
     echo "Temporary LOBs   = " . templobs($c, $sid) . "\n";
@@ -259,45 +259,45 @@ Run              = 10
 Temporary LOBs   = 0
 Loop count check = 100
 
-Explicit LOB with no free (i.e. a temp lob leak)
+Explicit LOB with no free
 
 Run              = 1
 Temporary LOBs   = 0
 Loop count check = 100
 
 Run              = 2
-Temporary LOBs   = 99
+Temporary LOBs   = 0
 Loop count check = 100
 
 Run              = 3
-Temporary LOBs   = 198
+Temporary LOBs   = 0
 Loop count check = 100
 
 Run              = 4
-Temporary LOBs   = 297
+Temporary LOBs   = 0
 Loop count check = 100
 
 Run              = 5
-Temporary LOBs   = 396
+Temporary LOBs   = 0
 Loop count check = 100
 
 Run              = 6
-Temporary LOBs   = 495
+Temporary LOBs   = 0
 Loop count check = 100
 
 Run              = 7
-Temporary LOBs   = 594
+Temporary LOBs   = 0
 Loop count check = 100
 
 Run              = 8
-Temporary LOBs   = 693
+Temporary LOBs   = 0
 Loop count check = 100
 
 Run              = 9
-Temporary LOBs   = 792
+Temporary LOBs   = 0
 Loop count check = 100
 
 Run              = 10
-Temporary LOBs   = 891
+Temporary LOBs   = 0
 Loop count check = 100
 Done
diff --git a/ext/oci8/tests/lob_043.phpt b/ext/oci8/tests/lob_043.phpt
new file mode 100644 (file)
index 0000000..5ae2d45
--- /dev/null
@@ -0,0 +1,101 @@
+--TEST--
+Bug #49560 (LOB resource destructor and refcount test)
+--SKIPIF--
+<?php if (!extension_loaded('oci8')) die ("skip no oci8 extension"); ?>
+--FILE--
+<?php
+
+require(dirname(__FILE__).'/connect.inc');
+
+// Initialization
+
+$stmtarray = array(
+       "drop table lob_043_tab",
+       "create table lob_043_tab(id number, c1 clob)",
+       "begin 
+       for i in 1..50000 loop
+         insert into lob_043_tab (id, c1) values (i, i || ' abcdefghijklmnopq');
+      end loop;
+     end;",
+);
+
+foreach ($stmtarray as $stmt) {
+       $s = oci_parse($c, $stmt);
+       $r = @oci_execute($s);
+       if (!$r) {
+               $m = oci_error($s);
+               if (!in_array($m['code'], array(   // ignore expected errors
+                           942 // table or view does not exist
+                       ,  2289 // sequence does not exist
+                       ,  4080 // trigger does not exist
+                        , 38802 // edition does not exist
+                ))) {
+                       echo $stmt . PHP_EOL . $m['message'] . PHP_EOL;
+               }
+       }
+}
+
+// Run Test
+
+function f1($c)
+{
+    $s = oci_parse($c, 'select id, c1 from lob_043_tab order by id');
+    oci_execute($s);
+    $r = array();
+    while (($row = oci_fetch_array($s, OCI_RETURN_NULLS+OCI_ASSOC+OCI_RETURN_LOBS)) !== false) {
+        $r[] = $row['C1'];
+    }
+    echo "f1 ended\n";
+    return $r;
+}
+
+function f2($c)
+{
+    $s = oci_parse($c, 'select id, c1 from lob_043_tab order by id');
+    oci_execute($s);
+    $r = array();
+    while (($row = oci_fetch_array($s, OCI_RETURN_NULLS+OCI_ASSOC)) !== false) {
+        $r[] = $row['C1'];
+    }
+    echo "f2 ended\n";
+    return $r;
+}
+
+echo "Test 1\n";
+$r = f1($c);
+/*
+  foreach ($r as $v) {
+  echo $v, "\n";
+  }
+*/
+
+echo "Test 2\n";
+$r = f2($c);
+/*
+  foreach ($r as $v) {
+  echo $v->load(), "\n";
+  }
+*/
+
+// Clean up
+
+$stmtarray = array(
+       "drop table lob_043_tab"
+);
+
+foreach ($stmtarray as $stmt) {
+       $s = oci_parse($c, $stmt);
+       oci_execute($s);
+}
+
+oci_close($c);
+
+?>
+===DONE===
+<?php exit(0); ?>
+--EXPECTF--
+Test 1
+f1 ended
+Test 2
+f2 ended
+===DONE===