"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Martin Pieuchot <mpi@openbsd.org>
Subject:
Re: tog yield commit reverted
To:
gameoftrees@openbsd.org
Date:
Tue, 4 Feb 2020 10:58:43 +0100

Download raw body.

Thread
On 03/02/20(Mon) 19:13, Stefan Sperling wrote:
> FYI,
> 
> I had to revert commit 6b8a2b8fcd99 because it broke tog search.
> It's quite likely that this could be fixed differently?

That means pthread_yield() is used for synchronisation, which isn't
guaranteed.

Do you have more details about the "breakage"?  Where was the other
thread stuck?  Or why couldn't it make progress?

> -----------------------------------------------
> commit 82954512f323c8a2d4f89d51be1e6b0f707b6c3a (master, noel/master, origin/master)
> from: Stefan Sperling <stsp@stsp.name>
> date: Mon Feb  3 18:10:18 2020 UTC
>  
>  revert 6b8a2b8fcd99c4365b1aa9513c0f0149beac2491; it broke 'n' (search next)
>  
>  In the log view, 'n' sometimes failed to search for the next matching commit
>  and tog dead-locked. So a yield or mutex unlock/lock is required for search
>  to work. Perhaps we need a different fix?
>  
> diff 044a69d55c4202e3cfefd2d420edfdf6de1e881a 4a3424f61f444454c2c44245fd95ffdea17c2dac
> blob - ca9591498c8d4a77dd452d5394cd10141c63e07f
> blob + 99c464bbb1c0922bc325ee00100e39b70f857f7f
> --- tog/tog.c
> +++ tog/tog.c
> @@ -755,6 +755,15 @@ view_input(struct tog_view **new, struct tog_view **de
>  	*focus = NULL;
>  
>  	if (view->searching && !view->search_next_done) {
> +		errcode = pthread_mutex_unlock(&tog_mutex);
> +		if (errcode)
> +			return got_error_set_errno(errcode,
> +			    "pthread_mutex_unlock");
> +		pthread_yield();
> +		errcode = pthread_mutex_lock(&tog_mutex);
> +		if (errcode)
> +			return got_error_set_errno(errcode,
> +			    "pthread_mutex_lock");
>  		view->search_next(view);
>  		return NULL;
>  	}
> @@ -1633,6 +1642,16 @@ trigger_log_thread(int load_all, int *commits_needed, 
>  		if (errcode)
>  			return got_error_set_errno(errcode,
>  			    "pthread_cond_signal");
> +		errcode = pthread_mutex_unlock(&tog_mutex);
> +		if (errcode)
> +			return got_error_set_errno(errcode,
> +			    "pthread_mutex_unlock");
> +		pthread_yield();
> +		errcode = pthread_mutex_lock(&tog_mutex);
> +		if (errcode)
> +			return got_error_set_errno(errcode,
> +			    "pthread_mutex_lock");
> +
>  		if (*commits_needed > 0 && (!load_all || --max_wait <= 0)) {
>  			/*
>  			 * Thread is not done yet; lose a key press
> 
>