Subject: Re: [PATCH 1/7] trace2: fix memory leak of thread name

From: Junio C Hamano <[email-masked]>
Date: Wed, 15 Sep 2021 14:01:39 -0700

"Jeff Hostetler via GitGitGadget" <[email-masked]> writes:

> From: Jeff Hostetler <[email-masked]>
>
> Do not leak the thread name (contained within the thread context) when
> a thread terminates.
>
> Signed-off-by: Jeff Hostetler <[email-masked]>
> ---
>  trace2/tr2_tls.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/trace2/tr2_tls.c b/trace2/tr2_tls.c
> index 067c23755fb..7da94aba522 100644
> --- a/trace2/tr2_tls.c
> +++ b/trace2/tr2_tls.c
> @@ -95,6 +95,7 @@ void tr2tls_unset_self(void)
>  
>  	pthread_setspecific(tr2tls_key, NULL);
>  
> +	strbuf_release(&ctx->thread_name);
>  	free(ctx->array_us_start);
>  	free(ctx);
>  }

So the idea is create allocates a new instance, and unset is to
release the resource held by it?

This is not a problem in _this_ patch but more about the base code
that is being fixed here, but using strbuf as thread_name member
sends a strong signal that it is designed to be inexpensive to
change thread_name after a context is created by create_self.  If
it is not the case, the member should be "const char *", which may
be computed using a temporary strbuf in create_self() and taken from
the strbuf with strbuf_detach(), I would think.

Thanks.