[PATCH] perf ui/gtk: Ensure not to call gtk_main_quit() twice

[PATCH] perf ui/gtk: Ensure not to call gtk_main_quit() twice

[Namhyung Kim]

Currently the gtk_main_quit() is called twice when perf exits so the
following warning is emitted:

  [penberg@tux perf]$ ./perf report --gtk
  ^Cperf: Interrupt

  (perf:4048): Gtk-CRITICAL **: IA__gtk_main_quit: assertion `main_loops != NULL' failed

Fix it by not to call it unnecessarily.

Reported-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/gtk/setup.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/ui/gtk/setup.c b/tools/perf/ui/gtk/setup.c
index ad40b3626fdb..ec1ee26b485a 100644
--- a/tools/perf/ui/gtk/setup.c
+++ b/tools/perf/ui/gtk/setup.c
@@ -13,6 +13,8 @@ int perf_gtk__init(void)
 
 void perf_gtk__exit(bool wait_for_ok __used)
 {
+	if (!perf_gtk__is_active_context(pgctx))
+		return;
 	perf_error__unregister(&perf_gtk_eops);
 	gtk_main_quit();
 }
-- 
1.7.9.2

Re: [PATCH] perf ui/gtk: Ensure not to call gtk_main_quit() twice

[Pekka Enberg]

On Fri, Aug 17, 2012 at 7:56 PM, Namhyung Kim <namhyung@kernel.org> wrote:
> Currently the gtk_main_quit() is called twice when perf exits so the
> following warning is emitted:
>
>   [penberg@tux perf]$ ./perf report --gtk
>   ^Cperf: Interrupt
>
>   (perf:4048): Gtk-CRITICAL **: IA__gtk_main_quit: assertion `main_loops != NULL' failed
>
> Fix it by not to call it unnecessarily.
>
> Reported-by: Pekka Enberg <penberg@kernel.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/ui/gtk/setup.c |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tools/perf/ui/gtk/setup.c b/tools/perf/ui/gtk/setup.c
> index ad40b3626fdb..ec1ee26b485a 100644
> --- a/tools/perf/ui/gtk/setup.c
> +++ b/tools/perf/ui/gtk/setup.c
> @@ -13,6 +13,8 @@ int perf_gtk__init(void)
>
>  void perf_gtk__exit(bool wait_for_ok __used)
>  {
> +       if (!perf_gtk__is_active_context(pgctx))
> +               return;
>         perf_error__unregister(&perf_gtk_eops);
>         gtk_main_quit();
>  }

Wouldn't it be nicer to rearrange the callers so that perf_gtk__exit()
is not called twice?

Re: [PATCH] perf ui/gtk: Ensure not to call gtk_main_quit() twice

[Namhyung Kim]

