Race condition in PDOStatement (patch)

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()