Unregistering the display change listener looks like a pointless
excercise given we'll exit in a moment.  When exiting qemu via
menu/file/quit this will not happen either.  Just drop the code.

Also return TRUE unconditionally.  This will tell gtk to ignore the
close request, so gtk will not start destroying widgets and causing
warnings due to UI code trying to talk to widgets which are gone.
Just depend on qmp_quit() doing it's job instead.

Reported-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/gtk.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index ef5bc42094..2925c66e0b 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -786,21 +786,13 @@ static gboolean gd_window_close(GtkWidget *widget, GdkEvent *event,
 {
     GtkDisplayState *s = opaque;
     bool allow_close = true;
-    int i;
 
     if (s->opts->has_window_close && !s->opts->window_close) {
         allow_close = false;
     }
 
     if (allow_close) {
-        for (i = 0; i < s->nb_vcs; i++) {
-            if (s->vc[i].type != GD_VC_GFX) {
-                continue;
-            }
-            unregister_displaychangelistener(&s->vc[i].gfx.dcl);
-        }
         qmp_quit(NULL);
-        return FALSE;
     }
 
     return TRUE;
-- 
2.9.3

On 03/14/2018 09:04 AM, Gerd Hoffmann wrote:
> Unregistering the display change listener looks like a pointless
> excercise given we'll exit in a moment.  When exiting qemu via

exercise

> menu/file/quit this will not happen either.  Just drop the code.
> 
> Also return TRUE unconditionally.  This will tell gtk to ignore the
> close request, so gtk will not start destroying widgets and causing
> warnings due to UI code trying to talk to widgets which are gone.
> Just depend on qmp_quit() doing it's job instead.
> 
> Reported-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Looks fine to me.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Mark, is your issue resolved? (you might add your Tested-by).

> ---
>  ui/gtk.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/ui/gtk.c b/ui/gtk.c
> index ef5bc42094..2925c66e0b 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -786,21 +786,13 @@ static gboolean gd_window_close(GtkWidget *widget, GdkEvent *event,
>  {
>      GtkDisplayState *s = opaque;
>      bool allow_close = true;
> -    int i;
>  
>      if (s->opts->has_window_close && !s->opts->window_close) {
>          allow_close = false;
>      }
>  
>      if (allow_close) {
> -        for (i = 0; i < s->nb_vcs; i++) {
> -            if (s->vc[i].type != GD_VC_GFX) {
> -                continue;
> -            }
> -            unregister_displaychangelistener(&s->vc[i].gfx.dcl);
> -        }
>          qmp_quit(NULL);
> -        return FALSE;
>      }
>  
>      return TRUE;
> 

On 15/03/18 11:35, Philippe Mathieu-Daudé wrote:

> On 03/14/2018 09:04 AM, Gerd Hoffmann wrote:
>> Unregistering the display change listener looks like a pointless
>> excercise given we'll exit in a moment.  When exiting qemu via
> 
> exercise
> 
>> menu/file/quit this will not happen either.  Just drop the code.
>>
>> Also return TRUE unconditionally.  This will tell gtk to ignore the
>> close request, so gtk will not start destroying widgets and causing
>> warnings due to UI code trying to talk to widgets which are gone.
>> Just depend on qmp_quit() doing it's job instead.
>>
>> Reported-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> Looks fine to me.
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> Mark, is your issue resolved? (you might add your Tested-by).

Yes, I can confirm that this patch fixes my test case.

Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.