From: Stefan Sperling <stsp@stsp.name>
Subject: Re: tog: pthread_cond_destroy: Invalid argument
To: Christian Weisgerber <naddy@mips.inka.de>
Cc: gameoftrees@openbsd.org
Date: Fri, 13 Nov 2020 13:58:46 +0100

On Thu, Nov 12, 2020 at 05:03:09PM +0100, Christian Weisgerber wrote:
> Christian Weisgerber:
> 
> > stop_log_thread() is called again from close_log_view(), which in
> > turn is called when the old view is replaced with the new view.
> > Backspace, ^L, 'B' all suffer from this problem in the log view.
> 
> Call pthread_cond_destroy(cond) exactly once when closing a view.
> 
> This moves the pthread_cond_destroy(need_commits) from stop_log_thread(),
> which can be called twice, to close_log_view(), which is called
> once.  It also destroys the commit_loaded condition variable, which
> is created in open_log_view() but was never destroyed.
> 
> This fixes the abort on FreeBSD.  Comments?

Yes, this looks like the right fix. The log thread can be stopped and
restarted so it makes sense for the condition variables to share their
lifetime with the log view.

> diff c9d2b2638181862042c8d712cf7bf4977fea15e2 /home/naddy/got
> blob - 7b48924a0835e067b5b49dc74d78a60a76d928d4
> file + tog/tog.c
> --- tog/tog.c
> +++ tog/tog.c
> @@ -2032,10 +2032,6 @@ stop_log_thread(struct tog_log_view_state *s)
>  		s->thread = NULL;
>  	}
>  
> -	errcode = pthread_cond_destroy(&s->thread_args.need_commits);
> -	if (errcode && err == NULL)
> -		err = got_error_set_errno(errcode, "pthread_cond_destroy");
> -
>  	if (s->thread_args.repo) {
>  		got_repo_close(s->thread_args.repo);
>  		s->thread_args.repo = NULL;
> @@ -2054,8 +2050,18 @@ close_log_view(struct tog_view *view)
>  {
>  	const struct got_error *err = NULL;
>  	struct tog_log_view_state *s = &view->state.log;
> +	int errcode;
>  
>  	err = stop_log_thread(s);
> +
> +	errcode = pthread_cond_destroy(&s->thread_args.need_commits);
> +	if (errcode && err == NULL)
> +		err = got_error_set_errno(errcode, "pthread_cond_destroy");
> +
> +	errcode = pthread_cond_destroy(&s->thread_args.commit_loaded);
> +	if (errcode && err == NULL)
> +		err = got_error_set_errno(errcode, "pthread_cond_destroy");
> +
>  	free_commits(&s->commits);
>  	free(s->in_repo_path);
>  	s->in_repo_path = NULL;
> -- 
> Christian "naddy" Weisgerber                          naddy@mips.inka.de
> 
>