<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://svn.reviewboard.kde.org/r/7045/">http://svn.reviewboard.kde.org/r/7045/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 10th, 2012, 8:45 p.m., <b>Gael de Chalendar</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This time, I was not able to reproduce the problem but I know it and I was not able to find a solution for years. Your analysis and your patch seem soon. It tested the patch version with no problem. I'll apply it soon.</pre>
</blockquote>
<p>On October 10th, 2012, 8:51 p.m., <b>Gael de Chalendar</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Nemanja, if you have write access, feel free to commit it yourself. I'll maybe not have time before a few days.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Gael, unfortunately I don't have write access (still); so if you can, please do commit changes these days. If not, Albert said he'll try to finish it in one month time frame. </pre>
<br />
<p>- Nemanja</p>
<br />
<p>On September 23rd, 2012, 11:03 p.m., Nemanja Hirsl wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Games.</div>
<div>By Nemanja Hirsl.</div>
<p style="color: grey;"><i>Updated Sept. 23, 2012, 11:03 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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);
I've created simple example to demonstrate what exactly is happening in the
code. Take a look at http://pastebin.com/KcyfenQc for details about example.
Useful link is also http://stackoverflow.com/questions/8657244/does-qts-qlist-
save-iterators-after-containers-modifications
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Tests performed:
1. New local game with 5 players (1 human - Baudouin, 4 AI)
1.1 During AI turn start a new game.
1.2 In original code crash happens. Debug traces:
ksirk(24791) Ksirk::KGameWindow::actionNewGame: valid
ksirk(24791)/libkdegames (KGame) KGame::setGameStatus: : GAMESTATUS CHANGED to 3
ksirk(24791) Ksirk::GameLogic::GameAutomaton::slotPropertyChanged: 6 (skin is 28672 )
ksirk(24791) Ksirk::GameLogic::GameAutomaton::slotPropertyChanged: END GameAutomaton::slotPropertyChanged 6 (skin is 28672 )
ksirk(24791) Ksirk::GameLogic::GameAutomaton::removeAllPlayers:
ksirk(24791)/libkdegames (KGame) KPlayer::~KPlayer: : this= KPlayer(0x3224b90) , id= 1025
ksirk(24791)/libkdegames (KGame) KGameIO::~KGameIO: : this= KGameIO(0x3225a00)
ksirk(24791)/libkdegames (KGame) KPlayer::removeGameIO: : KGameIO(0x3225a00) delete= false
ksirk(24791)/libkdegames (KGame) KGame::playerDeleted: : id ( 1025 ) to be removed KPlayer(0x3224b90)
ksirk(24791)/libkdegames (KGame) KGame::systemRemovePlayer:
ksirk(24791)/libkdegames (KGame) KGame::systemRemove: : Player ( 1025 ) to be removed KPlayer(0x3224b90)
ksirk(24791)/libkdegames (KGame) KGame::playerDeleted: : sending IdRemovePlayer 1025
ksirk(24791)/libkdegames (KGame) KPlayer::~KPlayer: done
ksirk(24791) Ksirk::GameLogic::AIPlayer::~AIPlayer: "Victor Emmanuel"
ksirk(24791)/libkdegames (KGame) KPlayer::~KPlayer: : this= KPlayer(0x3227e10) , id= 1027
ksirk(24791)/libkdegames (KGame) KGameIO::~KGameIO: : this= KGameIO(0x3ca81b0)
ksirk(24791)/libkdegames (KGame) KPlayer::removeGameIO: : KGameIO(0x3ca81b0) delete= false
ksirk(24791)/libkdegames (KGame) KGame::playerDeleted: : id ( 1027 ) to be removed KPlayer(0x3227e10)
ksirk(24791)/libkdegames (KGame) KGame::systemRemovePlayer:
ksirk(24791)/libkdegames (KGame) KGame::systemRemove: : Player ( 1027 ) to be removed KPlayer(0x3227e10)
ksirk(24791)/libkdegames (KGame) KGame::playerDeleted: : sending IdRemovePlayer 1027
ksirk(24791)/libkdegames (KGame) KPlayer::~KPlayer: done
ksirk(24791) Ksirk::GameLogic::AIPlayer::~AIPlayer: "Elisabeth"
ksirk(24791)/libkdegames (KGame) KPlayer::~KPlayer: : this= KPlayer(0x3179e30) , id= 1029
ksirk(24791)/libkdegames (KGame) KGameIO::~KGameIO: : this= KGameIO(0x317b6f0)
ksirk(24791)/libkdegames (KGame) KPlayer::removeGameIO: : KGameIO(0x317b6f0) delete= false
ksirk(24791)/libkdegames (KGame) KGame::playerDeleted: : id ( 1029 ) to be removed KPlayer(0x3179e30)
ksirk(24791)/libkdegames (KGame) KGame::systemRemovePlayer:
ksirk(24791)/libkdegames (KGame) KGame::systemRemove: : Player ( 1029 ) to be removed KPlayer(0x3179e30)
ksirk(24791)/libkdegames (KGame) KGame::playerDeleted: : sending IdRemovePlayer 1029
ksirk(24791)/libkdegames (KGame) KPlayer::~KPlayer: done
KCrash: Application 'ksirk' crashing...
Notice how order of AI Players is messed up. Order of players is shown below where first name (human - Baudouin was not printed)
1.3 After applying this patch debug crash is prevented and debug traces are:
ksirk(24998) Ksirk::KGameWindow::actionNewGame: valid
ksirk(24998)/libkdegames (KGame) KGame::setGameStatus: : GAMESTATUS CHANGED to 3
ksirk(24998) Ksirk::GameLogic::GameAutomaton::slotPropertyChanged: 6 (skin is 28672 )
ksirk(24998) Ksirk::GameLogic::GameAutomaton::slotPropertyChanged: END GameAutomaton::slotPropertyChanged 6 (skin is 28672 )
ksirk(24998) Ksirk::GameLogic::GameAutomaton::removeAllPlayers:
ksirk(24998)/libkdegames (KGame) KPlayer::~KPlayer: : this= KPlayer(0x2bfa110) , id= 1030
ksirk(24998)/libkdegames (KGame) KGameIO::~KGameIO: : this= KGameIO(0xa1c8f0)
ksirk(24998)/libkdegames (KGame) KPlayer::removeGameIO: : KGameIO(0xa1c8f0) delete= false
ksirk(24998)/libkdegames (KGame) KGame::playerDeleted: : id ( 1030 ) to be removed KPlayer(0x2bfa110)
ksirk(24998)/libkdegames (KGame) KGame::systemRemovePlayer:
ksirk(24998)/libkdegames (KGame) KGame::systemRemove: : Player ( 1030 ) to be removed KPlayer(0x2bfa110)
ksirk(24998)/libkdegames (KGame) KGame::playerDeleted: : sending IdRemovePlayer 1030
ksirk(24998)/libkdegames (KGame) KPlayer::~KPlayer: done
ksirk(24998) Ksirk::GameLogic::AIPlayer::~AIPlayer: "de Gaulle"
ksirk(24998)/libkdegames (KGame) KPlayer::~KPlayer: : this= KPlayer(0x29ffd10) , id= 1031
ksirk(24998)/libkdegames (KGame) KGameIO::~KGameIO: : this= KGameIO(0x2bf3b30)
ksirk(24998)/libkdegames (KGame) KPlayer::removeGameIO: : KGameIO(0x2bf3b30) delete= false
ksirk(24998)/libkdegames (KGame) KGame::playerDeleted: : id ( 1031 ) to be removed KPlayer(0x29ffd10)
ksirk(24998)/libkdegames (KGame) KGame::systemRemovePlayer:
ksirk(24998)/libkdegames (KGame) KGame::systemRemove: : Player ( 1031 ) to be removed KPlayer(0x29ffd10)
ksirk(24998)/libkdegames (KGame) KGame::playerDeleted: : sending IdRemovePlayer 1031
ksirk(24998)/libkdegames (KGame) KPlayer::~KPlayer: done
ksirk(24998) Ksirk::GameLogic::AIPlayer::~AIPlayer: "Victor Emmanuel"
ksirk(24998)/libkdegames (KGame) KPlayer::~KPlayer: : this= KPlayer(0x42c4020) , id= 1032
ksirk(24998)/libkdegames (KGame) KGameIO::~KGameIO: : this= KGameIO(0x35811d0)
ksirk(24998)/libkdegames (KGame) KPlayer::removeGameIO: : KGameIO(0x35811d0) delete= false
ksirk(24998)/libkdegames (KGame) KGame::playerDeleted: : id ( 1032 ) to be removed KPlayer(0x42c4020)
ksirk(24998)/libkdegames (KGame) KGame::systemRemovePlayer:
ksirk(24998)/libkdegames (KGame) KGame::systemRemove: : Player ( 1032 ) to be removed KPlayer(0x42c4020)
ksirk(24998)/libkdegames (KGame) KGame::playerDeleted: : sending IdRemovePlayer 1032
ksirk(24998)/libkdegames (KGame) KPlayer::~KPlayer: done
ksirk(24998) Ksirk::GameLogic::AIPlayer::~AIPlayer: "Akihito"
ksirk(24998)/libkdegames (KGame) KPlayer::~KPlayer: : this= KPlayer(0x357c400) , id= 1033
ksirk(24998)/libkdegames (KGame) KGameIO::~KGameIO: : this= KGameIO(0x3b16240)
ksirk(24998)/libkdegames (KGame) KPlayer::removeGameIO: : KGameIO(0x3b16240) delete= false
ksirk(24998)/libkdegames (KGame) KGame::playerDeleted: : id ( 1033 ) to be removed KPlayer(0x357c400)
ksirk(24998)/libkdegames (KGame) KGame::systemRemovePlayer:
ksirk(24998)/libkdegames (KGame) KGame::systemRemove: : Player ( 1033 ) to be removed KPlayer(0x357c400)
ksirk(24998)/libkdegames (KGame) KGame::playerDeleted: : sending IdRemovePlayer 1033
ksirk(24998)/libkdegames (KGame) KPlayer::~KPlayer: done
ksirk(24998) Ksirk::GameLogic::AIPlayer::~AIPlayer: "Elisabeth"
ksirk(24998)/libkdegames (KGame) KPlayer::~KPlayer: : this= KPlayer(0x2a730f0) , id= 1034
ksirk(24998)/libkdegames (KGame) KGameIO::~KGameIO: : this= KGameIO(0x2a27fd0)
ksirk(24998)/libkdegames (KGame) KPlayer::removeGameIO: : KGameIO(0x2a27fd0) delete= false
ksirk(24998)/libkdegames (KGame) KGame::playerDeleted: : id ( 1034 ) to be removed KPlayer(0x2a730f0)
ksirk(24998)/libkdegames (KGame) KGame::systemRemovePlayer:
ksirk(24998)/libkdegames (KGame) KGame::systemRemove: : Player ( 1034 ) to be removed KPlayer(0x2a730f0)
ksirk(24998)/libkdegames (KGame) KGame::playerDeleted: : sending IdRemovePlayer 1034
ksirk(24998)/libkdegames (KGame) KPlayer::~KPlayer: done
ksirk(24998) Ksirk::GameLogic::GameAutomaton::state: new state (id= 0 ) is INIT
Notice the correct order of all Players; all players removed correctly.
2. New standard TCP/IP game with patch applied
2.1 No issues noticed.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/KDE/kdegames/ksirk/ksirk/GameLogic/gameautomaton.cpp <span style="color: grey">(1309567)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/7045/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>