I have revised the patch for the race condition. New patch is much
less invasive and
scope of the change is restricted to pdo only.
Details are there in the bug.
Patch is attached.
Regards,
Basant.
On Tue, Oct 20, 2009 at 4:03 PM, Basant Kukreja
<basant.kukr...@gmail.com> wrote:
> 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)
@@ -2312,6 +2312,54 @@
return -1;
}
+static void init_stmt_properties(pdo_stmt_t* stmt TSRMLS_DC)
+{
+ HashTable* ht = &stmt->ce->default_properties;
+ HashTable* target = stmt->properties;
+
+ HashPosition pos;
+ zend_hash_internal_pointer_reset_ex(ht, &pos);
+ while(zend_hash_has_more_elements_ex(ht, &pos)
+ == SUCCESS) {
+ ulong index;
+ char* key = NULL;
+ uint keylen = 0;
+ int ret = zend_hash_get_current_key_ex(ht,
+
&key,
+
&keylen,
+
&index, 0,
+
&pos);
+ if ((keylen == sizeof("queryString"))
+ && (strncmp(key, "queryString", keylen) == 0)) {
+ zval* qval;
+ /* Since the value for the key queryString in
+ * stmt->ce->default_properties is shared by multiple
threads so
+ * we can not add the same zval in stmt->properties. we
need to
+ * create a null property object. See Bug 49937 */
+ ALLOC_INIT_ZVAL(qval);
+ zend_hash_add(stmt->properties, "queryString",
+ sizeof("queryString"),
(void**) &qval, sizeof(zval*), NULL);
+ }
+ else {
+ void* data = NULL;
+ zend_hash_get_current_data_ex(ht,
+
(void **) &data, &pos);
+ void *new_entry = NULL;
+ if (data) {
+ /* We expect keylen should be > 0.
default_properties hash
+ * should only contain named keys */
+ if (keylen) {
+ zend_hash_quick_update(target, key,
keylen, 0, data, sizeof(void*), &new_entry);
+ }
+ if (new_entry) {
+ zval_add_ref(new_entry);
+ }
+ }
+ }
+ zend_hash_move_forward_ex(ht, &pos);
+ }
+}
+
static zend_object_value dbstmt_clone_obj(zval *zobject TSRMLS_DC)
{
zend_object_value retval;
@@ -2325,7 +2373,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 *));
+ init_stmt_properties(stmt TSRMLS_CC);
old_stmt = (pdo_stmt_t *)zend_object_store_get_object(zobject
TSRMLS_CC);
@@ -2454,7 +2502,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 *));
+ init_stmt_properties(stmt TSRMLS_CC);
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;
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
