Re: [lttng-dev] [PATCH lttng-tools] Fix: cleanup stream on snapshot failure

[Jonathan Rajotte-Julien via lttng-dev <lttng-dev@lists.lttng.org>]

Hi Marcel,

Thanks for sending this patch.

Looks sensible to me, still do you have a reproducer for it? I went back to bug 1352 and even with https://bugs.lttng.org/attachments/546 was unable to force the assert failure.

Cheers

----- Original Message -----
> From: "Marcel Hamer via lttng-dev" <lttng-dev@lists.lttng.org>
> To: "lttng-dev" <lttng-dev@lists.lttng.org>
> Sent: Monday, 30 May, 2022 10:10:21
> Subject: [lttng-dev] [PATCH lttng-tools] Fix: cleanup stream on snapshot failure

> When a channel snapshot creation fails the stream should be cleaned up
> properly. If the stream is not closed and cleaned properly on a failure,
> the next time a snapshot is created an assert is triggered for:
> 
>	assert(!stream->trace_chunk);
> 
> inside the snapshot_channel function. Since the stream->trace_chunk was
> not reset to NULL. The reset to NULL happens inside the
> consumer_stream_close function.
> 
> Fixes #1352
> 
> Signed-off-by: Marcel Hamer <marcel.hamer@windriver.com>
> ---
> src/common/ust-consumer/ust-consumer.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/src/common/ust-consumer/ust-consumer.c
> b/src/common/ust-consumer/ust-consumer.c
> index f176ca40a..f43216829 100644
> --- a/src/common/ust-consumer/ust-consumer.c
> +++ b/src/common/ust-consumer/ust-consumer.c
> @@ -1147,13 +1147,13 @@ static int snapshot_channel(struct
> lttng_consumer_channel *channel,
> 		if (use_relayd) {
> 			ret = consumer_send_relayd_stream(stream, path);
> 			if (ret < 0) {
> -				goto error_unlock;
> +				goto error_close_stream;
> 			}
> 		} else {
> 			ret = consumer_stream_create_output_files(stream,
> 					false);
> 			if (ret < 0) {
> -				goto error_unlock;
> +				goto error_close_stream;
> 			}
> 			DBG("UST consumer snapshot stream (%" PRIu64 ")",
> 					stream->key);
> @@ -1170,19 +1170,19 @@ static int snapshot_channel(struct
> lttng_consumer_channel *channel,
> 		ret = lttng_ustconsumer_take_snapshot(stream);
> 		if (ret < 0) {
> 			ERR("Taking UST snapshot");
> -			goto error_unlock;
> +			goto error_close_stream;
> 		}
> 
> 		ret = lttng_ustconsumer_get_produced_snapshot(stream, &produced_pos);
> 		if (ret < 0) {
> 			ERR("Produced UST snapshot position");
> -			goto error_unlock;
> +			goto error_close_stream;
> 		}
> 
> 		ret = lttng_ustconsumer_get_consumed_snapshot(stream, &consumed_pos);
> 		if (ret < 0) {
> 			ERR("Consumerd UST snapshot position");
> -			goto error_unlock;
> +			goto error_close_stream;
> 		}
> 
> 		/*
> --
> 2.25.1
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev