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