diff options
author | Samuel Rødal <samuel.rodal@nokia.com> | 2010-11-08 09:05:28 (GMT) |
---|---|---|
committer | Samuel Rødal <samuel.rodal@nokia.com> | 2010-11-08 12:03:51 (GMT) |
commit | ee958e086847e2131f9b746e5c6de4491ae1f44b (patch) | |
tree | 7396429debac0cc0074fd202c6af4bad25429c8b | |
parent | e600496d74c607f2316dd66f117a3d693769cca2 (diff) | |
download | Qt-ee958e086847e2131f9b746e5c6de4491ae1f44b.zip Qt-ee958e086847e2131f9b746e5c6de4491ae1f44b.tar.gz Qt-ee958e086847e2131f9b746e5c6de4491ae1f44b.tar.bz2 |
Prevented race condition on texture destruction.
When texture destruction was triggered from a different thread, we
posted a signal to the QGLSignalProxy. However, before the signal is
delivered the corresponding QGLContext might be destroyed in the main
thread. We need to post a signal to a QObject which is destroyed when
the QGLContext is destroyed instead, to prevent trying to access an
invalid QGLContext pointer.
Task-number: QT-4238
Reviewed-by: Gunnar Sletta
-rw-r--r-- | src/opengl/qgl.cpp | 11 | ||||
-rw-r--r-- | src/opengl/qgl.h | 1 | ||||
-rw-r--r-- | src/opengl/qgl_p.h | 30 | ||||
-rw-r--r-- | tests/auto/qgl/tst_qgl.cpp | 8 |
4 files changed, 38 insertions, 12 deletions
diff --git a/src/opengl/qgl.cpp b/src/opengl/qgl.cpp index 84cfa97..ed9753e 100644 --- a/src/opengl/qgl.cpp +++ b/src/opengl/qgl.cpp @@ -1642,12 +1642,23 @@ const QGLContext *qt_gl_transfer_context(const QGLContext *ctx) return 0; } +QGLContextPrivate::QGLContextPrivate(QGLContext *context) + : internal_context(false) + , q_ptr(context) +{ + group = new QGLContextGroup(context); + texture_destroyer = new QGLTextureDestroyer; + texture_destroyer->moveToThread(qApp->thread()); +} + QGLContextPrivate::~QGLContextPrivate() { if (!group->m_refs.deref()) { Q_ASSERT(group->context() == q_ptr); delete group; } + + delete texture_destroyer; } void QGLContextPrivate::init(QPaintDevice *dev, const QGLFormat &format) diff --git a/src/opengl/qgl.h b/src/opengl/qgl.h index f85cad5..9ae619d 100644 --- a/src/opengl/qgl.h +++ b/src/opengl/qgl.h @@ -428,6 +428,7 @@ private: friend class QGLSharedResourceGuard; friend class QGLPixmapBlurFilter; friend class QGLExtensions; + friend class QGLTexture; friend QGLFormat::OpenGLVersionFlags QGLFormat::openGLVersionFlags(); #ifdef Q_WS_MAC public: diff --git a/src/opengl/qgl_p.h b/src/opengl/qgl_p.h index 18b2765..4742bdb 100644 --- a/src/opengl/qgl_p.h +++ b/src/opengl/qgl_p.h @@ -317,6 +317,7 @@ private: }; class QGLTexture; +class QGLTextureDestroyer; // This probably needs to grow to GL_MAX_VERTEX_ATTRIBS, but 3 is ok for now as that's // all the GL2 engine uses: @@ -326,7 +327,7 @@ class QGLContextPrivate { Q_DECLARE_PUBLIC(QGLContext) public: - explicit QGLContextPrivate(QGLContext *context) : internal_context(false), q_ptr(context) {group = new QGLContextGroup(context);} + explicit QGLContextPrivate(QGLContext *context); ~QGLContextPrivate(); QGLTexture *bindTexture(const QImage &image, GLenum target, GLint format, QGLContext::BindOptions options); @@ -417,6 +418,7 @@ public: GLuint current_fbo; GLuint default_fbo; QPaintEngine *active_engine; + QGLTextureDestroyer *texture_destroyer; bool vertexAttributeArraysEnabledState[QT_GL_VERTEX_ARRAY_TRACKED_COUNT]; @@ -476,25 +478,20 @@ private: QGLContext *m_ctx; }; -// ### make QGLContext a QObject in 5.0 and remove the proxy stuff -class Q_OPENGL_EXPORT QGLSignalProxy : public QObject +class QGLTextureDestroyer : public QObject { Q_OBJECT public: - QGLSignalProxy() : QObject() { + QGLTextureDestroyer() : QObject() { qRegisterMetaType<GLuint>("GLuint"); connect(this, SIGNAL(freeTexture(QGLContext *, QPixmapData *, GLuint)), this, SLOT(freeTexture_slot(QGLContext *, QPixmapData *, GLuint))); } - void emitAboutToDestroyContext(const QGLContext *context) { - emit aboutToDestroyContext(context); - } void emitFreeTexture(QGLContext *context, QPixmapData *boundPixmap, GLuint id) { emit freeTexture(context, boundPixmap, id); } - static QGLSignalProxy *instance(); + Q_SIGNALS: - void aboutToDestroyContext(const QGLContext *context); void freeTexture(QGLContext *context, QPixmapData *boundPixmap, GLuint id); private slots: @@ -518,6 +515,19 @@ private slots: } }; +// ### make QGLContext a QObject in 5.0 and remove the proxy stuff +class Q_OPENGL_EXPORT QGLSignalProxy : public QObject +{ + Q_OBJECT +public: + void emitAboutToDestroyContext(const QGLContext *context) { + emit aboutToDestroyContext(context); + } + static QGLSignalProxy *instance(); +Q_SIGNALS: + void aboutToDestroyContext(const QGLContext *context); +}; + class QGLTexture { public: QGLTexture(QGLContext *ctx = 0, GLuint tx_id = 0, GLenum tx_target = GL_TEXTURE_2D, @@ -537,7 +547,7 @@ public: #if !defined(Q_WS_X11) QPixmapData *boundPixmap = 0; #endif - QGLSignalProxy::instance()->emitFreeTexture(context, boundPixmap, id); + context->d_ptr->texture_destroyer->emitFreeTexture(context, boundPixmap, id); } } diff --git a/tests/auto/qgl/tst_qgl.cpp b/tests/auto/qgl/tst_qgl.cpp index 4220b45..e38bf42 100644 --- a/tests/auto/qgl/tst_qgl.cpp +++ b/tests/auto/qgl/tst_qgl.cpp @@ -2335,8 +2335,10 @@ public: Widget() : iterations(0) , display(0) + , producer(new Producer) { startTimer(400); + connect(this, SIGNAL(destroyed()), producer, SLOT(deleteLater())); } int iterations; @@ -2348,15 +2350,15 @@ protected: delete display; display = new DisplayWidget(this); - connect(&producer, SIGNAL(imageReady(const QImage &)), display, SLOT(setImage(const QImage &))); + connect(producer, SIGNAL(imageReady(const QImage &)), display, SLOT(setImage(const QImage &))); display->setGeometry(rect()); display->show(); } private: - Producer producer; DisplayWidget *display; + Producer *producer; }; } @@ -2369,6 +2371,8 @@ void tst_QGL::threadImages() while (widget->iterations <= 5) { qApp->processEvents(); } + + delete widget; } class tst_QGLDummy : public QObject |