Mikhail <mp39590@gmail.com> wrote:
> tog (in current got repo)
> /Mikhail - (it will land you on [151/151]
> n - (this must land you on [152/x])
> tog starts eating 100% cpu
> 
> Proposed fix:
> 
> diff /home/misha/work/got
> commit - 0778bf802f073bf7f785d53ab5ea4d8e6a8f0a59
> path + /home/misha/work/got
> blob - 7da57bf9f6480d998efd8205b57ef7814af17182
> file + tog/tog.c
> --- tog/tog.c
> +++ tog/tog.c
> @@ -3700,6 +3700,22 @@ search_next_log_view(struct tog_view *view)
>  		else
>  			entry = TAILQ_PREV(s->selected_entry, commit_queue_head,
>  			    entry);
> +
> +		/*
> +		 * There is scenario when matched_entry and selected_entry are
> +		 * the last in the queue of currently loaded commits, if so, we
> +		 * must request loading more commits to proceed with the search,
> +		 * if we leave 'entry' as NULL, we always will end up in this
> +		 * branch of 'if' and won't do any actual matching in the code
> +		 * below.
> +		 *
> +		 * To overcome this we set search_entry to selected_entry and
> +		 * make next iteration of this function to land in previous
> +		 * 'if' branch, which allows commits to be actually loaded.
> +		 */
> +		if (entry == NULL)
> +			s->search_entry = s->selected_entry;
> +
>  	} else {
>  		entry = s->selected_entry;
>  	}

Thanks, Mikhail! I can indeed reproduce, and your diff fixes the issue.
I'm inclined to move the s->search_entry assignment down to where we
poke the log thread, though, as we are already describing the process
there so it feels like the natural place (and then we don't really need
to provide further commentary); what do you think?

I would also like to add a regress for this specific case, too; however,
I don't have the time right now so would still like to send this fix
with the caveat that a test should be added as soon as time permits,
which I'll prioritise.

Thank you for finding and diagnosing this bug. I'm surprised it hasn't
come up sooner :)


-----------------------------------------------
commit b19108d1c8a2b0e0565280ab5d13bd43486f5c94 (main)
from: Mikhail <mp39590@gmail.com>
via: Mark Jamsek <mark@jamsek.dev>
date: Fri Aug 11 06:17:49 2023 UTC
 
 tog: fix log view search infinite loop
 
 When the last matched and last selected entry is the last loaded commit,
 we keep looping the same code path as search_entry is always NULL.
 Before poking the log thread for more commits, set search_entry to the
 currently selected commit, which is where the search resumes.
 
 M  tog/tog.c  |  1+  0-

1 file changed, 1 insertion(+), 0 deletions(-)

diff 0778bf802f073bf7f785d53ab5ea4d8e6a8f0a59 b19108d1c8a2b0e0565280ab5d13bd43486f5c94
commit - 0778bf802f073bf7f785d53ab5ea4d8e6a8f0a59
commit + b19108d1c8a2b0e0565280ab5d13bd43486f5c94
blob - 7da57bf9f6480d998efd8205b57ef7814af17182
blob + c4839d1c9023a068753142f189620d39826b0968
--- tog/tog.c
+++ tog/tog.c
@@ -3721,6 +3721,7 @@ search_next_log_view(struct tog_view *view)
 			 * allowing the main loop to make progress. Search
 			 * will resume at s->search_entry once we come back.
 			 */
+			s->search_entry = s->selected_entry;
 			s->thread_args.commits_needed++;
 			return trigger_log_thread(view, 0);
 		}


-- 
Mark Jamsek <https://bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68