[Kde-games-devel] Unusual KGameRenderer crash

[Parker Coates]

Hello all,

As KDEGames 4.6 has started hitting distributions, I've been getting
reports of an odd crash in KPat
(http://bugs.kde.org/show_bug.cgi?id=271655). What makes it odd is
that:
   1. It typically only happens when logging out of a KDE session
while leaving a KPat window open.
   2. The backtrace shows KGameRenderer code deleting QGraphicsView.

It didn't take too much digging to learn that KGameRenderer deletes
all of its KGameRendererClients when destroyed. I later found this was
documented in the ~KGameRenderer() destructor, but I had been unaware
of this during my initial port to KGameRenderer. In KPat's case, View
inherits from both QGraphicsView and KGameRendererClient (for threaded
rendering of the background) and the KGameRenderer lives in a
K_GLOBAL_STATIC.

During a normal shutdown, this isn't an issue because the main window
and its children (including the View) are deleted when it is closed.
When the View is destroyed, the KGameRendererClient destructor
unregisters itself with the KGameRenderer. Later, when the
K_GLOBAL_STATIC is destroyed, the KGameRenderer has no remaining
clients, so none are destroyed and everything terminates as expected.

When the session manager closes KPat during logout, the procedure is
different. Signals are sent and (K|Q)Application signal handlers
handle them.The main window (and its children) are never actually
closed or deleted; they're just forgotten and freed on process
cleanup. But the K_GLOBAL_STATIC destructor does still happen, the
KGameRenderer is destructed and it destroys its clients including the
View, which results in a SEGFAULT. The exact reason for the crash
isn't known, but lots of QWidget operations are prohibited after qApp
is gone and destruction is most likely one of them.

So now that I've got all that explained, the next question is how to
fix it. :D Personally, I think the right answer is stop KGameRenderer
from deleting its clients. (Stefan, I'm guessing you'll agree, as this
seems to be the decision made in Tagaro::Sprite.) As far as I can
tell, this could be done in a binary compatible way by replacing
KGameRendererClient's KGameRenderer* with a
QWeakPointer<KGameRenderer> which is checked to be non-null before
using it. Of course, this would be a behaviour change that could
theoretically cause memory leaks in existing applications.

On the other hand, KGameRenderer hopefully won't be around that much
longer, so is this even worth fixing? Maybe I should just try to work
around the issue in KPat. Does the issue effect any of the other
KGameRenderer based games? Does anyone have any particularly good
suggestions on how to rework the KPat code?

Thanks,

Parker