patchFreeciv - Patches: patch #4298, Fix shutdown sequence for gtk3...

 
 
Show feedback again

patch #4298: Fix shutdown sequence for gtk3 client

Submitted by:  Guillaume Melquiond <silene>
Submitted on:  Fri 08 Nov 2013 02:15:42 PM UTC  
 
Category: client-gtk-3.0Priority: 5 - Normal
Status: NonePrivacy: Public
Assigned to: NoneOpen/Closed: Open
Planned Release: 

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

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

 

Fri 08 Nov 2013 02:15:42 PM UTC, original submission:

The gtk3 client is not properly shut down, which makes it painful to track memory leaks. The attached patch fixes several issues related to the shutdown sequence. The first bugfix impacts all the clients, the other ones are specific to the gtk3 client (though some of them could/should be ported to the gtk2 client).

1. Move call to client_game_free earlier in client_exit. Rationale: client_game_free calls ui functions, so it should be called before ui_exit has been called, otherwise it causes use-after-free errors. Note: calling client_game_free right after client_remove_all_cli_conn should not introduce any issue, since this is already what set_client_state does.

2. Replace calls to client_exit with calls to gtk_main_quit. Rationale: the shutdown code was located in ui_main after the call to gtk_main; due to the exit paths calling client_exit, the client thus exited before the gtk_main call in ui_main could ever return. In other words, the shutdown code in ui_main was just dead code. Note: the only other client that calls client_exit from inside its ui loop is the xaw client; all the other clients properly leave their ui loop first.

3. Move shutdown code from ui_main to ui_exit. Rationale: this is the point of the ui_exit function in the first place.

4. Call gtk_widget_destroy on the toplevel window in ui_exit. Rationale: again, this is the point of ui_exit.

5. Remove call to tileset_free_tiles from ui_exit. Rationale: client_exit calls tileset_free before calling ui_exit, so calling tileset_free_tiles in ui_exit results in a double-free.

6. Remove call to g_object_unref in ui_exit. Rationale: gtk_widget_destroy already takes care of destroying the message buffer, so keeping the call around results in a double-free.

7. Remove call to luaconsole_dialog_popdown from luaconsole_dialog_done. Rationale: by the time the function is called, the ui is already down. Note: luaconsole_dialog_done was the only finalizer function to mess with the ui, all the other ones are well-behaved.

8. Fix inverted logic in luaconsole_dialog_popdown. Rationale: there is no point in destroying a widget that was never created in the first place.

Note: the client was run under valgrind to ensure that the patch introduced no use-after-free nor double-free.

Guillaume Melquiond <silene>

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #19337:  gtk3-shutdown.patch added by silene (3kB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by silene (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):

     

     

    Follows 1 latest change.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 08 Nov 2013 02:15:42 PM UTCsileneAttached File-=>Added gtk3-shutdown.patch, #19337
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup