[Kde-games-devel] Review Request: Bug 304362 KsirK crashes on "new game" after one is already finished
Gael de Chalendar
kleag at free.fr
Wed Oct 10 20:51:01 UTC 2012
> On Oct. 10, 2012, 8:45 p.m., Gael de Chalendar wrote:
> > 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.
Nemanja, if you have write access, feel free to commit it yourself. I'll maybe not have time before a few days.
- Gael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/7045/#review10973
-----------------------------------------------------------
On Sept. 23, 2012, 11:03 p.m., Nemanja Hirsl wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/7045/
> -----------------------------------------------------------
>
> (Updated Sept. 23, 2012, 11:03 p.m.)
>
>
> Review request for KDE Games.
>
>
> Description
> -------
>
> 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
>
>
> Diffs
> -----
>
> /trunk/KDE/kdegames/ksirk/ksirk/GameLogic/gameautomaton.cpp 1309567
>
> Diff: http://svn.reviewboard.kde.org/r/7045/diff/
>
>
> Testing
> -------
>
> 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.
>
>
> Thanks,
>
> Nemanja Hirsl
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-games-devel/attachments/20121010/f0c4a151/attachment.html>
More information about the kde-games-devel
mailing list