Hi,
    There is a race condition in pdo's
PDOStatement->ce.default_properties.ref_count. The integer
is incremented without any lock around it (or using any other atomic APIs).
This causes PDO to crash under stress. Details are given in bug report
http://bugs.php.net/bug.php?id=49937&thanks=1

I have attached the patch for review.

Note :
I could not find any easy locking mechanism available in php sources so needed
to use tsrm_mutex to implement atomic increments. It can be done very
efficiently in many modern OSes but for php 5.2.x, I didn't want to introduce
many changes.
Index: ext/pdo/pdo_stmt.c
===================================================================
--- ext/pdo/pdo_stmt.c  (revision 289806)
+++ ext/pdo/pdo_stmt.c  (working copy)
@@ -2325,7 +2325,7 @@
        stmt->refcount = 1;
        ALLOC_HASHTABLE(stmt->properties);
        zend_hash_init(stmt->properties, 0, NULL, ZVAL_PTR_DTOR, 0);
-       zend_hash_copy(stmt->properties, &stmt->ce->default_properties, 
(copy_ctor_func_t) zval_add_ref, (void *) &tmp, sizeof(zval *));
+       zend_hash_copy(stmt->properties, &stmt->ce->default_properties, 
(copy_ctor_func_t) zval_add_ref_atomic, (void *) &tmp, sizeof(zval *));
 
        old_stmt = (pdo_stmt_t *)zend_object_store_get_object(zobject 
TSRMLS_CC);
        
@@ -2454,7 +2454,7 @@
        stmt->refcount = 1;
        ALLOC_HASHTABLE(stmt->properties);
        zend_hash_init(stmt->properties, 0, NULL, ZVAL_PTR_DTOR, 0);
-       zend_hash_copy(stmt->properties, &ce->default_properties, 
(copy_ctor_func_t) zval_add_ref, (void *) &tmp, sizeof(zval *));
+       zend_hash_copy(stmt->properties, &ce->default_properties, 
(copy_ctor_func_t) zval_add_ref_atomic, (void *) &tmp, sizeof(zval *));
 
        retval.handle = zend_objects_store_put(stmt, 
(zend_objects_store_dtor_t)zend_objects_destroy_object, 
(zend_objects_free_object_storage_t)pdo_dbstmt_free_storage, 
(zend_objects_store_clone_t)dbstmt_clone_obj TSRMLS_CC);
        retval.handlers = &pdo_dbstmt_object_handlers;
Index: TSRM/TSRM.c
===================================================================
--- TSRM/TSRM.c (revision 289806)
+++ TSRM/TSRM.c (working copy)
@@ -714,6 +714,12 @@
        return retval;
 }
 
+TSRM_API void *tsrm_atomic_incr(volatile unsigned int* val)
+{
+       tsrm_mutex_lock(tsmm_mutex);
+       ++*val;
+       tsrm_mutex_unlock(tsmm_mutex);
+}
 
 
 /*
Index: TSRM/TSRM.h
===================================================================
--- TSRM/TSRM.h (revision 289806)
+++ TSRM/TSRM.h (working copy)
@@ -139,6 +139,7 @@
 
 TSRM_API void *tsrm_set_new_thread_begin_handler(tsrm_thread_begin_func_t 
new_thread_begin_handler);
 TSRM_API void *tsrm_set_new_thread_end_handler(tsrm_thread_end_func_t 
new_thread_end_handler);
+TSRM_API void *tsrm_atomic_incr(volatile unsigned int* val);
 
 /* these 3 APIs should only be used by people that fully understand the 
threading model
  * used by PHP/Zend and the selected SAPI. */
Index: Zend/zend_variables.c
===================================================================
--- Zend/zend_variables.c       (revision 289806)
+++ Zend/zend_variables.c       (working copy)
@@ -100,6 +100,17 @@
 }
 /* }}} */
 
+
+ZEND_API void zval_add_ref_atomic(zval **p) /* {{{ */
+{
+#ifdef ZTS
+       tsrm_atomic_incr(&(*p)->refcount);
+#else
+       (*p)->refcount++;
+#endif
+}
+/* }}} */
+
 ZEND_API void _zval_copy_ctor_func(zval *zvalue ZEND_FILE_LINE_DC) /* {{{ */
 {
        switch (zvalue->type) {
Index: Zend/zend_variables.h
===================================================================
--- Zend/zend_variables.h       (revision 289806)
+++ Zend/zend_variables.h       (working copy)
@@ -76,6 +76,7 @@
 #endif
 
 ZEND_API void zval_add_ref(zval **p);
+ZEND_API void zval_add_ref_atomic(zval **p);
 
 END_EXTERN_C()
 
-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to