diff mbox series

[babeltrace,1.5] Use trace->trace_id for check to remove trace from bt_ctx

Message ID 20180703162854.27550-1-jonathan.rajotte-julien@efficios.com
State Accepted, archived
Headers show
Series [babeltrace,1.5] Use trace->trace_id for check to remove trace from bt_ctx | expand

Commit Message

Jonathan Rajotte July 3, 2018, 4:28 p.m. UTC
Commit b9e6498df8b3e7c2ad312dccddf9f1a5e181648e remove the existence
guarantee of the trace_id hash table key by moving the trace->in_use
assignation before the assignation of trace_id and insertion into
the hash table.

Use the trade_id field value to validate if it should be
removed from the hash table.
A NULL trace_id field indicates that no insertion was performed.

This is mostly a workaround to a problem found in, at least, glib-2.28
where the g_hash_table_lookup_node abort on a SIGFPE signal due to
modulo by zero. The exact cause for this is unknown for now [1].
There is little reason for "mod" to be 0 at that point as explained in
the bug report.

Currently unable to reproduce.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1074401

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
---
 formats/lttng-live/lttng-live-comm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jonathan Rajotte July 3, 2018, 6:33 p.m. UTC | #1
On Tue, Jul 03, 2018 at 12:28:54PM -0400, Jonathan Rajotte wrote:
> Commit b9e6498df8b3e7c2ad312dccddf9f1a5e181648e remove the existence
> guarantee of the trace_id hash table key by moving the trace->in_use
> assignation before the assignation of trace_id and insertion into
> the hash table.
> 
> Use the trade_id field value to validate if it should be
> removed from the hash table.
> A NULL trace_id field indicates that no insertion was performed.

This is not true since trace_id start at zero. A solution to this is to start
the trace_id numbering a 1.

> 
> This is mostly a workaround to a problem found in, at least, glib-2.28
> where the g_hash_table_lookup_node abort on a SIGFPE signal due to
> modulo by zero. The exact cause for this is unknown for now [1].
> There is little reason for "mod" to be 0 at that point as explained in
> the bug report.
> 
> Currently unable to reproduce.
> 
> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1074401
> 
> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
> ---
>  formats/lttng-live/lttng-live-comm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/formats/lttng-live/lttng-live-comm.c b/formats/lttng-live/lttng-live-comm.c
> index 055b1c30..941d5c2f 100644
> --- a/formats/lttng-live/lttng-live-comm.c
> +++ b/formats/lttng-live/lttng-live-comm.c
> @@ -1466,7 +1466,7 @@ int del_traces(gpointer key, gpointer value, gpointer user_data)
>  		lvstream->in_trace = 0;
>  		bt_list_del(&lvstream->trace_stream_node);
>  	}
> -	if (trace->in_use) {
> +	if (trace->in_use && trace->trace_id) {
>  		ret = bt_context_remove_trace(bt_ctx, trace->trace_id);
>  		if (ret < 0)
>  			fprintf(stderr, "[error] removing trace from context\n");
> -- 
> 2.17.1
>
Jérémie Galarneau July 3, 2018, 8:07 p.m. UTC | #2
Merged in stable-1.5. I have addressed the problem you mentioned
regarding the trace_id.

Thanks!
Jérémie

On 3 July 2018 at 14:33, Jonathan Rajotte-Julien
<jonathan.rajotte-julien at efficios.com> wrote:
> On Tue, Jul 03, 2018 at 12:28:54PM -0400, Jonathan Rajotte wrote:
>> Commit b9e6498df8b3e7c2ad312dccddf9f1a5e181648e remove the existence
>> guarantee of the trace_id hash table key by moving the trace->in_use
>> assignation before the assignation of trace_id and insertion into
>> the hash table.
>>
>> Use the trade_id field value to validate if it should be
>> removed from the hash table.
>> A NULL trace_id field indicates that no insertion was performed.
>
> This is not true since trace_id start at zero. A solution to this is to start
> the trace_id numbering a 1.
>
>>
>> This is mostly a workaround to a problem found in, at least, glib-2.28
>> where the g_hash_table_lookup_node abort on a SIGFPE signal due to
>> modulo by zero. The exact cause for this is unknown for now [1].
>> There is little reason for "mod" to be 0 at that point as explained in
>> the bug report.
>>
>> Currently unable to reproduce.
>>
>> [1] https://bugzilla.redhat.com/show_bug.cgi?id=1074401
>>
>> Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien at efficios.com>
>> ---
>>  formats/lttng-live/lttng-live-comm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/formats/lttng-live/lttng-live-comm.c b/formats/lttng-live/lttng-live-comm.c
>> index 055b1c30..941d5c2f 100644
>> --- a/formats/lttng-live/lttng-live-comm.c
>> +++ b/formats/lttng-live/lttng-live-comm.c
>> @@ -1466,7 +1466,7 @@ int del_traces(gpointer key, gpointer value, gpointer user_data)
>>               lvstream->in_trace = 0;
>>               bt_list_del(&lvstream->trace_stream_node);
>>       }
>> -     if (trace->in_use) {
>> +     if (trace->in_use && trace->trace_id) {
>>               ret = bt_context_remove_trace(bt_ctx, trace->trace_id);
>>               if (ret < 0)
>>                       fprintf(stderr, "[error] removing trace from context\n");
>> --
>> 2.17.1
>>
>
> --
> Jonathan Rajotte-Julien
> EfficiOS
diff mbox series

Patch

diff --git a/formats/lttng-live/lttng-live-comm.c b/formats/lttng-live/lttng-live-comm.c
index 055b1c30..941d5c2f 100644
--- a/formats/lttng-live/lttng-live-comm.c
+++ b/formats/lttng-live/lttng-live-comm.c
@@ -1466,7 +1466,7 @@  int del_traces(gpointer key, gpointer value, gpointer user_data)
 		lvstream->in_trace = 0;
 		bt_list_del(&lvstream->trace_stream_node);
 	}
-	if (trace->in_use) {
+	if (trace->in_use && trace->trace_id) {
 		ret = bt_context_remove_trace(bt_ctx, trace->trace_id);
 		if (ret < 0)
 			fprintf(stderr, "[error] removing trace from context\n");