After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 701601 - Undoing move while showing previous move causes a segmentation fault
Undoing move while showing previous move causes a segmentation fault
Status: RESOLVED FIXED
Product: gnome-chess
Classification: Applications
Component: General
git master
Other Linux
: Normal normal
: ---
Assigned To: gnome-chess-maint
gnome-chess-maint
Depends on:
Blocks:
 
 
Reported: 2013-06-04 15:32 UTC by Chris Cummins
Modified: 2013-06-06 12:46 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Always undo from most the most recent move (1.73 KB, patch)
2013-06-04 16:44 UTC, Chris Cummins 		reviewed Details | Review
Always undo from most the most recent move (v2) (1.54 KB, patch)
2013-06-05 10:45 UTC, Chris Cummins 		committed Details | Review
Always undo from most the most recent move (1.54 KB, patch)
2013-06-05 17:30 UTC, Michael Catanzaro 		none Details | Review

Description Chris Cummins 2013-06-04 15:32:46 UTC 
To reproduce:

1) Start a new game
2) Make a move
3) Press "Show previous move"
4) Select Game->"Undo Move"
5) Observe the application crash

The cause seems to be that the ChessScene.move_number does not update as required when undoing a move while showing the previous move. For example, pressing "Undo Move" after making a normal move results in:

scene.move_number: -1, game.n_moves: 1 /* Undo black move */
scene.move_number: -1, game.n_moves: 0 /* Undo white move */

However, after showing the previous move, the ChessCene.move_number becomes larger than the ChessGame.n_moves:

scene.move_number: 1, game.n_moves: 1 /* Undo black move */
scene.move_number: 1, game.n_moves: 0 /* Attempt to undo white move and fail */

Backtrace:

(gdb) bt

+ Trace 232013

  • #0 chess_game_get_piece
    at chess-game.c line 6178
  • #1 chess_scene_update_board
    at chess-scene.c line 1120
  • #2 chess_scene_undo_cb
    at chess-scene.c line 918
  • #3 _chess_scene_undo_cb_chess_game_undo
    at chess-scene.c line 1361 






Comment 1


Chris Cummins





          2013-06-04 15:34:27 UTC
        



