buggnubg - Bugs: bug #33781, Possible crash after rollout...

 
 

bug #33781: Possible crash after rollout started from the hint window

Submitted by:  Philippe Michel <plm>
Submitted on:  Sat 16 Jul 2011 11:25:15 AM UTC  
 
Originator Name: Philippe MichelItem Group: Crash error
Category: Graphical User InterfaceRelease: 0.90-devel
Privacy: PublicOperating System: All
OS Type: AnyProcessor Type: Any
Severity: 3 - NormalOpen/Closed: Closed
Assigned to: Michael Petch <mdpetch>Priority: 5 - Normal
Status: Fixed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Sun 21 Aug 2011 06:12:12 AM UTC, comment #6:

I have reproduced this with the cube rollouts. I wasn't selecting a cube that was offered and doing a hint from there. When I did this scenario, the hint window came up and experienced the same sort of assertions/crash.

I now make sure the that the widget used to initiate the rollout is still available when the rollout function returns. If it's not available then we don't attempt to update the hint window because it has been (or is in the process of) being destroyed.

Michael Petch <mdpetch>
Project AdministratorIn charge of this item.
Sun 21 Aug 2011 05:25:45 AM UTC, comment #5:

first one mistake in my previous posts. The Rollout functions for cubes and moves do not return whether the match was interrupted, only how many games were rolled out.

If we wished to retain the previous functionality (Although I'm not sure why) on Hint/Rollouts, then gtkchequer.c's assertions and crashes would be fixed by either:

a) Disabling the "close" and "rollout" button on the hint window while the Rollout Window is still visible.

b) Make all the rollout windows act entirely modal (parent windows will not accept focus and act disabled). This may require some effort reworking of the code.

c) Modify gtkchequer.c so that after the rollout is complete and before any hint window updates we call gtk_widget_is_drawable(pw) to see if our widget is still available. If it's not then the TREEVIEW (And data store) are not available either (The parent window is gone) and we should return immediately.

The reason for the assertions and crashes with existing code is that the parent window can be destroyed before we start cleaning up after returning from the roll out window - a process that involves updating a Treeview that may have already been destroyed. In MoveListRolloutClicked (gtkchequer.c) calls that require a hint TREEVIEW to exist before being called are:

MoveListUpdate
MoveListClearSelection
RefreshMoveList

If the Treeview does not exist we run into trouble.

The fix I have made prevents the parent from being destroyed too early and staying consistent with move rollouts done from the main move list panel. In this case the Treeview won't be prematurely destroyed and the functions above won't be a problem.

Regarding cube rollouts I can't seem to get assertions and or crashes at this time. I have also tried on FreeBSD/64bit/CLANG without any luck.

Michael Petch <mdpetch>
Project AdministratorIn charge of this item.
Sun 21 Aug 2011 12:36:45 AM UTC, comment #4:

The interesting thing about this is I also followed all the gtkcube.c code for comparison purposes. I was concerned originally that there was a problem in both places. On Linux, I was able to reproduce your crash and/or assertions with the chequer rollouts. The cube rollouts follow a different path. With cubes when you "stop" the data updates are deferred. The relevant code in gtkcube.c that cause this deferral are:

if ( GeneralCubeDecisionR ( aarOutput, aarStdDev, aarsStatistics,
(ConstTanBoard)pchd->ms.anBoard, &ci, &pes->rc, pes,
RolloutProgress, p ) < 0 ) {
RolloutProgressEnd( &p, FALSE );
return;
}

The IF statement on GeneralCubeDecisionR checks to see if the rollout was stopped by the user (Stop/Stopall return -1/-2). They call RollOutProgressEnd and do not do any processing until the next GTK event (by calling return). One of the other solutions to this problem involved doing a similar short circuit in gtkchequer.c .

In gtkchecker.c this check is not done, but of course there is no statistics panel.

I have tried even with the statistics panel up to close a window underneath to generate assertions and I have been unable to do so.

If you can provide exact steps for producing assertions with the cube I'd appreciate it. It may well be that this could be due to an OS/GTK version difference as well.

Th only anomaly I noticed is that with cubes once the rollout window is up and it is stopped that the rollout button is not greyed out on the main move record panel allowing you to hit the rollout button again. This could be solved with some well placed calls to gtk_widget_set_sensitive .

Michael Petch <mdpetch>
Project AdministratorIn charge of this item.
Sat 20 Aug 2011 08:40:36 PM UTC, comment #3:

Preventing a crash obviously takes priority, but there is still something done wrong in the hint panel.

I realize that my initial description was ambiguous. There are gtk assertions failures for both kind of rollouts. There was a crash only for checker plays, but maybe cube rollouts survive by luck, just like you couldn't reproduce the problem on Windows.

The assertions failures certainly indicate that something improper is done that shouldn't even have been allowed, and closing the rollout window immediately would be annoying in the case of cube rollouts since the extended statistics would then become unavailable.

(file #23842)

Philippe Michel <plm>
Project Member
Sat 20 Aug 2011 03:20:26 PM UTC, comment #2:

There are a number of ways to fix this. One fix (The approach I took) keeps the UI consistent with chequer play rollouts done from the "game record" pane in the move list. When the rollout completes for a chequer play or is "Stop"ped by the user, close the rollout window, and update the parent window with rollout results. This prevents this bug from occurring as a side effect.

Michael Petch <mdpetch>
Project AdministratorIn charge of this item.
Sat 20 Aug 2011 11:23:30 AM UTC, comment #1:

Just a note. This seems to be a problem on *nix/BSD builds but isn't reproducible on WIN32.

Michael Petch <mdpetch>
Project AdministratorIn charge of this item.
Sat 16 Jul 2011 11:25:15 AM UTC, original submission:

After completing a rollout from the Hint window, it is possible to inadvertently close the Hint window before the one with the rollout result.

This causes a few assertions failure messages from gtk then a crash when the Rollout window is closed if the rollout was for a checker play.

Probably the Close button from the Hint window should be disabled until the Rollout window is closed.

Philippe Michel <plm>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #23842:  cube-assertion-failures.txt added by plm (495B - text/plain)
file #23661:  typescript added by plm (5KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by mdpetch (Updated the item)
  • -unavailable- added by plm (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 15 May 2012 03:32:55 AM UTCmdpetchOpen/ClosedOpen=>Closed
      StatusIn Progress=>Fixed
    Mon 22 Aug 2011 08:16:06 AM UTCmdpetchStatusConfirmed=>In Progress
    Sat 20 Aug 2011 08:40:36 PM UTCplmAttached File-=>Added cube-assertion-failures.txt, #23842
    Sat 20 Aug 2011 11:23:30 AM UTCmdpetchStatusNone=>Confirmed
    Fri 12 Aug 2011 11:57:44 PM UTCmdpetchAssigned toNone=>mdpetch
    Sat 16 Jul 2011 11:25:15 AM UTCplmAttached File-=>Added typescript, #23661

    Back to the top


    Powered by Savane 3.1-cleanup1