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

Ian Wadham ianw2 at optusnet.com.au
Sun Sep 23 22:29:15 UTC 2012


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/7045/#review10964
-----------------------------------------------------------


Before qDeleteAll(list); came in, I think the recommended code was:

while (! list.isEmpty()) {
    delete list.takeFirst();
}

Would that simplify things in this case?

- Ian Wadham


On Sept. 23, 2012, 4:18 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, 4:18 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/20120923/e83c206c/attachment-0001.html>


More information about the kde-games-devel mailing list