"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
tog log in shallow Git repositories
To:
gameoftrees@openbsd.org
Date:
Fri, 8 Jul 2022 14:30:13 +0200

Download raw body.

Thread
In a Git repository cloned with git clone --depth=1, tog hangs
on startup and is unresponsive to any input. This requires a
kill -9 to recover from.

The patch below fixes this such that 'tog log' displays the same
error message as 'got log' in such a repository:

$ got log
got: open: /tmp/test-got-clone-shallow/.git/objects/23/0e1f1bfa1d7bbd0dee0217e22bc2817e848ad0: No such file or directory
$ tog
tog: open: /tmp/test-got-clone-shallow/.git/objects/23/0e1f1bfa1d7bbd0dee0217e22bc2817e848ad0: No such file or directory
$ 

To do this properly we need to synchronize start-up of the log thread
with view_input(), otherwise tog will still hang forever in case the log
thread executes in parallel, and runs into an error in queue_commits(), and
exits before we even start waiting for its signal in trigger_log_thread().
This in turns means that mutex locking/unlocking in the log thread needs
to be revised.

Any errors from the log thread need to be propagated up via view_close()
to become visible when tog closes all active views during exit.

There is now a global tog_thread_error flag which should cause tog to
exit and which could also be used to improve error handling for other
threads in the future.

ok?

diff d2366e29426c9d2c88244cf41683f6574928caff c82b2a2333ce71095f7eaf674c13c22bcfc9db5e
commit - d2366e29426c9d2c88244cf41683f6574928caff
commit + c82b2a2333ce71095f7eaf674c13c22bcfc9db5e
blob - 7dc49c638a583244ce89554bdfee75bd523d1a99
blob + 17cc60e73fceae8247da28708b2bf8895bdfc2df
--- tog/tog.c
+++ tog/tog.c
@@ -337,6 +337,7 @@ struct tog_diff_view_state {
 };
 
 pthread_mutex_t tog_mutex = PTHREAD_MUTEX_INITIALIZER;
