[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-games-devel
Subject: [Kde-games-devel] Bug 304362 KsirK crashes on "new game" after one is already finished
From: Nemanja Hirsl <nemhirsl () gmail ! com>
Date: 2012-09-20 18:45:25
Message-ID: CAAmU1+RYBCQzhS7mGvAiP1Jv_5o+iyhPfy6xWevXxgw=G2m3ow () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
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@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@gmx.net, kleag@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
[Attachment #5 (text/html)]
Hi,<div><br></div><div>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.</div> <div>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. </div><div><br></div><div>Details are in forwarded \
email.</div><div> <br></div><div>Thanks in advance.</div><div><br></div><div>Best \
regards,</div><div>Nemanja</div><div><br></div><div>---------- Forwarded message \
----------</div><div><div class="gmail_quote">From: <b class="gmail_sendername"></b> \
<span dir="ltr"><<a \
href="mailto:nemhirsl@gmail.com">nemhirsl@gmail.com</a>></span><br>
Date: Sun, Sep 16, 2012 at 9:53 PM<br>Subject: Bug 304362 KsirK crashes on "new \
game" after one is already finished<br>To: <a \
href="mailto:majewsky@gmx.net">majewsky@gmx.net</a>, <a \
href="mailto:kleag@free.fr">kleag@free.fr</a><br> <br><br>Hi Stefan, Gael,<br>
<br>
I'm new to KDE and this is my first attempt to contribute to any project. I would \
really appreciate any feedback from you.<br> <br>
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).<br>
<br>
First, I would like to list all steps I did before I found possible solution:<br>
1. I've pulled the code from git and successfully built it.<br>
2. Enabled kdebug from kdebugdialog<br>
3. run ksirk in gdb<br>
4. Analyzed backtrace shown here: <a \
href="https://bugs.kde.org/show_bug.cgi?id=304362" \
target="_blank">https://bugs.kde.org/show_bug.cgi?id=304362</a><br> 5. Added some new \
kdebug printouts to make sure list iterators are OK:<br> In \
GameAutomaton::removeAllPlayers all list items (KPlayer*) are OK before qDeleteAll \
call.<br> 6. Code analysis:<br>
- PlayerList is list of KPlayer*;<br>
- Iterators became messed up in qDeleteAll (still crashing);<br>
- pulled out deletion from qDeleteAll to my own loop (still crashing);<br>
- 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 !<br>
<br>
In KPlayer::~KPlayer() see game()->playerDeleted(this);<br>
In KGame::playerDeleted(KPlayer *player) see systemRemovePlayer(player,false);<br>
in KGame::systemRemovePlayer(KPlayer* player,bool deleteit) see \
systemRemove(player,deleteit);<br> Finally result = \
d->mPlayerList.removeAll(p);<br> <br>
***<br>
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.<br>
***<br>
<br>
Change I found in git on 2010-07-03 17:41:42 Refactoring: Simplify unnecessarily \
complicated deletion statements.<br> <br>
<br>
There are also two more bugs (should be duplicates of each other): <a \
href="https://bugs.kde.org/show_bug.cgi?id=305000" \
target="_blank">https://bugs.kde.org/show_bug.cgi?id=305000</a> andhttps://<a \
href="http://bugs.kde.org/show_bug.cgi?id=303142" \
target="_blank">bugs.kde.org/show_bug.cgi?id=303142</a><br>
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?<br>
<br>
<br>
To sum up my questions:<br>
Do you agree with proposed solution? If not, what should I change?<br>
Can this be applied to bug in libkdegames?<br>
<br>
Again, I would really appreciate any feedback.<br>
Thank you for your time and help.<br>
<br>
Best regards,<br>
Nemanja</div><br></div>
--20cf3079b5eac0fbb004ca268610--
["gameautomaton.diff" (text/x-patch)]
diff --git a/ksirk/GameLogic/gameautomaton.cpp b/ksirk/GameLogic/gameautomaton.cpp
index ba4b3ca..a843f9d 100644
--- a/ksirk/GameLogic/gameautomaton.cpp
+++ b/ksirk/GameLogic/gameautomaton.cpp
@@ -2638,8 +2638,19 @@ void GameAutomaton::removeAllPlayers()
{
kDebug();
m_currentPlayer = "";
- qDeleteAll(*playerList());
+ /* Bug 304362. KPlayer destructor removes
+ * player from the list and makes iterators invalid.
+ * qDeleteAll crashes in that case. */
+ int count = playerList()->count();
+ PlayersArray::iterator it = playerList()->begin();
+ while (it != playerList()->end())
+ {
+ delete (*it);
+ (playerList()->count() != count) ? it = playerList()->begin() : it++;
+ }
playerList()->clear();
+ //qDeleteAll(*playerList());
+ //playerList()->clear();
}
void GameAutomaton::newGameNext()
_______________________________________________
kde-games-devel mailing list
kde-games-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-games-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic