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