From a8d09369fea1574b24309d7b7b2bb373021bf387 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Mon, 18 Jan 2010 14:16:16 +0100 Subject: Make QTextCodec reentrant. QTextCodec::codecForName and codedForMib were not reentrant Reviewed-by: Brad Reviewed-by: Denis --- src/corelib/codecs/qsimplecodec.cpp | 2 +- src/corelib/codecs/qtextcodec.cpp | 46 ++++++++++++++++++++++++++------ tests/auto/qtextcodec/tst_qtextcodec.cpp | 46 ++++++++++++++++++++++++++++++++ 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 #include @@ -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 QTextCodec::availableCodecs() { +#ifndef QT_NO_THREAD + QMutexLocker locker(textCodecsMutex()); +#endif setup(); QList codecs; @@ -1008,6 +1022,11 @@ QList 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 QTextCodec::availableCodecs() */ QList QTextCodec::availableMibs() { +#ifndef QT_NO_THREAD + QMutexLocker locker(textCodecsMutex()); +#endif setup(); QList 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 #include #include +#include +#include #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 codecList = QTextCodec::availableCodecs(); + QFuture res = QtConcurrent::mapped(codecList, loadAndConvert); + + QList mibList = QTextCodec::availableMibs(); + QFuture res2 = QtConcurrent::mapped(mibList, loadAndConvertMIB); + + QCOMPARE(res.results(), codecList); + QCOMPARE(res2.results(), mibList); +} + QTEST_MAIN(tst_QTextCodec) #include "tst_qtextcodec.moc" -- cgit v0.12 From 7724c474a941ea3516da6102a16b30ca254afe53 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Mon, 8 Feb 2010 12:25:10 +0100 Subject: QGraphicsItem: Do not crash at exit if there is static QGraphicsItem. The DataStore could have been destroyed before. Even if having static QGraphicsItem is not really supported, it is better not to crash Task-number: QTBUG-7629 Reviewed-by: bnilsen --- src/gui/graphicsview/qgraphicsitem.cpp | 3 ++- tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp index b4e19d1..39c41c4 100644 --- a/src/gui/graphicsview/qgraphicsitem.cpp +++ b/src/gui/graphicsview/qgraphicsitem.cpp @@ -1392,7 +1392,8 @@ QGraphicsItem::~QGraphicsItem() } delete d_ptr->transformData; - qt_dataStore()->data.remove(this); + if (QGraphicsItemCustomDataStore *dataStore = qt_dataStore()) + dataStore->data.remove(this); } /*! diff --git a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp index 7b54a3b..7c1b97e 100644 --- a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp +++ b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp @@ -90,6 +90,8 @@ Q_DECLARE_METATYPE(QRectF) #define COMPARE_REGIONS QTRY_COMPARE #endif +static QGraphicsRectItem staticItem; //QTBUG-7629, we should not crash at exit. + static void sendMousePress(QGraphicsScene *scene, const QPointF &point, Qt::MouseButton button = Qt::LeftButton) { QGraphicsSceneMouseEvent event(QEvent::GraphicsSceneMousePress); -- cgit v0.12