Make QTextCodec reentrant.

QTextCodec::codecForName and codedForMib were not reentrant

Reviewed-by: Brad
Reviewed-by: Denis

3 files changed, 85 insertions, 9 deletions

diff --git a/src/corelib/codecs/qsimplecodec.cpp b/src/corelib/codecs/qsimplecodec.cpp
index 445565a..4cc7912 100644
--- a/src/corelib/codecs/qsimplecodec.cpp
+++ b/src/corelib/codecs/qsimplecodec.cpp
@@ -681,7 +681,7 @@ QByteArray QSimpleTextCodec::convertFromUnicode(const QChar *in, int length, Con
     int u;
     const QChar* ucp = in;
     unsigned char* rp = (unsigned char *)r.data();
-    const unsigned char* rmp = (const unsigned char *)reverseMap->data();
+    const unsigned char* rmp = (const unsigned char *)reverseMap->constData();
     int rmsize = (int) reverseMap->size();
     while(i--)
     {
diff --git a/src/corelib/codecs/qtextcodec.cpp b/src/corelib/codecs/qtextcodec.cpp
index 698ca9e..e9c1803 100644
--- a/src/corelib/codecs/qtextcodec.cpp
+++ b/src/corelib/codecs/qtextcodec.cpp
@@ -79,7 +79,7 @@
 #  endif
 #endif // QT_NO_CODECS
 #include "qlocale.h"
-#include "private/qmutexpool_p.h"
+#include "qmutex.h"
 
 #include <stdlib.h>
 #include <ctype.h>
@@ -659,13 +659,13 @@ static void setupLocaleMapper()
 #endif
 }
 
-
-static void setup()
-{
 #ifndef QT_NO_THREAD
-    QMutexLocker locker(QMutexPool::globalInstanceGet(&all));
+Q_GLOBAL_STATIC_WITH_ARGS(QMutex, textCodecsMutex, (QMutex::Recursive));
 #endif
 
+// textCodecsMutex need to be locked to enter this function
+static void setup()
+{
     if (all)
         return;
 
@@ -903,8 +903,6 @@ QTextCodec::ConverterState::~ConverterState()
 */
 
 /*!
-    \nonreentrant
-
     Constructs a QTextCodec, and gives it the highest precedence. The
     QTextCodec should always be constructed on the heap (i.e. with \c
     new). Qt takes ownership and will delete it when the application
@@ -912,6 +910,9 @@ QTextCodec::ConverterState::~ConverterState()
 */
 QTextCodec::QTextCodec()
 {
+#ifndef QT_NO_THREAD
+    QMutexLocker locker(textCodecsMutex());
+#endif
     setup();
     all->prepend(this);
 }
@@ -929,8 +930,12 @@ QTextCodec::~QTextCodec()
     if (!destroying_is_ok)
         qWarning("QTextCodec::~QTextCodec: Called by application");
 #endif
-    if (all)
+    if (all) {
+#ifndef QT_NO_THREAD
+        QMutexLocker locker(textCodecsMutex());
+#endif
         all->removeAll(this);
+    }
 }
 
 /*!
@@ -951,6 +956,9 @@ QTextCodec *QTextCodec::codecForName(const QByteArray &name)
     if (name.isEmpty())
         return 0;
 
+#ifndef QT_NO_THREAD
+    QMutexLocker locker(textCodecsMutex());
+#endif
     setup();
 
     for (int i = 0; i < all->size(); ++i) {
@@ -973,6 +981,9 @@ QTextCodec *QTextCodec::codecForName(const QByteArray &name)
 */
 QTextCodec* QTextCodec::codecForMib(int mib)
 {
+#ifndef QT_NO_THREAD
+    QMutexLocker locker(textCodecsMutex());
+#endif
     setup();
 
     // Qt 3 used 1000 (mib for UCS2) as its identifier for the utf16 codec. Map
@@ -1001,6 +1012,9 @@ QTextCodec* QTextCodec::codecForMib(int mib)
 */
 QList<QByteArray> QTextCodec::availableCodecs()
 {
+#ifndef QT_NO_THREAD
+    QMutexLocker locker(textCodecsMutex());
+#endif
     setup();
 
     QList<QByteArray> codecs;
@@ -1008,6 +1022,11 @@ QList<QByteArray> QTextCodec::availableCodecs()
         codecs += all->at(i)->name();
         codecs += all->at(i)->aliases();
     }
+
+#ifndef QT_NO_THREAD
+    locker.unlock();
+#endif
+
 #ifndef QT_NO_TEXTCODECPLUGIN
     QFactoryLoader *l = loader();
     QStringList keys = l->keys();
@@ -1031,11 +1050,19 @@ QList<QByteArray> QTextCodec::availableCodecs()
 */
 QList<int> QTextCodec::availableMibs()
 {
+#ifndef QT_NO_THREAD
+    QMutexLocker locker(textCodecsMutex());
+#endif
     setup();
 
     QList<int> codecs;
     for (int i = 0; i < all->size(); ++i)
         codecs += all->at(i)->mibEnum();
+
+#ifndef QT_NO_THREAD
+    locker.unlock();
+#endif
+
 #ifndef QT_NO_TEXTCODECPLUGIN
     QFactoryLoader *l = loader();
     QStringList keys = l->keys();
@@ -1082,6 +1109,9 @@ QTextCodec* QTextCodec::codecForLocale()
     if (localeMapper)
         return localeMapper;
 
+#ifndef QT_NO_THREAD
+    QMutexLocker locker(textCodecsMutex());
+#endif
     setup();
 
     return localeMapper;
diff --git a/tests/auto/qtextcodec/tst_qtextcodec.cpp b/tests/auto/qtextcodec/tst_qtextcodec.cpp
index eb348fb..65b0448 100644
--- a/tests/auto/qtextcodec/tst_qtextcodec.cpp
+++ b/tests/auto/qtextcodec/tst_qtextcodec.cpp
@@ -47,6 +47,8 @@
 #include <qtextdocument.h>
 #include <time.h>
 #include <qprocess.h>
+#include <QtConcurrentMap>
+#include <QThreadPool>
 
 #ifdef Q_OS_SYMBIAN
 #define SRCDIR ""
@@ -58,6 +60,9 @@ class tst_QTextCodec : public QObject
     Q_OBJECT
 
 private slots:
+
+    void threadSafety();
+
     void toUnicode_data();
     void toUnicode();
     void codecForName_data();
@@ -1904,5 +1909,46 @@ void tst_QTextCodec::toLocal8Bit()
 }
 #endif
 
+static QByteArray loadAndConvert(const QByteArray &codecName)
+{
+    QTextCodec *c = QTextCodec::codecForName(codecName);
+    if (!c) {
+        qDebug() << "WARNING " << codecName << " not found? ";
+        return QByteArray();
+    }
+    QString str = QString::fromLatin1(codecName);
+    QByteArray b = c->fromUnicode(str);
+    c->toUnicode(b);
+    return codecName;
+}
+
+static int loadAndConvertMIB(int mib)
+{
+    QTextCodec *c = QTextCodec::codecForMib(mib);
+    if (!c) {
+        qDebug() << "WARNING " << mib << " not found? ";
+        return 0;
+    }
+    QString str = QString::number(mib);
+    QByteArray b = c->fromUnicode(str);
+    c->toUnicode(b);
+    return mib;
+}
+
+
+void tst_QTextCodec::threadSafety()
+{
+    QThreadPool::globalInstance()->setMaxThreadCount(12);
+    
+    QList<QByteArray> codecList = QTextCodec::availableCodecs();
+    QFuture<QByteArray> res = QtConcurrent::mapped(codecList, loadAndConvert);
+
+    QList<int> mibList = QTextCodec::availableMibs();
+    QFuture<int> res2 = QtConcurrent::mapped(mibList, loadAndConvertMIB);
+
+    QCOMPARE(res.results(), codecList);
+    QCOMPARE(res2.results(), mibList);
+}
+
 QTEST_MAIN(tst_QTextCodec)
 #include "tst_qtextcodec.moc"