On Tue, Feb 18, 2014 at 5:53 PM, Alexey Borzenkov <[email protected]> wrote: > Hi, > > I've recently installed Kubuntu 13.10 (haven't tried it in a while) > and while looking at what's new in KDE almost immediately got some > plasma crashes. While researching how to best report this bug I found > numerous bug reports, which ultimately collapsed into single bug > #302931 marked resolved. So I tried trusty alpha 2, updated, but even > with qt 4.8.5 it still crashed for me. And it's quite easily > reproducible for me: > > - Install Kubuntu under VMWare Player > - Change resolution to 1280x800 (not all resolutions seem to trigger this > bug!) > - Click on "Show activities" and then "Add widgets" > > Starting with a stack trace, which shows the bug happening during > deferred destruction of QDeclarativeElement and then QDeclarativeItem, > I immediately recognized that this happens during traversion of item > change listeners. Long story short I started to look at kubuntu > patches and found that the only candidate is > kubuntu_97_a11y_qt_and_qml_backport.diff, since it seems to add a very > pervasive QDeclarativeAccessibilityUpdater to declarative items, but > the actual instance is located in QDeclarativeEnginePrivate. My hunch > is that QDeclarativeEngine gets destroyed before QDeclarativeItem that > uses its listener and then later when deferred destructor for > QDeclarativeItem kicks in the memory belonging to > QDeclarativeAccessibilityUpdater might be already filled with garbage > and hence the crash during vtable dereference. > > I confirmed my hunch by applying the attached patch, which uses a > single global QDeclarativeAccessibilityUpdater instance (it's not > pretty, but very short), which is possible because > QDeclarativeAccessibilityUpdater is basically stateless, all it does > is forwarding to QAccessible functions (btw, why does it even inherit > from QObject? It doesn't have to, not without all that commented > code!). > > Sure enough, after 9 hours of compiling (so I would have a patched and > simply recompiled kubuntu 13.10 libqt4-declarative packages) I see no > crashes with my patch and 100% reproducible crashes without my patch. > > If the patch is not acceptable (I'm not sure it won't trip valgrind or > something like that), what would be a good place for > QDeclarativeAccessibilityUpdate instance? One idea is > QDeclarativeItemPrivate, but wouldn't it be weird for QDeclarativeItem > to basically listen to itself? :) > > In case attachment doesn't go thru here's the patch inline (might get > garbled by gmail): > > Index: qt4-x11-4.8.4+dfsg/src/declarative/qml/qdeclarativeengine.cpp > =================================================================== > --- qt4-x11-4.8.4+dfsg.orig/src/declarative/qml/qdeclarativeengine.cpp > 2012-11-23 14:09:53.000000000 +0400 > +++ qt4-x11-4.8.4+dfsg/src/declarative/qml/qdeclarativeengine.cpp > 2014-02-18 15:43:05.510776633 +0400 > @@ -2555,4 +2555,10 @@ > return true; > } > > +QDeclarativeAccessibilityUpdater > *QDeclarativeEnginePrivate::getAccessibilityUpdater(QDeclarativeEngine > *e) > +{ > + static QDeclarativeAccessibilityUpdater accessibilityUpdater; > + return &accessibilityUpdater; > +} > + > QT_END_NAMESPACE > Index: qt4-x11-4.8.4+dfsg/src/declarative/qml/qdeclarativeengine_p.h > =================================================================== > --- qt4-x11-4.8.4+dfsg.orig/src/declarative/qml/qdeclarativeengine_p.h > 2014-02-18 00:21:38.000000000 +0400 > +++ qt4-x11-4.8.4+dfsg/src/declarative/qml/qdeclarativeengine_p.h > 2014-02-18 15:41:20.606780904 +0400 > @@ -238,8 +238,6 @@ > > mutable QMutex mutex; > > - QDeclarativeAccessibilityUpdater accessibilityUpdater; > - > QDeclarativeTypeLoader typeLoader; > QDeclarativeImportDatabase importDatabase; > > @@ -314,7 +312,7 @@ > static QScriptValue formatTime(QScriptContext*, QScriptEngine*); > static QScriptValue formatDateTime(QScriptContext*, QScriptEngine*); > #endif > - static QDeclarativeAccessibilityUpdater > *getAccessibilityUpdater(QDeclarativeEngine *e) { return > &e->d_func()->accessibilityUpdater; } > + static QDeclarativeAccessibilityUpdater > *getAccessibilityUpdater(QDeclarativeEngine *e); > static QScriptEngine *getScriptEngine(QDeclarativeEngine *e) { > return &e->d_func()->scriptEngine; } > static QDeclarativeEngine *getEngine(QScriptEngine *e) { return > static_cast<QDeclarativeScriptEngine*>(e)->p->q_func(); } > static QDeclarativeEnginePrivate *get(QDeclarativeEngine *e) { > return e->d_func(); } > > Whoa. It's really nice to see people get down and dirty with Qt internals.
The best place for these patches is https://codereview.qt-project.org/#mine. It's a bit intimidating but worth it: http://qt-project.org/wiki/Gerrit-Introduction There is probably going to be another Qt4.x at some point and the original developers normally know best. Making something static because of a race condition/double delete does seem like a bit of a bodge. You're now sharing the QDeclarativeAccessibilityUpdater between engines which from what you described would also be bad. Do you have the original backtrace? David > Best regards, > Alexey. > > -- > kubuntu-devel mailing list > [email protected] > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/kubuntu-devel > -- kubuntu-devel mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/kubuntu-devel
