[Kde-games-devel] kpat issues (in recent svn)

[Parker Coates]

On Fri, May 8, 2009 at 2:30 PM, Parker Coates wrote:
> On Fri, May 8, 2009 at 11:34 AM, Parker Coates wrote:
>> On Thu, May 7, 2009 at 1:59 PM, Matthew Woehlke wrote:
>>> Anyone else notice that Grandfather's Clock crashes kpat? This has been
>>> going on for several days, and is happening with recent SVN (my last
>>> update was Tuesday or later).
>>
>> Seems to work fine here. I've played a couple games by hand and the
>> demo's played a half dozen with no anomalies What are you doing to
>> cause the crash? Do you have a back trace?
>
> Okay, I am able to reproduce at will. It occurs when launching a game
> of Grandfather's Clock if the window size is such that the cards
> aren't already in the cache at the correct size.
>
> The crash occurs in ClockSolver::translate_layout() when called from
> DealerScene::slotSolverEnded(). My guess is that there is a race
> condition between the solver thread and the cards actually being added
> to the piles on game start. I have a simple patch to
> ClockSolver::translate_layout() that stops the crash and seems to work
> fine, but I'm hesitant to commit, because it only treats the symptom
> of what appears to be a larger issue.
>
> The second question is why this issue only started up recently. My
> best guess is that it's due to the new Ancient Egyptians deck. Due to
> it's detail, it takes longer to render, and so a race condition that
> was never an issue before is now being exposed. On my machine, I can't
> recreate the issue with any other deck. On a faster machine, this
> might not be an issue at all.
>
> I'm going to look into it, but I don't have much experience with
> threading. My guess is that the solver thread is being started before
> the scene is actually ready.

So I didn't have much luck sorting out the route issue. The timer that
restarts the solver is started from far too many places for me to be
able to prevent the race condition.

So instead, I am committing the following patch:

     for (int i = 0; i < 12; i++)
     {
         Card *c = deal->target[i]->top();
-        Q_ASSERT( c );

-        W[8][i] = translateSuit( c->suit() ) + c->rank();
+        // It is not safe to assume that each target pile will always have a
+        // card on it. If it takes particularly long to render the
card graphics
+        // at the start of a new game, it is possible that this method can be
+        // called before the initial deal has been completed.
+        if (c)
+            W[8][i] = translateSuit( c->suit() ) + c->rank();
     }

I have tested it a bit, and I haven't seen any regressions.

Parker