]> granicus.if.org Git - icu/commitdiff
ICU-11603 BreakTransliterator thread safety changes, use LocalPointer and Mutex classes.
authorAndy Heninger <andy.heninger@gmail.com>
Wed, 29 Apr 2015 23:18:32 +0000 (23:18 +0000)
committerAndy Heninger <andy.heninger@gmail.com>
Wed, 29 Apr 2015 23:18:32 +0000 (23:18 +0000)
X-SVN-Rev: 37414

icu4c/source/i18n/brktrans.cpp
icu4c/source/i18n/brktrans.h
icu4c/source/i18n/rbt.cpp

index e59cef38b6f688808f92f46da963fdcc9c8bbf7e..10505494fd4b2bab95a319ffe1b8f9ed40a3f062 100644 (file)
 
 #if  !UCONFIG_NO_TRANSLITERATION && !UCONFIG_NO_BREAK_ITERATION
 
-#include "unicode/unifilt.h"
+#include "unicode/brkiter.h"
+#include "unicode/localpointer.h"
 #include "unicode/uchar.h"
+#include "unicode/unifilt.h"
 #include "unicode/uniset.h"
-#include "unicode/brkiter.h"
+
 #include "brktrans.h"
-#include "unicode/uchar.h"
 #include "cmemory.h"
+#include "mutex.h"
 #include "uprops.h"
 #include "uinvchar.h"
-#include "umutex.h"
 #include "util.h"
 #include "uvectr32.h"
 
@@ -37,10 +38,8 @@ static const UChar SPACE       = 32;  // ' '
  * '}'.
  */
 BreakTransliterator::BreakTransliterator(UnicodeFilter* adoptedFilter) :
