Palapeli crashes (was Palapeli and kdelibs4)

[Ian Wadham]

On 01/09/2017, at 7:10 AM, Christian Ehrlicher wrote:

> Am 31.08.2017 um 20:49 schrieb Luigi Toscano:
>> Luigi Toscano wrote:
>>> When those issues are solved, do you see other blockers for merging frameworks
>>> into master?
>> Is the answer "no blocker" and we can do the merge dance? Apart from the merge
>> itself, the procedure includes also fixing the branches in kde-build-metadata
>> and sysadmin/repo-metadata and moving the translations. I can do all or part
>> of it (for sure I will move the translations :)
> I tried to reproduce some of the crashes but got no luck (apart from one, see D7637). But those are not KF5 porting bugs so I would say that there are no more blockers.

Regarding https://bugs.kde.org/show_bug.cgi?id=336075 and Phabricator D7637,
great work Christian!!!

I had a look at some of these crashes back in KDE 4 days, but it is indeed hard to
reproduce any of them - except for bug 336075, the one where you restart the puzzle
while dragging a piece.

FWIW, my theory is that the crashes are (all?) caused by deleting a piece-object
while it is being operated on by an interactor.  How so?  Obviously all pieces get
deleted and re-created when you restart the puzzle, so that is one way.  Another,
from what I remember (bearing in mind that I am getting old :-)) is when two pieces
are joined or merged (i.e. the user takes one step in solving the puzzle).  At that time a
new composite piece-object is created and the component piece-objects are deleted,
or so I understand.

If you move the pieces to be merged close together, but not quite touching, then let
go of the mouse button, Palapeli drags them together automatically and merges them.
This is the normal user-action and there is then no problem: the move-interactor has
terminated before the merge.

However, if the move-interactor is still busy when the merge occurs, there is code that
is supposed to take care of this by signalling the replacement piece to the interactor,
using the replaced pieces as senders.  I am not convinced that the relevant code
works in all cases and I think this is where the crashes may lie.  Maybe some sort of
race condition internally in Palapeli or maybe some user-action that somehow re-
selects one of the merging or merged pieces at an awkward time.

The code I have been looking at is in palapeli/src/engine, files mergegroup.cpp,
interactors.cpp and piece.cpp, in particular the methods
      void Palapeli::MergeGroup::createMergedPiece()
      void Palapeli::Piece::announceReplaced(Palapeli::Piece* replacement)
      void Palapeli::MovePieceInteractor::pieceReplacedBy(Palapeli::Piece* replacement)

After all that is said, the crashes in Palapeli do not have a huge impact on the user,
because Palapeli saves its state to disk about every two seconds.

However the crashes are not a good look, especially if the user has just moved the last
piece into place and completed the puzzle… :-)

Hope this helps,
Cheers, Ian W.

> If you could do the merge it would be nice - I only did it once some months ago ... :)
> 
> Thx,
> Christian
>