(I didn't realize that bugzilla would collapse the stack trace for me)

Full backtrace:





+
Trace
    232014






    

  • 
#0
chess_game_get_piece

    
                at chess-game.c
                  line
                  6178

    

    • 

  • 
#1
chess_scene_update_board

    
                at chess-scene.c
                  line
                  1120

    

    • 

  • 
#2
chess_scene_undo_cb

    
                at chess-scene.c
                  line
                  918

    

    • 

  • 
#3
_chess_scene_undo_cb_chess_game_undo

    
                at chess-scene.c
                  line
                  1361

    

    • 

  • 
#4
g_cclosure_marshal_VOID__VOID

    
                at gmarshal.c
                  line
                  85

    

    • 

  • 
#5
g_closure_invoke

    
                at gclosure.c
                  line
                  777

    

    • 

  • 
#6
signal_emit_unlocked_R

    
                at gsignal.c
                  line
                  3584

    

    • 

  • 
#7
g_signal_emit_valist

    
                at gsignal.c
                  line
                  3328

    

    • 

  • 
#8
g_signal_emit_by_name

    
                at gsignal.c
                  line
                  3424

    

    • 

  • 
#9
chess_game_undo_cb

    
                at chess-game.c
                  line
                  6002

    

    • 

  • 
#10
_chess_game_undo_cb_chess_player_do_undo

    
                at chess-game.c
                  line
                  5632

    

    • 

  • 
#11
g_cclosure_marshal_VOID__VOIDv

    
                at gmarshal.c
                  line
                  115

    

    • 

  • 
#12
_g_closure_invoke_va

    
                at gclosure.c
                  line
                  840

    

    • 

  • 
#13
g_signal_emit_valist

    
                at gsignal.c
                  line
                  3234

    

    • 

  • 
#14
g_signal_emit_by_name

    
                at gsignal.c
                  line
                  3424

    

    • 

  • 
#15
chess_player_undo

    
                at chess-game.c
                  line
                  518

    

    • 

  • 
#16
undo_move_cb

    
                at gnome-chess.c
                  line
                  4005

    

    • 

  • 
#17
g_cclosure_marshal_VOID__VOID

    
                at gmarshal.c
                  line
                  85

    

    • 

  • 
#18
g_closure_invoke

    
                at gclosure.c
                  line
                  777

    

    • 

  • 
#19
signal_emit_unlocked_R

    
                at gsignal.c
                  line
                  3584

    

    • 

  • 
#20
g_signal_emit_valist

    
                at gsignal.c
                  line
                  3328

    

    • 

  • 
#21
g_signal_emit

    
                at gsignal.c
                  line
                  3384

    

    • 

  • 
#22
closure_accel_activate

    
                at gtkwidget.c
                  line
                  5733

    

    • 

  • 
#23
g_closure_invoke

    
                at gclosure.c
                  line
                  777

    

    • 

  • 
#24
signal_emit_unlocked_R

    
                at gsignal.c
                  line
                  3584

    

    • 

  • 
#25
g_signal_emit_valist

    
                at gsignal.c
                  line
                  3338

    

    • 

  • 
#26
g_signal_emit

    
                at gsignal.c
                  line
                  3384

    

    • 

  • 
#27
gtk_accel_group_activate

    
                at gtkaccelgroup.c
                  line
                  914

    

    • 

  • 
#28
gtk_accel_groups_activate

    
                at gtkaccelgroup.c
                  line
                  952

    

    • 

  • 
#29
gtk_window_activate_key

    
                at gtkwindow.c
                  line
                  9407

    

    • 

  • 
#30
gtk_window_key_press_event

    
                at gtkwindow.c
                  line
                  6068

    

    • 

  • 
#31
_gtk_marshal_BOOLEAN__BOXED

    
                at gtkmarshalers.c
                  line
                  85

    

    • 

  • 
#32
g_type_class_meta_marshal

    
                at gclosure.c
                  line
                  970

    

    • 

  • 
#33
g_closure_invoke

    
                at gclosure.c
                  line
                  777

    

    • 

  • 
#34
signal_emit_unlocked_R

    
                at gsignal.c
                  line
                  3622

    

    • 

  • 
#35
g_signal_emit_valist

    
                at gsignal.c
                  line
                  3338

    

    • 

  • 
#36
g_signal_emit

    
                at gsignal.c
                  line
                  3384

    

    • 

  • 
#37
gtk_widget_event_internal

    
                at gtkwidget.c
                  line
                  6714

    

    • 

  • 
#38
gtk_widget_event

    
                at gtkwidget.c
                  line
                  6371

    

    • 

  • 
#39
propagate_event

    
                at gtkmain.c
                  line
                  2490

    

    • 

  • 
#40
gtk_propagate_event

    
                at gtkmain.c
                  line
                  2536

    

    • 

  • 
#41
gtk_main_do_event

    
                at gtkmain.c
                  line
                  1716

    

    • 

  • 
#42
_gdk_event_emit

    
                at gdkevents.c
                  line
                  69

    

    • 

  • 
#43
gdk_event_source_dispatch

    
                at gdkeventsource.c
                  line
                  364

    

    • 

  • 
#44
g_main_dispatch

    
                at gmain.c
                  line
                  3054

    

    • 

  • 
#45
g_main_context_dispatch

    
                at gmain.c
                  line
                  3630

    

    • 

  • 
#46
g_main_context_iterate

    
                at gmain.c
                  line
                  3701

    

    • 

  • 
#47
g_main_context_iteration

    
                at gmain.c
                  line
                  3762

    

    • 

  • 
#48
g_application_run

    
                at gapplication.c
                  line
                  1623

    

    • 

  • 
#49
gnome_chess_main

    
                at gnome-chess.c
                  line
                  6557

    

    • 

  • 
#50
main

    
                at gnome-chess.c
                  line
                  6568

    

    • 













Comment 2


Chris Cummins





          2013-06-04 16:44:34 UTC
        



Created attachment 246017 [details] [review]
Always undo from most the most recent move

This makes the Undo action correctly undo the most recent move made, irrespective of what move is currently being displayed.






Comment 3


Michael Catanzaro





          2013-06-05 03:51:09 UTC
        



Review of attachment 246017 [details] [review]:

Thank you for this fix, it works great!

I'm not sure the loop is needed, though, since all it does is set scene.move_number to -1. Any reason not to just do that instead?  (update_board does get called each time scene.move_number is set, but once seems to suffice.)






Comment 4


Chris Cummins





          2013-06-05 10:45:08 UTC
        



Created attachment 246055 [details] [review]
Always undo from most the most recent move (v2)

Hi Michael, thanks - you're right. I was emulating the behavior of history_next_clicked_cb(), but closer inspection of the move_number setter shows that a single assignment as you suggest will do the right thing, with a single update_board() signal emitted if move_number was not already equal -1. 

Amended patch attached.






Comment 5


Michael Catanzaro





          2013-06-05 17:30:04 UTC
        



Created attachment 246105 [details] [review]
Always undo from most the most recent move

This makes the in-game undo action always undo the most recent
move. This prevents a Segmentation Fault when the user selects "Show
previous move" and then attempts to undo move.

v2: don't loop on assignment (review from Michael Catanzaro).






Comment 6


Michael Catanzaro





          2013-06-05 17:35:05 UTC
        



I'm not sure what the my above is, other than an accident trying to learn git bz.

Anyway I've pushed your patch to master and gnome-3-8, thanks again!






Comment 7


Michael Catanzaro





          2013-06-05 17:36:00 UTC
        



Review of attachment 246055 [details] [review]:

Good






Comment 8


Chris Cummins





          2013-06-06 12:46:04 UTC
        



I don't think I'll be trying git-bz until bugzilla has an "undo" button! Thanks for the review :-)