* [PATCH] perf script python: Fix dict reference counting
@ 2018-07-06 6:53 Janne Huttunen
2018-07-08 11:17 ` Jiri Olsa
0 siblings, 1 reply; 8+ messages in thread
From: Janne Huttunen @ 2018-07-06 6:53 UTC (permalink / raw)
To: linux-kernel
Cc: Alexander Shishkin, Andi Kleen, Arnaldo Carvalho de Melo,
Jaroslav Škarvada, Jiri Olsa, Namhyung Kim, Peter Zijlstra,
Janne Huttunen
The dictionaries are attached to the parameter tuple that steals the
references. The code should not decrement the reference counters
explicitly. Otherwise the objects might be released while they are
still in use which may cause perf crashes, assertions or just plain
weird behavior like unexpected data changes in stored objects.
Signed-off-by: Janne Huttunen <janne.huttunen@nokia.com>
---
tools/perf/util/scripting-engines/trace-event-python.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index 46e9e19..60fce44 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -908,14 +908,11 @@ static void python_process_tracepoint(struct perf_sample *sample,
if (_PyTuple_Resize(&t, n) == -1)
Py_FatalError("error resizing Python tuple");
- if (!dict) {
+ if (!dict)
call_object(handler, t, handler_name);
- } else {
+ else
call_object(handler, t, default_handler_name);
- Py_DECREF(dict);
- }
- Py_XDECREF(all_entries_dict);
Py_DECREF(t);
}
@@ -1235,7 +1232,6 @@ static void python_process_general_event(struct perf_sample *sample,
call_object(handler, t, handler_name);
- Py_DECREF(dict);
Py_DECREF(t);
}
--
2.5.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] perf script python: Fix dict reference counting
2018-07-06 6:53 [PATCH] perf script python: Fix dict reference counting Janne Huttunen
@ 2018-07-08 11:17 ` Jiri Olsa
2018-07-09 9:25 ` Janne Huttunen
0 siblings, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2018-07-08 11:17 UTC (permalink / raw)
To: Janne Huttunen
Cc: linux-kernel, Alexander Shishkin, Andi Kleen,
Arnaldo Carvalho de Melo, Jaroslav Škarvada, Namhyung Kim,
Peter Zijlstra
On Fri, Jul 06, 2018 at 09:53:44AM +0300, Janne Huttunen wrote:
> The dictionaries are attached to the parameter tuple that steals the
> references. The code should not decrement the reference counters
> explicitly. Otherwise the objects might be released while they are
> still in use which may cause perf crashes, assertions or just plain
> weird behavior like unexpected data changes in stored objects.
>
> Signed-off-by: Janne Huttunen <janne.huttunen@nokia.com>
> ---
> tools/perf/util/scripting-engines/trace-event-python.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> index 46e9e19..60fce44 100644
> --- a/tools/perf/util/scripting-engines/trace-event-python.c
> +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> @@ -908,14 +908,11 @@ static void python_process_tracepoint(struct perf_sample *sample,
> if (_PyTuple_Resize(&t, n) == -1)
> Py_FatalError("error resizing Python tuple");
>
> - if (!dict) {
> + if (!dict)
> call_object(handler, t, handler_name);
> - } else {
> + else
> call_object(handler, t, default_handler_name);
> - Py_DECREF(dict);
> - }
>
> - Py_XDECREF(all_entries_dict);
> Py_DECREF(t);
> }
>
> @@ -1235,7 +1232,6 @@ static void python_process_general_event(struct perf_sample *sample,
>
> call_object(handler, t, handler_name);
>
> - Py_DECREF(dict);
> Py_DECREF(t);
so the dict is released when the tuple is released?
jirka
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] perf script python: Fix dict reference counting
2018-07-08 11:17 ` Jiri Olsa
@ 2018-07-09 9:25 ` Janne Huttunen
2018-07-09 9:41 ` Jiri Olsa
0 siblings, 1 reply; 8+ messages in thread
From: Janne Huttunen @ 2018-07-09 9:25 UTC (permalink / raw)
To: Jiri Olsa
Cc: linux-kernel, Alexander Shishkin, Andi Kleen,
Arnaldo Carvalho de Melo, Jaroslav Škarvada, Namhyung Kim,
Peter Zijlstra
On Sun, 2018-07-08 at 13:17 +0200, Jiri Olsa wrote:
> On Fri, Jul 06, 2018 at 09:53:44AM +0300, Janne Huttunen wrote:
> >
> > The dictionaries are attached to the parameter tuple that steals the
> > references. The code should not decrement the reference counters
> > explicitly. Otherwise the objects might be released while they are
> > still in use which may cause perf crashes, assertions or just plain
> > weird behavior like unexpected data changes in stored objects.
> >
> > Signed-off-by: Janne Huttunen <janne.huttunen@nokia.com>
> > ---
> > tools/perf/util/scripting-engines/trace-event-python.c | 8 ++------
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> > index 46e9e19..60fce44 100644
> > --- a/tools/perf/util/scripting-engines/trace-event-python.c
> > +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> > @@ -908,14 +908,11 @@ static void python_process_tracepoint(struct perf_sample *sample,
> > if (_PyTuple_Resize(&t, n) == -1)
> > Py_FatalError("error resizing Python tuple");
> >
> > - if (!dict) {
> > + if (!dict)
> > call_object(handler, t, handler_name);
> > - } else {
> > + else
> > call_object(handler, t, default_handler_name);
> > - Py_DECREF(dict);
> > - }
> >
> > - Py_XDECREF(all_entries_dict);
> > Py_DECREF(t);
> > }
> >
> > @@ -1235,7 +1232,6 @@ static void python_process_general_event(struct perf_sample *sample,
> >
> > call_object(handler, t, handler_name);
> >
> > - Py_DECREF(dict);
> > Py_DECREF(t);
>
> so the dict is released when the tuple is released?
To the best of my knowledge, yes.
As far as I can see, there is only a single reference to each dict
and according to the Python documentation PyTuple_SetItem() "steals"
the reference passed to it. If so, afterwards the tuple owns the
only reference to the dict(s) and should take care of releasing
them when appropriate.
I even built libpython with reference debugging enabled and when I
run perf without the fix I get this:
Fatal Python error: Objects/tupleobject.c:238 object at 0x7f10f2041b40 has negative ref count -1
Aborted (core dumped)
With the fix I get no errors.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] perf script python: Fix dict reference counting
2018-07-09 9:25 ` Janne Huttunen
@ 2018-07-09 9:41 ` Jiri Olsa
2018-07-09 10:59 ` [PATCH v2] " Janne Huttunen
0 siblings, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2018-07-09 9:41 UTC (permalink / raw)
To: Janne Huttunen
Cc: linux-kernel, Alexander Shishkin, Andi Kleen,
Arnaldo Carvalho de Melo, Jaroslav Škarvada, Namhyung Kim,
Peter Zijlstra
On Mon, Jul 09, 2018 at 12:25:33PM +0300, Janne Huttunen wrote:
> On Sun, 2018-07-08 at 13:17 +0200, Jiri Olsa wrote:
> > On Fri, Jul 06, 2018 at 09:53:44AM +0300, Janne Huttunen wrote:
> > >
> > > The dictionaries are attached to the parameter tuple that steals the
> > > references. The code should not decrement the reference counters
> > > explicitly. Otherwise the objects might be released while they are
> > > still in use which may cause perf crashes, assertions or just plain
> > > weird behavior like unexpected data changes in stored objects.
> > >
> > > Signed-off-by: Janne Huttunen <janne.huttunen@nokia.com>
> > > ---
> > > tools/perf/util/scripting-engines/trace-event-python.c | 8 ++------
> > > 1 file changed, 2 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
> > > index 46e9e19..60fce44 100644
> > > --- a/tools/perf/util/scripting-engines/trace-event-python.c
> > > +++ b/tools/perf/util/scripting-engines/trace-event-python.c
> > > @@ -908,14 +908,11 @@ static void python_process_tracepoint(struct perf_sample *sample,
> > > if (_PyTuple_Resize(&t, n) == -1)
> > > Py_FatalError("error resizing Python tuple");
> > >
> > > - if (!dict) {
> > > + if (!dict)
> > > call_object(handler, t, handler_name);
> > > - } else {
> > > + else
> > > call_object(handler, t, default_handler_name);
> > > - Py_DECREF(dict);
> > > - }
> > >
> > > - Py_XDECREF(all_entries_dict);
> > > Py_DECREF(t);
> > > }
> > >
> > > @@ -1235,7 +1232,6 @@ static void python_process_general_event(struct perf_sample *sample,
> > >
> > > call_object(handler, t, handler_name);
> > >
> > > - Py_DECREF(dict);
> > > Py_DECREF(t);
> >
> > so the dict is released when the tuple is released?
>
> To the best of my knowledge, yes.
>
> As far as I can see, there is only a single reference to each dict
> and according to the Python documentation PyTuple_SetItem() "steals"
> the reference passed to it. If so, afterwards the tuple owns the
> only reference to the dict(s) and should take care of releasing
> them when appropriate.
>
> I even built libpython with reference debugging enabled and when I
> run perf without the fix I get this:
>
> Fatal Python error: Objects/tupleobject.c:238 object at 0x7f10f2041b40 has negative ref count -1
> Aborted (core dumped)
>
> With the fix I get no errors.
>
ok, please include and mention that crash in the changelog
thanks,
jirka
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH v2] perf script python: Fix dict reference counting
2018-07-09 9:41 ` Jiri Olsa
@ 2018-07-09 10:59 ` Janne Huttunen
2018-07-09 14:55 ` Jiri Olsa
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Janne Huttunen @ 2018-07-09 10:59 UTC (permalink / raw)
To: linux-kernel
Cc: Alexander Shishkin, Andi Kleen, Arnaldo Carvalho de Melo,
Jaroslav Škarvada, Jiri Olsa, Namhyung Kim, Peter Zijlstra,
Janne Huttunen
The dictionaries are attached to the parameter tuple that steals
the references and takes care of releasing them when appropriate.
The code should not decrement the reference counts explicitly.
E.g. if libpython has been built with reference debugging enabled,
the superfluous DECREFs will trigger this error when running perf
script:
Fatal Python error: Objects/tupleobject.c:238 object at
0x7f10f2041b40 has negative ref count -1
Aborted (core dumped)
If the reference debugging is not enabled, the superfluous DECREFs
might cause the dict objects to be silently released while they are
still in use. This may trigger various other assertions or just
cause perf crashes and/or weird and unexpected data changes in the
stored Python objects.
Signed-off-by: Janne Huttunen <janne.huttunen@nokia.com>
---
tools/perf/util/scripting-engines/trace-event-python.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index 46e9e19..60fce44 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -908,14 +908,11 @@ static void python_process_tracepoint(struct perf_sample *sample,
if (_PyTuple_Resize(&t, n) == -1)
Py_FatalError("error resizing Python tuple");
- if (!dict) {
+ if (!dict)
call_object(handler, t, handler_name);
- } else {
+ else
call_object(handler, t, default_handler_name);
- Py_DECREF(dict);
- }
- Py_XDECREF(all_entries_dict);
Py_DECREF(t);
}
@@ -1235,7 +1232,6 @@ static void python_process_general_event(struct perf_sample *sample,
call_object(handler, t, handler_name);
- Py_DECREF(dict);
Py_DECREF(t);
}
--
2.5.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] perf script python: Fix dict reference counting
2018-07-09 10:59 ` [PATCH v2] " Janne Huttunen
@ 2018-07-09 14:55 ` Jiri Olsa
2018-07-11 8:14 ` Namhyung Kim
2018-07-12 14:04 ` [tip:perf/urgent] " tip-bot for Janne Huttunen
2 siblings, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2018-07-09 14:55 UTC (permalink / raw)
To: Janne Huttunen
Cc: linux-kernel, Alexander Shishkin, Andi Kleen,
Arnaldo Carvalho de Melo, Jaroslav Škarvada, Namhyung Kim,
Peter Zijlstra
On Mon, Jul 09, 2018 at 01:59:50PM +0300, Janne Huttunen wrote:
> The dictionaries are attached to the parameter tuple that steals
> the references and takes care of releasing them when appropriate.
> The code should not decrement the reference counts explicitly.
> E.g. if libpython has been built with reference debugging enabled,
> the superfluous DECREFs will trigger this error when running perf
> script:
>
> Fatal Python error: Objects/tupleobject.c:238 object at
> 0x7f10f2041b40 has negative ref count -1
> Aborted (core dumped)
>
> If the reference debugging is not enabled, the superfluous DECREFs
> might cause the dict objects to be silently released while they are
> still in use. This may trigger various other assertions or just
> cause perf crashes and/or weird and unexpected data changes in the
> stored Python objects.
>
> Signed-off-by: Janne Huttunen <janne.huttunen@nokia.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
thanks,
jirka
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] perf script python: Fix dict reference counting
2018-07-09 10:59 ` [PATCH v2] " Janne Huttunen
2018-07-09 14:55 ` Jiri Olsa
@ 2018-07-11 8:14 ` Namhyung Kim
2018-07-12 14:04 ` [tip:perf/urgent] " tip-bot for Janne Huttunen
2 siblings, 0 replies; 8+ messages in thread
From: Namhyung Kim @ 2018-07-11 8:14 UTC (permalink / raw)
To: Janne Huttunen
Cc: linux-kernel, Alexander Shishkin, Andi Kleen,
Arnaldo Carvalho de Melo, Jaroslav Škarvada, Jiri Olsa,
Peter Zijlstra, kernel-team
Hell,
On Mon, Jul 09, 2018 at 01:59:50PM +0300, Janne Huttunen wrote:
> The dictionaries are attached to the parameter tuple that steals
> the references and takes care of releasing them when appropriate.
> The code should not decrement the reference counts explicitly.
> E.g. if libpython has been built with reference debugging enabled,
> the superfluous DECREFs will trigger this error when running perf
> script:
>
> Fatal Python error: Objects/tupleobject.c:238 object at
> 0x7f10f2041b40 has negative ref count -1
> Aborted (core dumped)
>
> If the reference debugging is not enabled, the superfluous DECREFs
> might cause the dict objects to be silently released while they are
> still in use. This may trigger various other assertions or just
> cause perf crashes and/or weird and unexpected data changes in the
> stored Python objects.
>
> Signed-off-by: Janne Huttunen <janne.huttunen@nokia.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tip:perf/urgent] perf script python: Fix dict reference counting
2018-07-09 10:59 ` [PATCH v2] " Janne Huttunen
2018-07-09 14:55 ` Jiri Olsa
2018-07-11 8:14 ` Namhyung Kim
@ 2018-07-12 14:04 ` tip-bot for Janne Huttunen
2 siblings, 0 replies; 8+ messages in thread
From: tip-bot for Janne Huttunen @ 2018-07-12 14:04 UTC (permalink / raw)
To: linux-tip-commits
Cc: janne.huttunen, acme, jskarvad, tglx, alexander.shishkin, peterz,
linux-kernel, jolsa, namhyung, mingo, hpa, ak
Commit-ID: db0ba84c04ef2cf293aaada5ae97531127844d9d
Gitweb: https://git.kernel.org/tip/db0ba84c04ef2cf293aaada5ae97531127844d9d
Author: Janne Huttunen <janne.huttunen@nokia.com>
AuthorDate: Mon, 9 Jul 2018 13:59:50 +0300
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 11 Jul 2018 09:45:24 -0400
perf script python: Fix dict reference counting
The dictionaries are attached to the parameter tuple that steals the
references and takes care of releasing them when appropriate. The code
should not decrement the reference counts explicitly. E.g. if libpython
has been built with reference debugging enabled, the superfluous DECREFs
will trigger this error when running perf script:
Fatal Python error: Objects/tupleobject.c:238 object at
0x7f10f2041b40 has negative ref count -1
Aborted (core dumped)
If the reference debugging is not enabled, the superfluous DECREFs might
cause the dict objects to be silently released while they are still in
use. This may trigger various other assertions or just cause perf
crashes and/or weird and unexpected data changes in the stored Python
objects.
Signed-off-by: Janne Huttunen <janne.huttunen@nokia.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jaroslav Skarvada <jskarvad@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1531133990-17485-1-git-send-email-janne.huttunen@nokia.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/scripting-engines/trace-event-python.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c
index 8b2eb6dbff4d..bc32e57d17be 100644
--- a/tools/perf/util/scripting-engines/trace-event-python.c
+++ b/tools/perf/util/scripting-engines/trace-event-python.c
@@ -908,14 +908,11 @@ static void python_process_tracepoint(struct perf_sample *sample,
if (_PyTuple_Resize(&t, n) == -1)
Py_FatalError("error resizing Python tuple");
- if (!dict) {
+ if (!dict)
call_object(handler, t, handler_name);
- } else {
+ else
call_object(handler, t, default_handler_name);
- Py_DECREF(dict);
- }
- Py_XDECREF(all_entries_dict);
Py_DECREF(t);
}
@@ -1235,7 +1232,6 @@ static void python_process_general_event(struct perf_sample *sample,
call_object(handler, t, handler_name);
- Py_DECREF(dict);
Py_DECREF(t);
}
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-07-12 14:04 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-06 6:53 [PATCH] perf script python: Fix dict reference counting Janne Huttunen
2018-07-08 11:17 ` Jiri Olsa
2018-07-09 9:25 ` Janne Huttunen
2018-07-09 9:41 ` Jiri Olsa
2018-07-09 10:59 ` [PATCH v2] " Janne Huttunen
2018-07-09 14:55 ` Jiri Olsa
2018-07-11 8:14 ` Namhyung Kim
2018-07-12 14:04 ` [tip:perf/urgent] " tip-bot for Janne Huttunen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).