This got lost on IRC the other day when I should have sent it to the
list: stsp reported that the tog test_log_mark_keymap regression test
would randomly hang, which I was able to reproduce.

This is due to a race condition between the main thread checking the
log_complete flag and the log thread setting it. When the F keymap is
entered to toggle fullscreen, the main thread might need more commits
queued to fully populate the log view when redrawing the screen. So it
first checks the log_complete flag to see if the commit graph has
already been traversed and, if so, doesn't bother waking the log thread
to request any more commits. But if the log_complete flag isn't set, it
does wake the log thread and than waits for it to signal when a commit
is loaded.

In the log thread, after we have finished traversing the commit graph,
we briefly release the mutex before setting the log_complete flag and
this causes the race: the main thread checks and the flag is unset so
it wakes the log thread and then waits on the log thread in
trigger_log_thread() to signal when a new commit is loaded.
In between the main thread checking the flag, the log thread exits
the loop and sets the flag.

The fix sets the flag in the loop before releasing the mutex.
And because we can exit the loop on error or receipt of a fatal signal,
we also keep the existing log_complete assignment outside the loop.


commit 89dc2ce648b3694fe5f8a48991bb0edfd5901360 (main)
from: Mark Jamsek <mark@jamsek.dev>
date: Fri Dec 27 12:24:27 2024 UTC

tog: fix log view race condition evinced by regress

When the commit graph has been traversed, we set the log_complete flag
to signal the main thread that we've completed iterating commit history,
which request_log_commits() checks and, if set, returns early instead of
calling trigger_log_thread(). If called, trigger_log_thread() wakes the
log thread, then waits on the log thread to unblock when another commit
has been loaded. There's a race between the log thread setting the
log_complete flag and request_log_commits() checking it, which causes
trigger_log_thread() to wait on a signal that never unblocks because
there are no more commits to be loaded.

M  tog/tog.c  |  2+  0-

1 file changed, 2 insertions(+), 0 deletions(-)

commit - 6c185310f28c4ac8a4ed01a7e6f2991529017b28
commit + 89dc2ce648b3694fe5f8a48991bb0edfd5901360
blob - b37bfa5a5a29303875829cda627929d35d603f5f
blob + fc81e400b4e1984a83056f6313f5a785f103e6a1
--- tog/tog.c
+++ tog/tog.c
@@ -4003,6 +4003,8 @@ log_thread(void *arg)
 			    TAILQ_FIRST(&a->real_commits->head);
 			*a->selected_entry = *a->first_displayed_entry;
 		}
+		if (done)
+			a->log_complete = 1;
 
 		errcode = pthread_cond_signal(&a->commit_loaded);
 		if (errcode) {


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