+static volatile sig_atomic_t tog_thread_error;
 
 struct tog_log_thread_args {
 	pthread_cond_t need_commits;
@@ -657,10 +658,10 @@ tog_fatal_signal_received(void)
 static const struct got_error *
 view_close(struct tog_view *view)
 {
-	const struct got_error *err = NULL;
+	const struct got_error *err = NULL, *child_err = NULL;
 
 	if (view->child) {
-		view_close(view->child);
+		child_err = view_close(view->child);
 		view->child = NULL;
 	}
 	if (view->close)
@@ -670,7 +671,7 @@ view_close(struct tog_view *view)
 	if (view->window)
 		delwin(view->window);
 	free(view);
-	return err;
+	return err ? err : child_err;
 }
 
 static struct tog_view *
@@ -1361,7 +1362,8 @@ view_loop(struct tog_view *view)
 		return err;
 	update_panels();
 	doupdate();
-	while (!TAILQ_EMPTY(&views) && !done && !tog_fatal_signal_received()) {
+	while (!TAILQ_EMPTY(&views) && !done && !tog_thread_error &&
+	    !tog_fatal_signal_received()) {
 		/* Refresh fast during initialization, then become slower. */
 		if (fast_refresh && fast_refresh-- == 0)
 			halfdelay(10); /* switch to once per second */
@@ -1460,9 +1462,12 @@ view_loop(struct tog_view *view)
 	}
 done:
 	while (!TAILQ_EMPTY(&views)) {
+		const struct got_error *close_err;
 		view = TAILQ_FIRST(&views);
 		TAILQ_REMOVE(&views, view, entry);
-		view_close(view);
+		close_err = view_close(view);
+		if (close_err && err == NULL)
+			err = close_err;
 	}
 
 	errcode = pthread_mutex_unlock(&tog_mutex);
@@ -2198,10 +2203,8 @@ trigger_log_thread(struct tog_view *view, int wait)
 
 	halfdelay(1); /* fast refresh while loading commits */
 
-	while (ta->commits_needed > 0 || ta->load_all) {
-		if (ta->log_complete)
-			break;
-
+	while (!ta->log_complete && !tog_thread_error &&
+	    (ta->commits_needed > 0 || ta->load_all)) {
 		/* Wake the log thread. */
 		errcode = pthread_cond_signal(&ta->need_commits);
 		if (errcode)
@@ -2476,15 +2479,33 @@ log_thread(void *arg)
 	struct tog_log_thread_args *a = arg;
 	int done = 0;
 
-	err = block_signals_used_by_main_thread();
-	if (err)
+	/*
+	 * Sync startup with main thread such that we begin our
+	 * work once view_input() has released the mutex.
+	 */
+	errcode = pthread_mutex_lock(&tog_mutex);
+	if (errcode) {
+		err = got_error_set_errno(errcode, "pthread_mutex_lock");
 		return (void *)err;
+	}
 
+	err = block_signals_used_by_main_thread();
+	if (err) {
+		pthread_mutex_unlock(&tog_mutex);
+		goto done;
+	}
+
 	while (!done && !err && !tog_fatal_signal_received()) {
+		errcode = pthread_mutex_unlock(&tog_mutex);
+		if (errcode) {
+			err = got_error_set_errno(errcode,
+			    "pthread_mutex_unlock");
+			goto done;
+		}
 		err = queue_commits(a);
 		if (err) {
 			if (err->code != GOT_ERR_ITER_COMPLETED)
-				return (void *)err;
+				goto done;
 			err = NULL;
 			done = 1;
 		} else if (a->commits_needed > 0 && !a->load_all)
@@ -2494,7 +2515,7 @@ log_thread(void *arg)
 		if (errcode) {
 			err = got_error_set_errno(errcode,
 			    "pthread_mutex_lock");
-			break;
+			goto done;
 		} else if (*a->quit)
 			done = 1;
 		else if (*a->first_displayed_entry == NULL) {
@@ -2508,7 +2529,7 @@ log_thread(void *arg)
 			err = got_error_set_errno(errcode,
 			    "pthread_cond_signal");
 			pthread_mutex_unlock(&tog_mutex);
-			break;
+			goto done;
 		}
 
 		if (done)
@@ -2517,27 +2538,33 @@ log_thread(void *arg)
 			if (a->commits_needed == 0 && !a->load_all) {
 				errcode = pthread_cond_wait(&a->need_commits,
 				    &tog_mutex);
-				if (errcode)
+				if (errcode) {
 					err = got_error_set_errno(errcode,
 					    "pthread_cond_wait");
+					pthread_mutex_unlock(&tog_mutex);
+					goto done;
+				}
 				if (*a->quit)
 					done = 1;
 			}
 		}
-
-		errcode = pthread_mutex_unlock(&tog_mutex);
-		if (errcode && err == NULL)
-			err = got_error_set_errno(errcode,
-			    "pthread_mutex_unlock");
 	}
 	a->log_complete = 1;
+	errcode = pthread_mutex_unlock(&tog_mutex);
+	if (errcode)
+		err = got_error_set_errno(errcode, "pthread_mutex_unlock");
+done:
+	if (err) {
+		tog_thread_error = 1;
+		pthread_cond_signal(&a->commit_loaded);
+	}
 	return (void *)err;
 }
 
 static const struct got_error *
 stop_log_thread(struct tog_log_view_state *s)
 {
-	const struct got_error *err = NULL;
+	const struct got_error *err = NULL, *thread_err = NULL;
 	int errcode;
 
 	if (s->thread) {
@@ -2550,7 +2577,7 @@ stop_log_thread(struct tog_log_view_state *s)
 		if (errcode)
 			return got_error_set_errno(errcode,
 			    "pthread_mutex_unlock");
-		errcode = pthread_join(s->thread, (void **)&err);
+		errcode = pthread_join(s->thread, (void **)&thread_err);
 		if (errcode)
 			return got_error_set_errno(errcode, "pthread_join");
 		errcode = pthread_mutex_lock(&tog_mutex);
@@ -2578,7 +2605,7 @@ stop_log_thread(struct tog_log_view_state *s)
 		s->thread_args.graph = NULL;
 	}
 
-	return err;
+	return err ? err : thread_err;
 }
 
 static const struct got_error *