Message ID | 1517600891-23632-3-git-send-email-francis.deslauriers@efficios.com |
---|---|
State | Superseded, archived |
Headers |
From: francis.deslauriers at efficios.com (Francis Deslauriers) Date: Fri, 2 Feb 2018 14:48:05 -0500 Subject: [lttng-dev] [PATCH lttng-ust 2/8] Fix: missing event removal from the event hashtable In-Reply-To: <1517600891-23632-1-git-send-email-francis.deslauriers@efficios.com> References: <1517600891-23632-1-git-send-email-francis.deslauriers@efficios.com> Message-ID: <1517600891-23632-3-git-send-email-francis.deslauriers@efficios.com> |
Series |
Support provider duplicates and unloading
|
|
Commit Message
Francis Deslauriers
Feb. 2, 2018, 7:48 p.m. UTC
Signed-off-by: Francis Deslauriers <francis.deslauriers at efficios.com>
---
liblttng-ust/lttng-events.c | 4 ++++
1 file changed, 4 insertions(+)
Comments
----- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers francis.deslauriers at efficios.com wrote: Is this really a fix ? Or is it a preparation step in order to be able to remove events before the end of the session lifetime ? Thanks, Mathieu > Signed-off-by: Francis Deslauriers <francis.deslauriers at efficios.com> > --- > liblttng-ust/lttng-events.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c > index f4a7ccc..7419f78 100644 > --- a/liblttng-ust/lttng-events.c > +++ b/liblttng-ust/lttng-events.c > @@ -883,7 +883,11 @@ void _lttng_event_destroy(struct lttng_event *event) > { > struct lttng_enabler_ref *enabler_ref, *tmp_enabler_ref; > > + /* Remove from event list. */ > cds_list_del(&event->node); > + /* Remove from event hash table. */ > + cds_hlist_del(&event->hlist); > + > lttng_destroy_context(event->ctx); > lttng_free_event_filter_runtime(event); > /* Free event enabler refs */ > -- > 2.7.4
2018-02-07 15:50 GMT-05:00 Mathieu Desnoyers <mathieu.desnoyers at efficios.com>: > > > ----- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers francis.deslauriers at efficios.com wrote: > > Is this really a fix ? Or is it a preparation step in order to be able to > remove events before the end of the session lifetime ? It's not a fix in the sense that it was not causing issues because it was only called at session destruction. But it's a fix in the sense that the _lttng_event_destroy function was not doing all the work necessary to destroy an event passed as argument. Taken by itself, a call to this function would leave one of the hlists of the hash table in a incorrect state by freeing an element without cds_hlist_del() it from the hlist before. That was my thought process. What do you think? Thank you, Francis > Thanks, > > Mathieu > > >> Signed-off-by: Francis Deslauriers <francis.deslauriers at efficios.com> >> --- >> liblttng-ust/lttng-events.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c >> index f4a7ccc..7419f78 100644 >> --- a/liblttng-ust/lttng-events.c >> +++ b/liblttng-ust/lttng-events.c >> @@ -883,7 +883,11 @@ void _lttng_event_destroy(struct lttng_event *event) >> { >> struct lttng_enabler_ref *enabler_ref, *tmp_enabler_ref; >> >> + /* Remove from event list. */ >> cds_list_del(&event->node); >> + /* Remove from event hash table. */ >> + cds_hlist_del(&event->hlist); >> + >> lttng_destroy_context(event->ctx); >> lttng_free_event_filter_runtime(event); >> /* Free event enabler refs */ >> -- >> 2.7.4 > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com
----- On Feb 8, 2018, at 3:42 PM, Francis Deslauriers francis.deslauriers at efficios.com wrote: > 2018-02-07 15:50 GMT-05:00 Mathieu Desnoyers <mathieu.desnoyers at efficios.com>: >> >> >> ----- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers >> francis.deslauriers at efficios.com wrote: >> >> Is this really a fix ? Or is it a preparation step in order to be able to >> remove events before the end of the session lifetime ? > > It's not a fix in the sense that it was not causing issues because it > was only called at session destruction. > But it's a fix in the sense that the _lttng_event_destroy function was > not doing all the work necessary to destroy an event passed as > argument. Taken by itself, a call to this function would leave one of > the hlists of the hash table in a incorrect state by freeing an > element without cds_hlist_del() it from the hlist before. > > That was my thought process. What do you think? A fix is something I need to backport to make things work. I don't see this as a fix based on that definition. Thanks, Mathieu > > Thank you, > Francis > >> Thanks, >> >> Mathieu >> >> >>> Signed-off-by: Francis Deslauriers <francis.deslauriers at efficios.com> >>> --- >>> liblttng-ust/lttng-events.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c >>> index f4a7ccc..7419f78 100644 >>> --- a/liblttng-ust/lttng-events.c >>> +++ b/liblttng-ust/lttng-events.c >>> @@ -883,7 +883,11 @@ void _lttng_event_destroy(struct lttng_event *event) >>> { >>> struct lttng_enabler_ref *enabler_ref, *tmp_enabler_ref; >>> >>> + /* Remove from event list. */ >>> cds_list_del(&event->node); >>> + /* Remove from event hash table. */ >>> + cds_hlist_del(&event->hlist); >>> + >>> lttng_destroy_context(event->ctx); >>> lttng_free_event_filter_runtime(event); >>> /* Free event enabler refs */ >>> -- >>> 2.7.4 >> >> -- >> Mathieu Desnoyers >> EfficiOS Inc. >> http://www.efficios.com > > > > -- > Francis Deslauriers > Software developer > EfficiOS inc.
diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c index f4a7ccc..7419f78 100644 --- a/liblttng-ust/lttng-events.c +++ b/liblttng-ust/lttng-events.c @@ -883,7 +883,11 @@ void _lttng_event_destroy(struct lttng_event *event) { struct lttng_enabler_ref *enabler_ref, *tmp_enabler_ref; + /* Remove from event list. */ cds_list_del(&event->node); + /* Remove from event hash table. */ + cds_hlist_del(&event->hlist); + lttng_destroy_context(event->ctx); lttng_free_event_filter_runtime(event); /* Free event enabler refs */