2012-08-17 16:57:41

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH] 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 <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
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


2012-08-18 07:52:14

by Pekka Enberg

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

On Fri, Aug 17, 2012 at 7:56 PM, Namhyung Kim <[email protected]> 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 <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> 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?

2012-08-20 01:57:21

by Namhyung Kim

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

On Sat, 18 Aug 2012 10:52:03 +0300, Pekka Enberg wrote:
> On Fri, Aug 17, 2012 at 7:56 PM, Namhyung Kim <[email protected]> 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 <[email protected]>
>> Signed-off-by: Namhyung Kim <[email protected]>
>> ---
>> 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:

2012-08-20 02:02:23

by Namhyung Kim

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

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:

2012-08-20 02:06:42

by Namhyung Kim

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

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

2012-08-20 05:04:42

by Pekka Enberg

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

On Mon, Aug 20, 2012 at 4:59 AM, Namhyung Kim <[email protected]> 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 <[email protected]>

2012-08-21 16:29:40

by Namhyung Kim

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

Commit-ID: 2708bf3a30b7bfa02d60a3f03603b6b92d093f1a
Gitweb: http://git.kernel.org/tip/2708bf3a30b7bfa02d60a3f03603b6b92d093f1a
Author: Namhyung Kim <[email protected]>
AuthorDate: Sat, 18 Aug 2012 01:56:23 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
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 <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
Acked-by: Pekka Enberg <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Pekka Enberg <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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();
}