]> 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

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]

index e78ec00119d85628e5d0acfb57fc8a84f45822b9..2b60f0a3995a11c8d5a4fd92181da056f0ff641b 100644 (file)
@@ -1368,24 +1368,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()
  *
  * Fetch & print out error message if we get an error
@@ -2111,9 +2108,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 9cea077cb5dd6d71615bc2e34ed80bd760f1b2ac..7d7963b6a12cdc658fa17cb16e4bb6f486fe3ddb 100644 (file)
@@ -1106,9 +1106,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)) {
@@ -1132,9 +1130,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 e210aae6bee799f7c2147c014986c68f84967eb0..540d6d96864f4a7677f2473a8b5769b62d26c148 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;
 
@@ -672,7 +680,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 b5ef31c4515b8175d2adb4c1dadfc062e4ce1527..04614fe406dab85c7acf05c9993426a64871fdfd 100644 (file)
@@ -95,6 +95,7 @@ php_oci_statement *php_oci_statement_create (php_oci_connection *connection, zst
 
        statement->connection = connection;
        statement->has_data = 0;
+       statement->has_descr = 0;
        statement->parent_stmtid = 0;
        zend_list_addref(statement->connection->rsrc_id);
 
@@ -133,6 +134,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)
@@ -145,6 +180,10 @@ int php_oci_statement_fetch(php_oci_statement *statement, ub4 nrows TSRMLS_DC)
        php_oci_out_column *column;
        zstr zstr_data;
 
+       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 ) {
@@ -577,6 +616,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;
@@ -590,6 +630,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 5b4c7efb0a448cea42bf3642c6aea92b2073808a..8c5045c9bd30064fbf6a8cad4f6e4bef404593f0 100644 (file)
@@ -136,6 +136,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 */
@@ -153,6 +154,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) */
@@ -206,6 +208,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; /* }}} */
 
@@ -377,7 +380,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);
@@ -463,6 +467,7 @@ int php_oci_bind_by_name(php_oci_statement *, zstr, int, zval*, long, ub2, zend_
 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===