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.

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)
@@ -1933,7 +1932,8 @@ tog_io_close(void)
 static const struct got_error *
 tog_io_close(void)
 {
-	const struct got_error *err = NULL;
+	const struct got_error	*err = NULL;
+	int			 rc;
 
 	if (tog_io.cin && fclose(tog_io.cin) == EOF)
 		err = got_ferror(tog_io.cin, GOT_ERR_IO);
@@ -1942,6 +1942,10 @@ tog_io_close(void)
 	if (tog_io.f && fclose(tog_io.f) == EOF && err == NULL)
 		err = got_ferror(tog_io.f, GOT_ERR_IO);
 
+	rc = pthread_cond_destroy(&tog_io.blame_is_drawn);
+	if (rc && err == NULL)
+		err = got_error_set_errno(rc, "pthread_cond_destroy");
+
 	return err;
 }
 
@@ -4177,6 +4181,7 @@ init_mock_term(const char *test_script_path)
 init_mock_term(const char *test_script_path)
 {
 	const struct got_error	*err = NULL;
+	int			 rc;
 
 	if (test_script_path == NULL || *test_script_path == '\0')
 		return got_error_msg(GOT_ERR_IO, "TOG_TEST_SCRIPT not defined");
@@ -4211,7 +4216,14 @@ init_mock_term(const char *test_script_path)
 		err = got_error_msg(GOT_ERR_IO,
 		    "newterm: failed to initialise curses");
 
+	rc = pthread_cond_init(&tog_io.blame_is_drawn, NULL);
+	if (rc) {
+		err = got_error_set_errno(rc, "pthread_cond_init");
+		goto done;
+	}
+
 	using_mock_io = 1;
+
 done:
 	if (err)
 		tog_io_close();
@@ -6002,6 +6014,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 +6144,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 +6241,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");

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