https://bugs.openldap.org/show_bug.cgi?id=10395

--- Comment #10 from Howard Chu <[email protected]> ---

+   /** The count of nested RDONLY txns under this txn also named child txns */
+   unsigned int    mt_rdonly_child_count;

I don't understand the "also named child txns" part of this comment.

+       txn->mt_rdonly_child_count = 0;
+#ifdef _WIN32
+       txn->mt_child_mutex = CreateMutex(NULL, FALSE, NULL);
+#else
+       pthread_mutex_init(&txn->mt_child_mutex, NULL);
+#endif


All of the platform-specific thread/mutex stuff should be #defined as macros in
the header block, e.g. around mdb.c:320. There should be no thread/mutex ifdefs
anywhere else in the code.

+       /* Nested transactions:
+        * If RDONLY: Any number of children, writemap allowed
+        * If write: Max 1 child, no writemap

These comments are no longer unambiguous.

The meaning before: only write txns may have nested txns, the nested txns may
only be write txns, and there may only be 1.

The meaning now should be: Only write txns may have nested txns; if the nested
txn is a write txn there may only be 1; if the nested txn is a read txn there
may be arbitrarily many.

+       /* Not useful when nested RDONLY but correctly freed in mdb_txn_end */

These allocations should be skipped for RDONLY txns, especially since there may
be arbitrarily many of them.

+       if (F_ISSET(flags, MDB_RDONLY)) {
+           pthread_mutex_lock(&parent->mt_child_mutex);
+           parent->mt_rdonly_child_count++;
+           pthread_mutex_unlock(&parent->mt_child_mutex);
+       } else {
+           pthread_mutex_lock(&parent->mt_child_mutex);
+           parent->mt_rdonly_child_count = 0;
+           pthread_mutex_unlock(&parent->mt_child_mutex);
+       }

The else clause should be unnecessary; if your counting has been correct then
it should already be zero. If it's not correct then you have a bug.

+   if (!txn->mt_parent && F_ISSET(txn->mt_flags, MDB_TXN_RDONLY)) {

This appears to only be correct if you used MDB_NOTLS.

+               env->me_pgstate = ((MDB_ntxn *)txn)->mnt_pgstate;

pgstate should only exist in a writable child txn. In a read txn it should be
unused, what is this value then?

-- 
You are receiving this mail because:
You are on the CC list for the issue.

Reply via email to