-    Transliterator(UNICODE_STRING("Any-BreakInternal", 17), adoptedFilter),
-    fInsertion(SPACE) {
-        cachedBI = NULL;
-        cachedBoundaries = NULL;
+        Transliterator(UNICODE_STRING("Any-BreakInternal", 17), adoptedFilter),
+        cachedBI(NULL), cachedBoundaries(NULL), fInsertion(SPACE) {
     }
 
 
@@ -48,20 +47,13 @@ BreakTransliterator::BreakTransliterator(UnicodeFilter* adoptedFilter) :
  * Destructor.
  */
 BreakTransliterator::~BreakTransliterator() {
-    delete cachedBI;
-    cachedBI = NULL;
-    delete cachedBoundaries;
-    cachedBoundaries = NULL;
 }
 
 /**
  * Copy constructor.
  */
 BreakTransliterator::BreakTransliterator(const BreakTransliterator& o) :
-        Transliterator(o) {
-    cachedBI = NULL;
-    cachedBoundaries = NULL;
-    fInsertion = o.fInsertion;
+        Transliterator(o), cachedBI(NULL), cachedBoundaries(NULL), fInsertion(o.fInsertion) {
 }
 
 
@@ -79,27 +71,26 @@ void BreakTransliterator::handleTransliterate(Replaceable& text, UTransPosition&
                                                     UBool isIncremental ) const {
 
         UErrorCode status = U_ZERO_ERROR;
-        BreakIterator *bi = NULL;
-        UVector32 *boundaries = NULL;
+        LocalPointer<BreakIterator> bi;
+        LocalPointer<UVector32> boundaries;
 
-        umtx_lock(NULL);
-        if (cachedBI) {
-            bi = cachedBI;
-            boundaries = cachedBoundaries;
+        {
+            Mutex m;
             BreakTransliterator *nonConstThis = const_cast<BreakTransliterator *>(this);
-            nonConstThis->cachedBI = NULL;
-            nonConstThis->cachedBoundaries = NULL;
+            boundaries.adoptInstead(nonConstThis->cachedBoundaries.orphan());
+            bi.adoptInstead(nonConstThis->cachedBI.orphan());
+        }
+        if (bi.isNull()) {
+            bi.adoptInstead(BreakIterator::createWordInstance(Locale::getEnglish(), status));
+        }
+        if (boundaries.isNull()) {
+            boundaries.adoptInstead(new UVector32(status));
         }
-        umtx_unlock(NULL);
-        if (bi == NULL) {
-            boundaries = new UVector32(status);
-            bi = BreakIterator::createWordInstance(Locale::getEnglish(), status);
-        }    
 
-        if (bi == NULL || boundaries == NULL || U_FAILURE(status)) {
+        if (bi.isNull() || boundaries.isNull() || U_FAILURE(status)) {
             return;
         }
-            
+
         boundaries->removeAllElements();
         UnicodeString sText = replaceableAsString(text);
         bi->setText(sText);
@@ -148,22 +139,15 @@ void BreakTransliterator::handleTransliterate(Replaceable& text, UTransPosition&
         offsets.start = isIncremental ? lastBoundary + delta : offsets.limit;
 
         // Return break iterator & boundaries vector to the cache.
-        umtx_lock(NULL);
-        BreakTransliterator *nonConstThis = const_cast<BreakTransliterator *>(this);
-        if (nonConstThis->cachedBI == NULL) {
-            nonConstThis->cachedBI = bi;
-            bi = NULL;
-        }
-        if (nonConstThis->cachedBoundaries == NULL) {
-            nonConstThis->cachedBoundaries = boundaries;
-            boundaries = NULL;
-        }
-        umtx_unlock(NULL);
-        if (bi) {
-            delete bi;
-        }
-        if (boundaries) {
-            delete boundaries;
+        {
+            Mutex m;
+            BreakTransliterator *nonConstThis = const_cast<BreakTransliterator *>(this);
+            if (nonConstThis->cachedBI.isNull()) {
+                nonConstThis->cachedBI.adoptInstead(bi.orphan());
+            }
+            if (nonConstThis->cachedBoundaries.isNull()) {
+                nonConstThis->cachedBoundaries.adoptInstead(boundaries.orphan());
+            }
         }
 
         // TODO:  do something with U_FAILURE(status);
index 0ed428232a274c286517e51aae50da7aef906338..48f83de886901be530864b828012ebc54fe25a9c 100644 (file)
@@ -16,6 +16,8 @@
 
 #include "unicode/translit.h"
 
+#include "unicode/localpointer.h"
+
 
 U_NAMESPACE_BEGIN
 
@@ -30,10 +32,6 @@ class UVector32;
 class BreakTransliterator : public Transliterator {
 public:
 
-    BreakTransliterator(const UnicodeString &ID, 
-                        UnicodeFilter *adoptedFilter,
-                        BreakIterator *bi, 
-                        const UnicodeString &insertion);
     /**
      * Constructs a transliterator.
      * @param adoptedFilter    the filter for this transliterator.
@@ -85,9 +83,9 @@ public:
                                      UBool isIncremental) const;
 
  private:
-     BreakIterator     *cachedBI;
-     UVector32         *cachedBoundaries;
-     UnicodeString      fInsertion;
+     LocalPointer<BreakIterator> cachedBI;
+     LocalPointer<UVector32>     cachedBoundaries;
+     UnicodeString               fInsertion;
 
      static UnicodeString replaceableAsString(Replaceable &r);
 
index 877375d6bf1c7d3c0016b1971f8fdb278b28b0eb..5c4d3beaee5ece34a2951ef7d06b04009da29deb 100644 (file)
@@ -18,6 +18,7 @@
 #include "rbt_data.h"
 #include "rbt_rule.h"
 #include "rbt.h"
+#include "mutex.h"
 #include "umutex.h"
 
 U_NAMESPACE_BEGIN
@@ -254,15 +255,14 @@ RuleBasedTransliterator::handleTransliterate(Replaceable& text, UTransPosition&
         //
         // TODO(andy): Need a better scheme for handling this.
         UBool needToLock;
-        umtx_lock(NULL);
-        needToLock = (&text != gLockedText);
-        umtx_unlock(NULL);
-
+        {
+            Mutex m;
+            needToLock = (&text != gLockedText);
+        }
         if (needToLock) {
-            umtx_lock(&transliteratorDataMutex);
-            umtx_lock(NULL);
+            umtx_lock(&transliteratorDataMutex);  // Contention, longish waits possible here.
+            Mutex m;
             gLockedText = &text;
-            umtx_unlock(NULL);
             lockedMutexAtThisLevel = TRUE;
         }
     }
@@ -276,9 +276,10 @@ RuleBasedTransliterator::handleTransliterate(Replaceable& text, UTransPosition&
            }
     }
     if (lockedMutexAtThisLevel) {
-        umtx_lock(NULL);
-        gLockedText = NULL;
-        umtx_unlock(NULL);
+        {
+            Mutex m;
+            gLockedText = NULL;
+        }
         umtx_unlock(&transliteratorDataMutex);
     }
 }