From: Mark Jamsek <mark@jamsek.com>
Subject: Re: tog: basic diff regress
To: Game of Trees <gameoftrees@openbsd.org>
Date: Sun, 16 Apr 2023 19:03:00 +1000

On 23-04-16 05:58PM, Mark Jamsek wrote:
> On 23-04-15 05:23PM, Stefan Sperling wrote:
> > On Sat, Apr 15, 2023 at 09:46:51PM +1000, Mark Jamsek wrote:
> > > The below diff (which applies on top of main) adds a new WAIT_FOR_UI
> > > instruction to the test harness, and a basic blame regress test that
> > > demonstrates its use.  When the instruction is received a flag is set,
> > > which delays processing further instructions till it is unset, which, in
> > > this case, is when the blame process is complete. This can easily be
> > > extrapolated to future cases that require waiting till the UI has been
> > > drawn.
> > 
> > Looks fine. ok
> > 
> > I would have expected pthread_cond_wait() to be involved while waiting.
> > This would require more than one WAIT-style instruction to identify
> > which condition the main thread should for.
> > I guess the way you implemented this we will essentially busy-loop
> > in the view input handlers until the variable is flipped by the other
> > thread? That is a bit less elegant than waiting via pthread APIs.
> > But of course if it works it can be good enough for tests. And in
> > any case it is better than sleeping for some fixed amount of time.
> 
> Sure, that would be better than busy waiting the view loop! And if we
> keep the wait_for_ui flag, we can actually keep the general WAIT_FOR_UI
> instruction and wait conditions will just be context dependent. So we
> can use WAIT_FOR_UI irrespective of the view.

Please disregard the previous diff:

diff /home/mark/src/got
commit - c736b84ab8efb53399d58afe57a2e40c4c7dd1b5
path + /home/mark/src/got
blob - 8c21271a99fe7416ffc1a6b97ce8ce11403a6f35
file + tog/tog.c
--- tog/tog.c
+++ tog/tog.c
@@ -614,10 +614,11 @@ struct tog_io {
 
 /* curses io for tog regress */
 struct tog_io {
-	FILE	*cin;
-	FILE	*cout;
-	FILE	*f;
-	int	 wait_for_ui;
+	FILE		*cin;
+	FILE		*cout;
+	FILE		*f;
+	int		 wait_for_ui;
+	pthread_cond_t	 blame_is_drawn;
 } tog_io;
 static int using_mock_io;
 
@@ -1632,9 +1633,6 @@ tog_read_script_key(FILE *script, int *ch, int *done)
 
 	*ch = -1;
 
-	if (tog_io.wait_for_ui)
-		goto done;
-
 	if (getline(&line, &linesz, script) == -1) {
 		if (feof(script)) {
 			*done = 1;
@@ -1644,6 +1642,7 @@ tog_read_script_key(FILE *script, int *ch, int *done)
 			goto done;
 		}
 	}
+
 	if (strncasecmp(line, "WAIT_FOR_UI", 11) == 0)
 		tog_io.wait_for_ui = 1;
 	else if (strncasecmp(line, "KEY_ENTER", 9) == 0)
@@ -4212,6 +4211,7 @@ done:
 		    "newterm: failed to initialise curses");
 
 	using_mock_io = 1;
+
 done:
 	if (err)
 		tog_io_close();
@@ -6002,6 +6002,15 @@ draw_blame(struct tog_view *view)
 	if (view->gline > blame->nlines)
 		view->gline = blame->nlines;
 
+	if (tog_io.wait_for_ui) {
+		int rc;
+
+		rc = pthread_cond_wait(&tog_io.blame_is_drawn, &tog_mutex);
+		if (rc)
+			return got_error_set_errno(rc, "pthread_cond_wait");
+		tog_io.wait_for_ui = 0;
+	}
+
 	if (asprintf(&line, "[%d/%d] %s%s", view->gline ? view->gline :
 	    s->first_displayed_line - 1 + s->selected_line, blame->nlines,
 	    s->blame_complete ? "" : "annotating... ", s->path) == -1) {
@@ -6123,11 +6132,6 @@ draw_blame(struct tog_view *view)
 
 	view_border(view);
 
-	if (tog_io.wait_for_ui) {
-		if (s->blame_complete)
-			tog_io.wait_for_ui = 0;
-	}
-
 	return NULL;
 }
 
@@ -6225,6 +6229,14 @@ blame_thread(void *arg)
 	ta->repo = NULL;
 	*ta->complete = 1;
 
+	if (tog_io.wait_for_ui) {
+		int rc;
+
+		rc = pthread_cond_signal(&tog_io.blame_is_drawn);
+		if (rc && err == NULL)
+			err = got_error_set_errno(rc, "pthread_cond_signal");
+	}
+
 	errcode = pthread_mutex_unlock(&tog_mutex);
 	if (errcode && err == NULL)
 		err = got_error_set_errno(errcode, "pthread_mutex_unlock");
@@ -6505,6 +6517,14 @@ open_blame_view(struct tog_view *view, char *path,
 	view->search_setup = search_setup_blame_view;
 	view->search_next = search_next_view_match;
 
+	if (using_mock_io) {
+		int rc;
+
+		rc = pthread_cond_init(&tog_io.blame_is_drawn, NULL);
+		if (rc)
+			return got_error_set_errno(rc, "pthread_cond_init");
+	}
+
 	return run_blame(view);
 }
 
@@ -6524,6 +6544,14 @@ close_blame_view(struct tog_view *view)
 		got_object_qid_free(blamed_commit);
 	}
 
+	if (using_mock_io) {
+		int rc;
+
+		rc = pthread_cond_destroy(&tog_io.blame_is_drawn);
+		if (rc && err == NULL)
+			err = got_error_set_errno(rc, "pthread_cond_destroy");
+	}
+
 	free(s->path);
 	free_colors(&s->colors);
 	return err;

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