Diff
Modified: trunk/Source/WebCore/ChangeLog (118567 => 118568)
--- trunk/Source/WebCore/ChangeLog 2012-05-25 21:42:32 UTC (rev 118567)
+++ trunk/Source/WebCore/ChangeLog 2012-05-25 21:46:57 UTC (rev 118568)
@@ -1,3 +1,59 @@
+2012-05-25 Dan Bernstein <[email protected]>
+
+ characterBreakIterator() is not safe to use reentrantly or from multiple threads
+ https://bugs.webkit.org/show_bug.cgi?id=87521
+
+ Reviewed by Darin Adler.
+
+ Replaced characterBreakIterator() with a NonSharedCharacterBreakIterator class, which
+ obtains a unique TextBreakIterator. Replaced the global shared instance with a single-entry
+ cache.
+
+ * dom/CharacterData.cpp:
+ (WebCore::CharacterData::parserAppendData): Changed to use NonSharedCharacterBreakIterator.
+
+ * platform/graphics/StringTruncator.cpp:
+ (WebCore::centerTruncateToBuffer): Ditto.
+ (WebCore::rightTruncateToBuffer): Ditto.
+
+ * platform/text/String.cpp:
+ (WebCore::numGraphemeClusters): Ditto.
+ (WebCore::numCharactersInGraphemeClusters): Ditto.
+
+ * platform/text/TextBreakIterator.h: Removed the declaration of characterBreakIterator().
+ (NonSharedCharacterBreakIterator): Added. An instance of this class has a character break
+ iterator instance that is unique to it for the lifetime of the instance.
+ (WebCore::NonSharedCharacterBreakIterator::operator TextBreakIterator*): Added.
+
+ * platform/text/TextBreakIteratorICU.cpp:
+ (WebCore::NonSharedCharacterBreakIterator::NonSharedCharacterBreakIterator): Added. Tries
+ to swap the m_iterator member variable with the cached instance. If that fails, initializes
+ m_iterator to a new character break iterator.
+ (WebCore::NonSharedCharacterBreakIterator::~NonSharedCharacterBreakIterator): Added. Tries
+ to put the m_iterator member variable back in the cache. If that fails, meaning there is
+ already something in the cache, destroys m_iterator.
+
+ * platform/text/gtk/TextBreakIteratorGtk.cpp:
+ (WebCore::NonSharedCharacterBreakIterator::NonSharedCharacterBreakIterator): Same as in
+ TextBreakIteratorICU.cpp.
+ (WebCore::NonSharedCharacterBreakIterator::~NonSharedCharacterBreakIterator): Ditto.
+ (WebCore::cursorMovementIterator): Moved the old implementation of characterBreakIterator()
+ here.
+
+ * platform/text/qt/TextBreakIteratorQt.cpp:
+ (WebCore::NonSharedCharacterBreakIterator::NonSharedCharacterBreakIterator): Same as in
+ TextBreakIteratorICU.cpp.
+ (WebCore::NonSharedCharacterBreakIterator::~NonSharedCharacterBreakIterator): Ditto.
+ (WebCore::cursorMovementIterator): Moved the old implementation of characterBreakIterator()
+ here.
+
+ * platform/text/wince/TextBreakIteratorWinCE.cpp:
+ (WebCore::NonSharedCharacterBreakIterator::NonSharedCharacterBreakIterator): Same as in
+ TextBreakIteratorICU.cpp.
+ (WebCore::NonSharedCharacterBreakIterator::~NonSharedCharacterBreakIterator): Ditto.
+ (WebCore::cursorMovementIterator): Moved the old implementation of characterBreakIterator()
+ here.
+
2012-05-25 Simon Fraser <[email protected]>
Terrible performance on http://alliances.commandandconquer.com/ and http://www.lordofultima.com/
Modified: trunk/Source/WebCore/dom/CharacterData.cpp (118567 => 118568)
--- trunk/Source/WebCore/dom/CharacterData.cpp 2012-05-25 21:42:32 UTC (rev 118567)
+++ trunk/Source/WebCore/dom/CharacterData.cpp 2012-05-25 21:46:57 UTC (rev 118568)
@@ -72,7 +72,7 @@
// see <https://bugs.webkit.org/show_bug.cgi?id=29092>.
// We need at least two characters look-ahead to account for UTF-16 surrogates.
if (end < dataLength) {
- TextBreakIterator* it = characterBreakIterator(data, (end + 2 > dataLength) ? dataLength : end + 2);
+ NonSharedCharacterBreakIterator it(data, (end + 2 > dataLength) ? dataLength : end + 2);
if (!isTextBreak(it, end))
end = textBreakPreceding(it, end);
}
Modified: trunk/Source/WebCore/platform/graphics/StringTruncator.cpp (118567 => 118568)
--- trunk/Source/WebCore/platform/graphics/StringTruncator.cpp 2012-05-25 21:42:32 UTC (rev 118567)
+++ trunk/Source/WebCore/platform/graphics/StringTruncator.cpp 2012-05-25 21:46:57 UTC (rev 118568)
@@ -63,7 +63,7 @@
ASSERT(keepCount < STRING_BUFFER_SIZE);
unsigned omitStart = (keepCount + 1) / 2;
- TextBreakIterator* it = characterBreakIterator(string.characters(), length);
+ NonSharedCharacterBreakIterator it(string.characters(), length);
unsigned omitEnd = boundedTextBreakFollowing(it, omitStart + (length - keepCount) - 1, length);
omitStart = textBreakAtOrPreceding(it, omitStart);
@@ -82,7 +82,7 @@
ASSERT(keepCount < length);
ASSERT(keepCount < STRING_BUFFER_SIZE);
- TextBreakIterator* it = characterBreakIterator(string.characters(), length);
+ NonSharedCharacterBreakIterator it(string.characters(), length);
unsigned keepLength = textBreakAtOrPreceding(it, keepCount);
unsigned truncatedLength = keepLength + 1;
Modified: trunk/Source/WebCore/platform/text/String.cpp (118567 => 118568)
--- trunk/Source/WebCore/platform/text/String.cpp 2012-05-25 21:42:32 UTC (rev 118567)
+++ trunk/Source/WebCore/platform/text/String.cpp 2012-05-25 21:46:57 UTC (rev 118568)
@@ -51,7 +51,7 @@
unsigned numGraphemeClusters(const String& s)
{
- TextBreakIterator* it = characterBreakIterator(s.characters(), s.length());
+ NonSharedCharacterBreakIterator it(s.characters(), s.length());
if (!it)
return s.length();
@@ -63,7 +63,7 @@
unsigned numCharactersInGraphemeClusters(const String& s, unsigned numGraphemeClusters)
{
- TextBreakIterator* it = characterBreakIterator(s.characters(), s.length());
+ NonSharedCharacterBreakIterator it(s.characters(), s.length());
if (!it)
return min(s.length(), numGraphemeClusters);
Modified: trunk/Source/WebCore/platform/text/TextBreakIterator.h (118567 => 118568)
--- trunk/Source/WebCore/platform/text/TextBreakIterator.h 2012-05-25 21:42:32 UTC (rev 118567)
+++ trunk/Source/WebCore/platform/text/TextBreakIterator.h 2012-05-25 21:46:57 UTC (rev 118568)
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2006 Lars Knoll <[email protected]>
- * Copyright (C) 2007 Apple Inc. All rights reserved.
+ * Copyright (C) 2007, 2011, 2012 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -31,12 +31,6 @@
// Note: The returned iterator is good only until you get another iterator, with the exception of acquireLineBreakIterator.
- // Iterates over "extended grapheme clusters", as defined in UAX #29.
- // Note that platform implementations may be less sophisticated - e.g. ICU prior to
- // version 4.0 only supports "legacy grapheme clusters".
- // Use this for general text processing, e.g. string truncation.
- TextBreakIterator* characterBreakIterator(const UChar*, int length);
-
// This is similar to character break iterator in most cases, but is subject to
// platform UI conventions. One notable example where this can be different
// from character break iterator is Thai prepend characters, see bug 24342.
@@ -104,6 +98,23 @@
TextBreakIterator* m_iterator;
};
+// Iterates over "extended grapheme clusters", as defined in UAX #29.
+// Note that platform implementations may be less sophisticated - e.g. ICU prior to
+// version 4.0 only supports "legacy grapheme clusters".
+// Use this for general text processing, e.g. string truncation.
+
+class NonSharedCharacterBreakIterator {
+ WTF_MAKE_NONCOPYABLE(NonSharedCharacterBreakIterator);
+public:
+ NonSharedCharacterBreakIterator(const UChar*, int length);
+ ~NonSharedCharacterBreakIterator();
+
+ operator TextBreakIterator*() const { return m_iterator; }
+
+private:
+ TextBreakIterator* m_iterator;
+};
+
}
#endif
Modified: trunk/Source/WebCore/platform/text/TextBreakIteratorICU.cpp (118567 => 118568)
--- trunk/Source/WebCore/platform/text/TextBreakIteratorICU.cpp 2012-05-25 21:42:32 UTC (rev 118567)
+++ trunk/Source/WebCore/platform/text/TextBreakIteratorICU.cpp 2012-05-25 21:46:57 UTC (rev 118568)
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2006 Lars Knoll <[email protected]>
- * Copyright (C) 2007 Apple Inc. All rights reserved.
+ * Copyright (C) 2007, 2011, 2012 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -24,7 +24,9 @@
#include "LineBreakIteratorPoolICU.h"
#include "PlatformString.h"
+#include <wtf/Atomics.h>
+using namespace WTF;
using namespace std;
namespace WebCore {
@@ -52,14 +54,6 @@
return iterator;
}
-TextBreakIterator* characterBreakIterator(const UChar* string, int length)
-{
- static bool createdCharacterBreakIterator = false;
- static TextBreakIterator* staticCharacterBreakIterator;
- return setUpIterator(createdCharacterBreakIterator,
- staticCharacterBreakIterator, UBRK_CHARACTER, string, length);
-}
-
TextBreakIterator* wordBreakIterator(const UChar* string, int length)
{
static bool createdWordBreakIterator = false;
@@ -91,6 +85,21 @@
LineBreakIteratorPool::sharedPool().put(reinterpret_cast<UBreakIterator*>(iterator));
}
+static TextBreakIterator* nonSharedCharacterBreakIterator;
+
+NonSharedCharacterBreakIterator::NonSharedCharacterBreakIterator(const UChar* buffer, int length)
+{
+ m_iterator = nonSharedCharacterBreakIterator;
+ bool createdIterator = m_iterator && weakCompareAndSwap(reinterpret_cast<void**>(&nonSharedCharacterBreakIterator), m_iterator, 0);
+ m_iterator = setUpIterator(createdIterator, m_iterator, UBRK_CHARACTER, buffer, length);
+}
+
+NonSharedCharacterBreakIterator::~NonSharedCharacterBreakIterator()
+{
+ if (!weakCompareAndSwap(reinterpret_cast<void**>(&nonSharedCharacterBreakIterator), 0, m_iterator))
+ ubrk_close(reinterpret_cast<UBreakIterator*>(m_iterator));
+}
+
TextBreakIterator* sentenceBreakIterator(const UChar* string, int length)
{
static bool createdSentenceBreakIterator = false;
Modified: trunk/Source/WebCore/platform/text/gtk/TextBreakIteratorGtk.cpp (118567 => 118568)
--- trunk/Source/WebCore/platform/text/gtk/TextBreakIteratorGtk.cpp 2012-05-25 21:42:32 UTC (rev 118567)
+++ trunk/Source/WebCore/platform/text/gtk/TextBreakIteratorGtk.cpp 2012-05-25 21:46:57 UTC (rev 118568)
@@ -23,11 +23,13 @@
*/
#include "config.h"
-
#include "TextBreakIterator.h"
+#include <wtf/Atomics.h>
#include <wtf/gobject/GOwnPtr.h>
#include <pango/pango.h>
+
+using namespace WTF;
using namespace std;
#define UTF8_IS_SURROGATE(character) (character >= 0x10000 && character <= 0x10FFFF)
@@ -219,17 +221,27 @@
return iterator;
}
-TextBreakIterator* characterBreakIterator(const UChar* string, int length)
+static TextBreakIterator* nonSharedCharacterBreakIterator;
+
+NonSharedCharacterBreakIterator::NonSharedCharacterBreakIterator(const UChar* buffer, int length)
{
- static bool createdCharacterBreakIterator = false;
- static TextBreakIterator* staticCharacterBreakIterator;
- return setUpIterator(createdCharacterBreakIterator, staticCharacterBreakIterator, UBRK_CHARACTER, string, length);
+ m_iterator = nonSharedCharacterBreakIterator;
+ bool createdIterator = m_iterator && weakCompareAndSwap(reinterpret_cast<void**>(&nonSharedCharacterBreakIterator), m_iterator, 0);
+ m_iterator = setUpIterator(createdIterator, m_iterator, UBRK_CHARACTER, buffer, length);
}
+NonSharedCharacterBreakIterator::~NonSharedCharacterBreakIterator()
+{
+ if (!weakCompareAndSwap(reinterpret_cast<void**>(&nonSharedCharacterBreakIterator), 0, m_iterator))
+ ubrk_close(m_iterator);
+}
+
TextBreakIterator* cursorMovementIterator(const UChar* string, int length)
{
// FIXME: This needs closer inspection to achieve behaviour identical to the ICU version.
- return characterBreakIterator(string, length);
+ static bool createdCursorMovementIterator = false;
+ static TextBreakIterator* staticCursorMovementIterator;
+ return setUpIterator(createdCursorMovementIterator, staticCursorMovementIterator, UBRK_CHARACTER, string, length);
}
TextBreakIterator* wordBreakIterator(const UChar* string, int length)
Modified: trunk/Source/WebCore/platform/text/qt/TextBreakIteratorQt.cpp (118567 => 118568)
--- trunk/Source/WebCore/platform/text/qt/TextBreakIteratorQt.cpp 2012-05-25 21:42:32 UTC (rev 118567)
+++ trunk/Source/WebCore/platform/text/qt/TextBreakIteratorQt.cpp 2012-05-25 21:46:57 UTC (rev 118568)
@@ -24,6 +24,7 @@
#include <QtCore/qtextboundaryfinder.h>
#include <algorithm>
#include <qdebug.h>
+#include <wtf/Atomics.h>
// #define DEBUG_TEXT_ITERATORS
#ifdef DEBUG_TEXT_ITERATORS
@@ -32,6 +33,7 @@
#define DEBUG if (1) {} else qDebug
#endif
+using namespace WTF;
using namespace std;
namespace WebCore {
@@ -66,15 +68,27 @@
return setUpIterator(staticWordBreakIterator, QTextBoundaryFinder::Word, string, length);
}
- TextBreakIterator* characterBreakIterator(const UChar* string, int length)
+ static TextBreakIterator* nonSharedCharacterBreakIterator;
+
+ NonSharedCharacterBreakIterator::NonSharedCharacterBreakIterator(const UChar* buffer, int length)
{
- static TextBreakIterator staticCharacterBreakIterator;
- return setUpIterator(staticCharacterBreakIterator, QTextBoundaryFinder::Grapheme, string, length);
+ m_iterator = nonSharedCharacterBreakIterator;
+ bool createdIterator = m_iterator && weakCompareAndSwap(reinterpret_cast<void**>(&nonSharedCharacterBreakIterator), m_iterator, 0);
+ if (!createdIterator)
+ m_iterator = new TextBreakIterator();
+ setUpIterator(*m_iterator, QTextBoundaryFinder::Grapheme, buffer, length);
}
+ NonSharedCharacterBreakIterator::~NonSharedCharacterBreakIterator()
+ {
+ if (!weakCompareAndSwap(reinterpret_cast<void**>(&nonSharedCharacterBreakIterator), 0, m_iterator))
+ delete m_iterator;
+ }
+
TextBreakIterator* cursorMovementIterator(const UChar* string, int length)
{
- return characterBreakIterator(string, length);
+ static TextBreakIterator staticCursorMovementIterator;
+ return setUpIterator(staticCursorMovementIterator, QTextBoundaryFinder::Grapheme, string, length);
}
static TextBreakIterator* staticLineBreakIterator;
Modified: trunk/Source/WebCore/platform/text/wince/TextBreakIteratorWinCE.cpp (118567 => 118568)
--- trunk/Source/WebCore/platform/text/wince/TextBreakIteratorWinCE.cpp 2012-05-25 21:42:32 UTC (rev 118567)
+++ trunk/Source/WebCore/platform/text/wince/TextBreakIteratorWinCE.cpp 2012-05-25 21:46:57 UTC (rev 118568)
@@ -23,11 +23,13 @@
#include "TextBreakIterator.h"
#include "PlatformString.h"
+#include <wtf/Atomics.h>
#include <wtf/StdLibExtras.h>
#include <wtf/unicode/Unicode.h>
+using namespace WTF;
+using namespace WTF::Unicode;
using namespace std;
-using namespace WTF::Unicode;
namespace WebCore {
@@ -235,13 +237,23 @@
return &iterator;
}
-TextBreakIterator* characterBreakIterator(const UChar* string, int length)
+static CharBreakIterator* nonSharedCharacterBreakIterator;
+
+NonSharedCharacterBreakIterator::NonSharedCharacterBreakIterator(const UChar* buffer, int length)
{
- DEFINE_STATIC_LOCAL(CharBreakIterator, iterator, ());
- iterator.reset(string, length);
- return &iterator;
+ m_iterator = nonSharedCharacterBreakIterator;
+ bool createdIterator = m_iterator && weakCompareAndSwap(reinterpret_cast<void**>(&nonSharedCharacterBreakIterator), m_iterator, 0);
+ if (!createdIterator)
+ m_iterator = new CharBreakIterator;
+ m_iterator.reset(string, length);
}
+NonSharedCharacterBreakIterator::~NonSharedCharacterBreakIterator()
+{
+ if (!weakCompareAndSwap(reinterpret_cast<void**>(&nonSharedCharacterBreakIterator), 0, m_iterator))
+ delete m_iterator;
+}
+
static TextBreakIterator* staticLineBreakIterator;
TextBreakIterator* acquireLineBreakIterator(const UChar* string, int length, const AtomicString&)
@@ -324,7 +336,9 @@
TextBreakIterator* cursorMovementIterator(const UChar* string, int length)
{
- return characterBreakIterator(string, length);
+ DEFINE_STATIC_LOCAL(CharBreakIterator, iterator, ());
+ iterator.reset(string, length);
+ return &iterator;
}
} // namespace WebCore
