Re: Regression in Frameworks - DBus Hangs

2018-11-12 Thread David Edmundson
I don't think application CI jobs run with workspace available. I also
can't think of anything that's been merged in workspace.

---
quitOnLastWindowClosed is also blocked if a QEventLoopLocker is placed on
the main application.

All KJobs do this and a Kjob existing and hanging seems plausible, but
that's a lot harder to grep for.

All stdout/stderr of our newly spawned konsole should be forwarded to the
test.
@Ben is that available?
Ideally with QT_LOGGING_RULES=*.debug=true


Re: Regression in Frameworks - DBus Hangs

2018-11-12 Thread Michael Pyne
On Mon, Nov 12, 2018 at 10:52:17PM +1300, Ben Cooksley wrote:
> In this instance I hoped someone would know of a behaviour change they
> introduced around D-Bus or application shutdown stuff, particularly
> given the time window i'd provided.
> 
> >
> > I imagine the majority of people on this list is in the same situation. I 
> > don't know what can be realistically expected.
> > It's quite a jump to go from that to saying people don't check emails.
> >
> > David
> 
> Based on this i've run some additional diagnostics on a CI worker this
> evening, and based on my reading of the test code it would appear that
> the culprit is Konsole no longer exiting when it's last window is
> closed. Konsole's code didn't change at all between the test working
> and it breaking, which indicates this is a Frameworks regression.
> 
> The test does this:
> 
> QDBusInterface iface(_interfaceName,
>  QStringLiteral("/konsole/MainWindow_1"),
>  QStringLiteral("org.qtproject.Qt.QWidget"));
> QVERIFY2(iface.isValid(), "Unable to get a dbus interface to Konsole!");
> 
> QDBusReply instanceReply = iface.call(QStringLiteral("close"));
> 
> Which indeed results in the Window closing, at least as far as D-Bus
> is concerned:
> 
> jenkins@0f9cdb19eeb8:/home/jenkins/konsole> qdbus org.kde.konsole-3065
> /
> /KBookmarkManager
> /KBookmarkManager/konsole
> /MainApplication
> /org
> /org/kde
> /org/kde/konsole
> 
> I also confirmed this by taking a screenshot of the Xvfb instance
> being used by this session, which showed nothing but black -
> confirming the window was indeed closed.
> 
> Instructing Konsole to close by running "qdbus org.kde.konsole-3065
> /MainApplication quit" caused the test to finish (as expected).
> 
> Thoughts anyone?

There is a Qt property on QApplication which is devoted to precisely
this feature. The property, "quitOnLastWindowClosed" will cause the
application to automatically quit if the last window has been closed.

It dates back to Qt 4 and a cursory search of qtbase, KF5 and
kde/workspace for any commits that may have changed the default for this
property [1] don't show any recent changes (disabling it when it was
previously enabled by default or vice versa).

Manually enabling the property in konsole might be enough to make the
test pass. That wouldn't explain what the "regression" was but it seems
strictly more correct either way.

If it's a recent change then it may be related to distro
changes to Qt, something to do with Plasma integration (which is why I
searched in kde/workspace), or some other aspect of the way KF5-based
applications configure themselves by default based on the settings of
the desktop they are being run within.

The only reference of any sort that I see in either KF5 or kde/workspace
for "quitOnLastWindowClosed" is in kxmlgui (for KMainWindow). There
aren't many recent commits so anyone who can reproduce the hang should
have a quick job verifying if reverting some of those would help or not.

[1] Using git -P log --since="8 weeks ago" -G quitOnLastWindow --oneline
for each git repository

Regards,
 - Michael Pyne


Re: Regression in Frameworks - DBus Hangs

2018-11-12 Thread Ben Cooksley
On Mon, Nov 12, 2018 at 7:27 AM David Edmundson
 wrote:
>
> I ran the test locally which passed, then left it for someone else as:
>  - I can't reproduce locally.
>  - There are absolutely no logs on build.k.o to give a clue.
>  - I have no access to build.k.o to do anything (AFAIK).
>  - Even the phab ticket cited in the commit that disables the test is not 
> accessible to the public.

I see. Had I received a response saying "I can't reproduce this" then
we could have gone further in this.
>From my perspective though it looks like nobody has even looked into
this at all as i've no way of knowing if people have read it or tried
to do something about it.
Knowing that others can't reproduce it locally is important and makes
quite the difference.

In this instance I hoped someone would know of a behaviour change they
introduced around D-Bus or application shutdown stuff, particularly
given the time window i'd provided.

>
> I imagine the majority of people on this list is in the same situation. I 
> don't know what can be realistically expected.
> It's quite a jump to go from that to saying people don't check emails.
>
> David

Based on this i've run some additional diagnostics on a CI worker this
evening, and based on my reading of the test code it would appear that
the culprit is Konsole no longer exiting when it's last window is
closed. Konsole's code didn't change at all between the test working
and it breaking, which indicates this is a Frameworks regression.

The test does this:

QDBusInterface iface(_interfaceName,
 QStringLiteral("/konsole/MainWindow_1"),
 QStringLiteral("org.qtproject.Qt.QWidget"));
QVERIFY2(iface.isValid(), "Unable to get a dbus interface to Konsole!");

QDBusReply instanceReply = iface.call(QStringLiteral("close"));

Which indeed results in the Window closing, at least as far as D-Bus
is concerned:

jenkins@0f9cdb19eeb8:/home/jenkins/konsole> qdbus org.kde.konsole-3065
/
/KBookmarkManager
/KBookmarkManager/konsole
/MainApplication
/org
/org/kde
/org/kde/konsole

I also confirmed this by taking a screenshot of the Xvfb instance
being used by this session, which showed nothing but black -
confirming the window was indeed closed.

Instructing Konsole to close by running "qdbus org.kde.konsole-3065
/MainApplication quit" caused the test to finish (as expected).

Thoughts anyone?

Regards,
Ben