[Boost-commit] svn:boost r56284 - sandbox/stm/libs/stm/src

Subject: [Boost-commit] svn:boost r56284 - sandbox/stm/libs/stm/src
From: justin_at_[hidden]
Date: 2009-09-18 01:47:04

Author: jgottschlich
Date: 2009-09-18 01:47:04 EDT (Fri, 18 Sep 2009)
New Revision: 56284
URL: http://svn.boost.org/trac/boost/changeset/56284

Log:
Fixed bug in initialize_thread found by Aruthur Athrun. Many thanks to Arthur!

Text files modified: 
   sandbox/stm/libs/stm/src/transaction.cpp |    96 +++++++++++++++++++++++++++++++-------- 
   1 files changed, 76 insertions(+), 20 deletions(-)

Modified: sandbox/stm/libs/stm/src/transaction.cpp
==============================================================================
--- sandbox/stm/libs/stm/src/transaction.cpp	(original)
+++ sandbox/stm/libs/stm/src/transaction.cpp	2009-09-18 01:47:04 EDT (Fri, 18 Sep 2009)
@@ -11,25 +11,6 @@
 //
 //////////////////////////////////////////////////////////////////////////////
 
-/* The DRACO Research Group (rogue.colorado.edu/draco) */ 
-/*****************************************************************************\
- *
- * Copyright Notices/Identification of Licensor(s) of
- * Original Software in the File
- *
- * Copyright 2000-2006 The Board of Trustees of the University of Colorado
- * Contact: Technology Transfer Office,
- * University of Colorado at Boulder;
- * https://www.cu.edu/techtransfer/
- *
- * All rights reserved by the foregoing, respectively.
- *
- * This is licensed software.  The software license agreement with
- * the University of Colorado specifies the terms and conditions
- * for use and redistribution.
- *
-\*****************************************************************************/
-
 #include <boost/stm/transaction.hpp>
 #include <boost/stm/contention_manager.hpp>
 #include <iostream>
@@ -160,6 +141,42 @@
 {
    lock_general_access();
 
+   //--------------------------------------------------------------------------
+   // WARNING: before you think lock_all_mutexes() does not make sense, make
+   //          sure you read the following example, which will certainly change
+   //          your mind about what you think you know ...
+   //
+   //          end_transaction() must lock all mutexes() in addition to the
+   //          important general access mutex, which serializes commits.
+   //
+   //          In order to make end_transaction as efficient as possible, we
+   //          must release general_access() before we release the specific
+   //          threaded mutexes. Unfortunately, because of this, a thread can 
+   //          can enter this function and add a new thread (and mutex) to the
+   //          mutex list. Then end_transaction() can finish its execution and
+   //          unlock all mutexes. The problem is that between end_transaction
+   //          and this function, any number of operations can be performed.
+   //          One of those operations may lock the mutex of the new thread,
+   //          which may then be unlocked by end_transaction. If that happens, 
+   //          all kinds of inconsistencies could occur ...
+   //
+   //          In order to fix this, we could change the unlock order of 
+   //          end_transaction() so it unlocks all mutexes before releasing the
+   //          the general mutex. The problem with that is end_transaction is
+   //          a high serialization point and the general mutex is the most
+   //          contended upon lock. As such, it is not wise to prolong its
+   //          release. Instead, we can change this method, so it locks all the
+   //          thread's mutexes. This ensures that this method cannot be entered
+   //          until end_transaction() completes, guaranteeing that
+   //          end_transaction() cannot unlock mutexes it doesn't own.
+   //
+   //          Questions? Contact Justin Gottschlich or Vicente Botet.
+   //
+   //          DO NOT REMOVE LOCK_ALL_MUTEXES / UNLOCK_ALL_MUTEXES!!
+   //
+   //--------------------------------------------------------------------------
+   lock_all_mutexes();
+
    size_t threadId = THREAD_ID;
 
 #ifndef USE_SINGLE_THREAD_CONTEXT_MAP
@@ -313,6 +330,46 @@
    
 #endif
 
+   //--------------------------------------------------------------------------
+   // WARNING: before you think unlock_all_mutexes() does not make sense, make
+   //          sure you read the following example, which will certainly change
+   //          your mind about what you think you know ...
+   //
+   //          end_transaction() must lock all mutexes() in addition to the
+   //          important general access mutex, which serializes commits.
+   //
+   //          In order to make end_transaction as efficient as possible, we
+   //          must release general_access() before we release the specific
+   //          threaded mutexes. Unfortunately, because of this, a thread can 
+   //          can enter this function and add a new thread (and mutex) to the
+   //          mutex list. Then end_transaction() can finish its execution and
+   //          unlock all mutexes. The problem is that between end_transaction
+   //          and this function, any number of operations can be performed.
+   //          One of those operations may lock the mutex of the new thread,
+   //          which may then be unlocked by end_transaction. If that happens, 
+   //          all kinds of inconsistencies could occur ...
+   //
+   //          In order to fix this, we could change the unlock order of 
+   //          end_transaction() so it unlocks all mutexes before releasing the
+   //          the general mutex. The problem with that is end_transaction is
+   //          a high serialization point and the general mutex is the most
+   //          contended upon lock. As such, it is not wise to prolong its
+   //          release. Instead, we can change this method, so it locks all the
+   //          thread's mutexes. This ensures that this method cannot be entered
+   //          until end_transaction() completes, guaranteeing that
+   //          end_transaction() cannot unlock mutexes it doesn't own.
+   //
+   //          Questions? Contact Justin Gottschlich or Vicente Botet.
+   //
+   //          DO NOT REMOVE LOCK_ALL_MUTEXES / UNLOCK_ALL_MUTEXES!!
+   //
+   //--------------------------------------------------------------------------
+   // this will unlock the mutex of the thread that was just added, but it
+   // doesn't matter because that mutex will already be unlocked
+   //--------------------------------------------------------------------------
+   unlock_all_mutexes();
+   //--------------------------------------------------------------------------
+
    unlock_general_access();
 }
 
@@ -418,7 +475,6 @@
    }
 #endif
       
-
    unlock_inflight_access();
    unlock_general_access();
 }