[Kde-games-devel] Bug 304362 KsirK crashes on "new game" after one is already finished

[Nemanja Hirsl]

Hi,

I would like to get involved in KDE development especially in kdegames. Not
so long ago I opened a bug (from subject) for ksirk game and now I believe
I have a possible solution. Since this is my first try to contribute to KDE
I would really appreciate any feedback.
Probably Stefan and Gael are busy and don't have time to answer some of my
questions, so I'm forwarding my email to the kde-games-devel list.

Details are in forwarded email.

Thanks in advance.

Best regards,
Nemanja

---------- Forwarded message ----------
From: <nemhirsl at gmail.com>
Date: Sun, Sep 16, 2012 at 9:53 PM
Subject: Bug 304362 KsirK crashes on "new game" after one is already
finished
To: majewsky at gmx.net, kleag at free.fr


Hi Stefan, Gael,

I'm new to KDE and this is my first attempt to contribute to any project. I
would really appreciate any feedback from you.

Recently, I've opened a bug (from the subject) and now, after a couple of
days of playing with the code and analyzing various situations, I've came
up with possible solution (attached, see below for explanation).

First, I would like to list all steps I did before I found possible
solution:
1. I've pulled the code from git and successfully built it.
2. Enabled kdebug from kdebugdialog
3. run ksirk in gdb
4. Analyzed backtrace shown here:
https://bugs.kde.org/show_bug.cgi?id=304362
5. Added some new kdebug printouts to make sure list iterators are OK:
In GameAutomaton::removeAllPlayers all list items (KPlayer*) are OK before
qDeleteAll call.
6. Code analysis:
 - PlayerList is list of KPlayer*;
 - Iterators became messed up in qDeleteAll (still crashing);
 - pulled out deletion from qDeleteAll to my own loop (still crashing);
 - KPlayer destructor analyzed - When object is deleted, it get pulled out
from the game (playerList), modifying original playerList. This is the
problem because iterators to a QList became invalid after any (insertion
or) removal !

In KPlayer::~KPlayer() see  game()->playerDeleted(this);
In  KGame::playerDeleted(KPlayer *player) see
systemRemovePlayer(player,false);
in KGame::systemRemovePlayer(KPlayer* player,bool deleteit) see
 systemRemove(player,deleteit);
Finally result = d->mPlayerList.removeAll(p);

***
To prevent this crash qDeleteAll should not be called for lists modified in
destructors (like in case of Kplayer). The solution I propose is to keep
all possible cases working.
***

Change I found in git on 2010-07-03 17:41:42 Refactoring: Simplify
unnecessarily complicated deletion statements.


There are also two more bugs (should be duplicates of each other):
https://bugs.kde.org/show_bug.cgi?id=305000 andhttps://
bugs.kde.org/show_bug.cgi?id=303142
The reason for this crash is the same: qDeleteAll is called on Kplayer*
list, but this time in libkdegames: void KGame::deletePlayers(). There I
saw comment Stefan left, but maybe the same solution could be applied in
this case?


To sum up my questions:
Do you agree with proposed solution? If not, what should I change?
Can this be applied to bug in libkdegames?

Again, I would really appreciate any feedback.
Thank you for your time and help.

Best regards,
Nemanja
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-games-devel/attachments/20120920/af0b6443/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gameautomaton.diff
Type: text/x-patch
Size: 854 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-games-devel/attachments/20120920/af0b6443/attachment.diff>