From 88a417b47b758d35ef67d7a73f9b9a8ef8a843e1 Mon Sep 17 00:00:00 2001 From: Sarah Smith Date: Thu, 12 Aug 2010 17:23:27 +1000 Subject: Fix memory leak. Mac only bug - run examples/opengl/overpainting under leak detect instrument on Mac OSX to see it. Memory leak in any application that called getProcAddress() - CFStrings have to be cleaned up if the name of the function you got them from contains the word "Create" - and we weren't doing that. Reviewed-by: Gunnar Sletta --- src/opengl/qgl_mac.mm | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/opengl/qgl_mac.mm b/src/opengl/qgl_mac.mm index 4d7532e..66fe7d3 100644 --- a/src/opengl/qgl_mac.mm +++ b/src/opengl/qgl_mac.mm @@ -804,17 +804,22 @@ void QGLContext::generateFontDisplayLists(const QFont & /* fnt */, int /* listBa static CFBundleRef qt_getOpenGLBundle() { CFBundleRef bundle = 0; + CFStringRef urlString = QCFString::toCFStringRef(QLatin1String("/System/Library/Frameworks/OpenGL.framework")); QCFType url = CFURLCreateWithFileSystemPath(kCFAllocatorDefault, - QCFString::toCFStringRef(QLatin1String("/System/Library/Frameworks/OpenGL.framework")), kCFURLPOSIXPathStyle, false); + urlString, kCFURLPOSIXPathStyle, false); if (url) bundle = CFBundleCreate(kCFAllocatorDefault, url); + CFRelease(urlString); return bundle; } void *QGLContext::getProcAddress(const QString &proc) const { - return CFBundleGetFunctionPointerForName(QCFType(qt_getOpenGLBundle()), - QCFString(proc)); + CFStringRef procName = QCFString(proc).toCFStringRef(proc); + void *result = CFBundleGetFunctionPointerForName(QCFType(qt_getOpenGLBundle()), + procName); + CFRelease(procName); + return result; } #ifndef QT_MAC_USE_COCOA /***************************************************************************** -- cgit v0.12 From d32127afb9b1ec49d4dc5cc672eaea11ebe56b72 Mon Sep 17 00:00:00 2001 From: Olivier Goffart Date: Thu, 12 Aug 2010 10:25:41 +0200 Subject: QCoreApplication::library path, ensure mutex lock ordering Do not keep the libraryPathMutex locked while calling QFactoryLoader::refreshAll QFactoryLoader also lock a mutex, and recurse into QCoreApplication::libraryPath Reviewed-by: Brad Helgrind warning for reference: ==8442== Thread #1: lock order "0xDEBA470 before 0xDF63600" violated ==8442== at 0x4C2911E: QMutex::lock() (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so) ==8442== by 0x7EE5870: QMutexLocker::QMutexLocker(QMutex*) (qmutex.h:102) ==8442== by 0x8025A8E: QCoreApplication::libraryPaths() (qcoreapplication.cpp:2234) ==8442== by 0x8006FDC: QFactoryLoader::update() (qfactoryloader.cpp:109) ==8442== by 0x8006F32: QFactoryLoader::QFactoryLoader(char const*, QString const&, Qt::CaseSensitivity) (qfactoryloader.cpp:99) ==8442== by 0x6DE4351: qt_guiPlatformPlugin() (qguiplatformplugin.cpp:101) ==8442== by 0x6DE7A5B: QApplicationPrivate::x11_apply_settings() (qapplication_x11.cpp:934) ==8442== by 0x6DE9BAD: qt_set_x11_resources(char const*, char const*, char const*, char const*) (qapplication_x11.cpp:1111) ==8442== by 0x6DF0CB3: qt_init(QApplicationPrivate*, int, _XDisplay*, unsigned long, unsigned long) (qapplication_x11.cpp:2323) ==8442== by 0x6D4EF4D: QApplicationPrivate::construct(_XDisplay*, unsigned long, unsigned long) (qapplication.cpp:793) ==8442== by 0x6D4EABE: QApplication::QApplication(int&, char**, int) (qapplication.cpp:712) ==8442== by 0x41350F: main (tst_examples.cpp:227) ==8442== Required order was established by acquisition of lock at 0xDEBA470 ==8442== at 0x4C2911E: QMutex::lock() (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so) ==8442== by 0x7EE5870: QMutexLocker::QMutexLocker(QMutex*) (qmutex.h:102) ==8442== by 0x8025FC3: QCoreApplication::addLibraryPath(QString const&) (qcoreapplication.cpp:2335) ==8442== by 0x6DE792D: QApplicationPrivate::x11_apply_settings() (qapplication_x11.cpp:927) ==8442== by 0x6DE9BAD: qt_set_x11_resources(char const*, char const*, char const*, char const*) (qapplication_x11.cpp:1111) ==8442== by 0x6DF0CB3: qt_init(QApplicationPrivate*, int, _XDisplay*, unsigned long, unsigned long) (qapplication_x11.cpp:2323) ==8442== by 0x6D4EF4D: QApplicationPrivate::construct(_XDisplay*, unsigned long, unsigned long) (qapplication.cpp:793) ==8442== by 0x6D4EABE: QApplication::QApplication(int&, char**, int) (qapplication.cpp:712) ==8442== by 0x41350F: main (tst_examples.cpp:227) ==8442== followed by a later acquisition of lock at 0xDF63600 ==8442== at 0x4C2911E: QMutex::lock() (in /usr/lib/valgrind/vgpreload_helgrind-amd64-linux.so) ==8442== by 0x7EE5870: QMutexLocker::QMutexLocker(QMutex*) (qmutex.h:102) ==8442== by 0x80086A9: QFactoryLoader::refreshAll() (qfactoryloader.cpp:249) ==8442== by 0x802607E: QCoreApplication::addLibraryPath(QString const&) (qcoreapplication.cpp:2344) ==8442== by 0x6DE792D: QApplicationPrivate::x11_apply_settings() (qapplication_x11.cpp:927) ==8442== by 0x6DE9BAD: qt_set_x11_resources(char const*, char const*, char const*, char const*) (qapplication_x11.cpp:1111) ==8442== by 0x6DF0CB3: qt_init(QApplicationPrivate*, int, _XDisplay*, unsigned long, unsigned long) (qapplication_x11.cpp:2323) ==8442== by 0x6D4EF4D: QApplicationPrivate::construct(_XDisplay*, unsigned long, unsigned long) (qapplication.cpp:793) ==8442== by 0x6D4EABE: QApplication::QApplication(int&, char**, int) (qapplication.cpp:712) ==8442== by 0x41350F: main (tst_examples.cpp:227)a --- src/corelib/kernel/qcoreapplication.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/corelib/kernel/qcoreapplication.cpp b/src/corelib/kernel/qcoreapplication.cpp index 0a5e06e..512e193 100644 --- a/src/corelib/kernel/qcoreapplication.cpp +++ b/src/corelib/kernel/qcoreapplication.cpp @@ -2308,6 +2308,7 @@ void QCoreApplication::setLibraryPaths(const QStringList &paths) if (!coreappdata()->app_libpaths) coreappdata()->app_libpaths = new QStringList; *(coreappdata()->app_libpaths) = paths; + locker.unlock(); QFactoryLoader::refreshAll(); } @@ -2341,6 +2342,7 @@ void QCoreApplication::addLibraryPath(const QString &path) if (!canonicalPath.isEmpty() && !coreappdata()->app_libpaths->contains(canonicalPath)) { coreappdata()->app_libpaths->prepend(canonicalPath); + locker.unlock(); QFactoryLoader::refreshAll(); } } -- cgit v0.12 From 3ee89bc0830f69d44f272eff5a0c886bff33c92e Mon Sep 17 00:00:00 2001 From: Alexis Menard Date: Thu, 12 Aug 2010 14:31:24 +0200 Subject: Properly emit geometryChanged() when the position change. Also emit the signal at the very end, so people can rely on the resize event to adjust some stuff in their item. Reviewed-by:yoann --- src/gui/graphicsview/qgraphicswidget.cpp | 3 ++- tests/auto/qgraphicswidget/tst_qgraphicswidget.cpp | 20 +++++++++++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/gui/graphicsview/qgraphicswidget.cpp b/src/gui/graphicsview/qgraphicswidget.cpp index c486c45..0fabd18 100644 --- a/src/gui/graphicsview/qgraphicswidget.cpp +++ b/src/gui/graphicsview/qgraphicswidget.cpp @@ -385,12 +385,12 @@ void QGraphicsWidget::setGeometry(const QRectF &rect) if (wd->inSetPos) { //set the new pos d->geom.moveTopLeft(pos()); + emit geometryChanged(); return; } } QSizeF oldSize = size(); QGraphicsLayoutItem::setGeometry(newGeom); - emit geometryChanged(); // Send resize event bool resized = newGeom.size() != oldSize; if (resized) { @@ -403,6 +403,7 @@ void QGraphicsWidget::setGeometry(const QRectF &rect) emit heightChanged(); QApplication::sendEvent(this, &re); } + emit geometryChanged(); } /*! diff --git a/tests/auto/qgraphicswidget/tst_qgraphicswidget.cpp b/tests/auto/qgraphicswidget/tst_qgraphicswidget.cpp index a771332..bda22eb 100644 --- a/tests/auto/qgraphicswidget/tst_qgraphicswidget.cpp +++ b/tests/auto/qgraphicswidget/tst_qgraphicswidget.cpp @@ -111,6 +111,7 @@ private slots: void fontPropagationSceneChange(); void geometry_data(); void geometry(); + void geometryChanged(); void width(); void height(); void getContentsMargins_data(); @@ -776,11 +777,28 @@ void tst_QGraphicsWidget::geometry() QFETCH(QSizeF, size); widget.setPos(pos); widget.resize(size); - if (!size.isNull()) + if (!size.isNull() && !pos.isNull()) + QCOMPARE(spy.count(), 2); + if (!size.isNull() && pos.isNull()) QCOMPARE(spy.count(), 1); QCOMPARE(widget.geometry(), QRectF(pos, size)); } +void tst_QGraphicsWidget::geometryChanged() +{ + QGraphicsWidget w; + w.setGeometry(0, 0, 200, 200); + QCOMPARE(w.geometry(), QRectF(0, 0, 200, 200)); + QSignalSpy spy(&w, SIGNAL(geometryChanged())); + w.setGeometry(0, 0, 100, 100); + QCOMPARE(spy.count(), 1); + QCOMPARE(w.geometry(), QRectF(0, 0, 100, 100)); + w.setPos(10, 10); + QCOMPARE(spy.count(), 2); + QCOMPARE(w.geometry(), QRectF(10, 10, 100, 100)); + +} + void tst_QGraphicsWidget::width() { QGraphicsWidget w; -- cgit v0.12