Summary: Fix shutdown sequence for gtk3 client
                 Project: Freeciv
            Submitted by: silene
            Submitted on: Fri 08 Nov 2013 03:15:42 PM CET
                Category: client-gtk-3.0
                Priority: 5 - Normal
                  Status: None
                 Privacy: Public
             Assigned to: None
        Originator Email: 
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: 



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.


File Attachments:

Date: Fri 08 Nov 2013 03:15:42 PM CET  Name: gtk3-shutdown.patch  Size: 3kB  
By: silene



Reply to this item at:


  Message sent via/by Gna!

Freeciv-dev mailing list

Reply via email to