On Sat, 18 Aug 2012 10:52:03 +0300, Pekka Enberg wrote:
> On Fri, Aug 17, 2012 at 7:56 PM, Namhyung Kim <namhyung@kernel.org> wrote:
>> Currently the gtk_main_quit() is called twice when perf exits so the
>> following warning is emitted:
>>
>>   [penberg@tux perf]$ ./perf report --gtk
>>   ^Cperf: Interrupt
>>
>>   (perf:4048): Gtk-CRITICAL **: IA__gtk_main_quit: assertion `main_loops != NULL' failed
>>
>> Fix it by not to call it unnecessarily.
>>
>> Reported-by: Pekka Enberg <penberg@kernel.org>
>> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>> ---
>>  tools/perf/ui/gtk/setup.c |    2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/tools/perf/ui/gtk/setup.c b/tools/perf/ui/gtk/setup.c
>> index ad40b3626fdb..ec1ee26b485a 100644
>> --- a/tools/perf/ui/gtk/setup.c
>> +++ b/tools/perf/ui/gtk/setup.c
>> @@ -13,6 +13,8 @@ int perf_gtk__init(void)
>>
>>  void perf_gtk__exit(bool wait_for_ok __used)
>>  {
>> +       if (!perf_gtk__is_active_context(pgctx))
>> +               return;
>>         perf_error__unregister(&perf_gtk_eops);
>>         gtk_main_quit();
>>  }
>
> Wouldn't it be nicer to rearrange the callers so that perf_gtk__exit()
> is not called twice?

You mean this?


diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
index c7820e569660..d25e145e9a89 100644
--- a/tools/perf/ui/setup.c
+++ b/tools/perf/ui/setup.c
@@ -2,6 +2,7 @@
 
 #include "../cache.h"
 #include "../debug.h"
+#include "gtk/gtk.h"
 
 
 pthread_mutex_t ui__lock = PTHREAD_MUTEX_INITIALIZER;
@@ -37,7 +38,8 @@ void exit_browser(bool wait_for_ok)
 {
 	switch (use_browser) {
 	case 2:
-		perf_gtk__exit(wait_for_ok);
+		if (perf_gtk__is_active_context(pgctx))
+			perf_gtk__exit(wait_for_ok);
 		break;
 
 	case 1:

Re: [PATCH] perf ui/gtk: Ensure not to call gtk_main_quit() twice

[Namhyung Kim]

On Mon, 20 Aug 2012 10:50:21 +0900, Namhyung Kim wrote:
> On Sat, 18 Aug 2012 10:52:03 +0300, Pekka Enberg wrote:
>> Wouldn't it be nicer to rearrange the callers so that perf_gtk__exit()
>> is not called twice?
>
> You mean this?
>
>
> diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
> index c7820e569660..d25e145e9a89 100644
> --- a/tools/perf/ui/setup.c
> +++ b/tools/perf/ui/setup.c
> @@ -2,6 +2,7 @@
>  
>  #include "../cache.h"
>  #include "../debug.h"
> +#include "gtk/gtk.h"

Oops, it should be 

   #ifndef NO_GTK2_SUPPORT
   # include "gtk/gtk.h"
   #endif

Thanks,
Namhyung


>  
>  
>  pthread_mutex_t ui__lock = PTHREAD_MUTEX_INITIALIZER;
> @@ -37,7 +38,8 @@ void exit_browser(bool wait_for_ok)
>  {
>  	switch (use_browser) {
>  	case 2:
> -		perf_gtk__exit(wait_for_ok);
> +		if (perf_gtk__is_active_context(pgctx))
> +			perf_gtk__exit(wait_for_ok);
>  		break;
>  
>  	case 1:

Re: [PATCH] perf ui/gtk: Ensure not to call gtk_main_quit() twice

[Namhyung Kim]

On Mon, 20 Aug 2012 10:55:24 +0900, Namhyung Kim wrote:
> On Mon, 20 Aug 2012 10:50:21 +0900, Namhyung Kim wrote:
>> On Sat, 18 Aug 2012 10:52:03 +0300, Pekka Enberg wrote:
>>> Wouldn't it be nicer to rearrange the callers so that perf_gtk__exit()
>>> is not called twice?
>>
>> You mean this?
>>
>>
>> diff --git a/tools/perf/ui/setup.c b/tools/perf/ui/setup.c
>> index c7820e569660..d25e145e9a89 100644
>> --- a/tools/perf/ui/setup.c
>> +++ b/tools/perf/ui/setup.c
>> @@ -2,6 +2,7 @@
>>  
>>  #include "../cache.h"
>>  #include "../debug.h"
>> +#include "gtk/gtk.h"
>
> Oops, it should be 
>
>    #ifndef NO_GTK2_SUPPORT
>    # include "gtk/gtk.h"
>    #endif
>

Forgot to add the #ifdefery to the below code also. :-/  Anyway, it needs
to expose gtk specifics to general code with the #ifdef's. So I'd still
prefer the original patch.

Thanks,
Namhyung

Re: [PATCH] perf ui/gtk: Ensure not to call gtk_main_quit() twice

[Pekka Enberg]

On Mon, Aug 20, 2012 at 4:59 AM, Namhyung Kim <namhyung@kernel.org> wrote:
> Forgot to add the #ifdefery to the below code also. :-/  Anyway, it needs
> to expose gtk specifics to general code with the #ifdef's. So I'd still
> prefer the original patch.

Fair enough.

Acked-by: Pekka Enberg <penberg@kernel.org>

[tip:perf/core] perf ui gtk: Ensure not to call gtk_main_quit() twice

[tip-bot for Namhyung Kim]

Commit-ID:  2708bf3a30b7bfa02d60a3f03603b6b92d093f1a
Gitweb:     http://git.kernel.org/tip/2708bf3a30b7bfa02d60a3f03603b6b92d093f1a
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Sat, 18 Aug 2012 01:56:23 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 20 Aug 2012 09:29:12 -0300

perf ui gtk: Ensure not to call gtk_main_quit() twice

Currently the gtk_main_quit() is called twice when perf exits so the
following warning is emitted:

  [penberg@tux perf]$ ./perf report --gtk
  ^Cperf: Interrupt

  (perf:4048): Gtk-CRITICAL **: IA__gtk_main_quit: assertion `main_loops != NULL' failed

Fix it by not to call it unnecessarily.

Reported-by: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Pekka Enberg <penberg@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1345222583-3964-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/gtk/setup.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/tools/perf/ui/gtk/setup.c b/tools/perf/ui/gtk/setup.c
index ad40b36..ec1ee26 100644
--- a/tools/perf/ui/gtk/setup.c
+++ b/tools/perf/ui/gtk/setup.c
@@ -13,6 +13,8 @@ int perf_gtk__init(void)
 
 void perf_gtk__exit(bool wait_for_ok __used)
 {
+	if (!perf_gtk__is_active_context(pgctx))
+		return;
 	perf_error__unregister(&perf_gtk_eops);
 	gtk_main_quit();
 }