Merge pull request #4010 from libgit2/ethomson/clar_threads

Introduce some clar helpers for child threads

8 files changed, 122 insertions, 12 deletions

diff --git a/src/global.h b/src/global.h
index 219951525..88f40aad1 100644
--- a/src/global.h
+++ b/src/global.h
@@ -16,6 +16,12 @@ typedef struct {
 	git_error error_t;
 	git_buf error_buf;
 	char oid_fmt[GIT_OID_HEXSZ+1];
+
+	/* On Windows, this is the current child thread that was started by
+	 * `git_thread_create`.  This is used to set the thread's exit code
+	 * when terminated by `git_thread_exit`.  It is unused on POSIX.
+	 */
+	git_thread *current_thread;
 } git_global_st;
 
 #ifdef GIT_OPENSSL
diff --git a/src/unix/pthread.h b/src/unix/pthread.h
index 0f3f17927..3f23d10d5 100644
--- a/src/unix/pthread.h
+++ b/src/unix/pthread.h
@@ -17,6 +17,8 @@ typedef struct {
 	pthread_create(&(git_thread_ptr)->thread, NULL, start_routine, arg)
 #define git_thread_join(git_thread_ptr, status) \
 	pthread_join((git_thread_ptr)->thread, status)
+#define git_thread_currentid() ((size_t)(pthread_self()))
+#define git_thread_exit(retval) pthread_exit(retval)
 
 /* Git Mutex */
 #define git_mutex pthread_mutex_t
diff --git a/src/win32/thread.c b/src/win32/thread.c
index 80d56ce5d..87318c9d3 100644
--- a/src/win32/thread.c
+++ b/src/win32/thread.c
@@ -26,6 +26,9 @@ static DWORD WINAPI git_win32__threadproc(LPVOID lpParameter)
 {
 	git_thread *thread = lpParameter;
 
+	/* Set the current thread for `git_thread_exit` */
+	GIT_GLOBAL->current_thread = thread;
+
 	thread->result = thread->proc(thread->param);
 
 	git__free_tls_data();
@@ -95,6 +98,21 @@ int git_thread_join(
 	return 0;
 }
 
+void git_thread_exit(void *value)
+{
+	assert(GIT_GLOBAL->current_thread);
+	GIT_GLOBAL->current_thread->result = value;
+
+	git__free_tls_data();
+
+	ExitThread(CLEAN_THREAD_EXIT);
+}
+
+size_t git_thread_currentid(void)
+{
+	return GetCurrentThreadId();
+}
+
 int git_mutex_init(git_mutex *GIT_RESTRICT mutex)
 {
 	InitializeCriticalSection(mutex);
diff --git a/src/win32/thread.h b/src/win32/thread.h
index 0d01822a6..7f4a2170f 100644
--- a/src/win32/thread.h
+++ b/src/win32/thread.h
@@ -41,6 +41,8 @@ int git_thread_create(git_thread *GIT_RESTRICT,
 	void *(*) (void *),
 	void *GIT_RESTRICT);
 int git_thread_join(git_thread *, void **);
+size_t git_thread_currentid(void);
+void git_thread_exit(void *);
 
 int git_mutex_init(git_mutex *GIT_RESTRICT mutex);
 int git_mutex_free(git_mutex *);
diff --git a/tests/clar_libgit2.h b/tests/clar_libgit2.h
index d7e635302..663d1362a 100644
--- a/tests/clar_libgit2.h
+++ b/tests/clar_libgit2.h
@@ -41,6 +41,51 @@
 	} \
 	} while(0)
 
+/**
+ * Thread safe assertions; you cannot use `cl_git_report_failure` from a
+ * child thread since it will try to `longjmp` to abort and "the effect of
+ * a call to longjmp() where initialization of the jmp_buf structure was
+ * not performed in the calling thread is undefined."
+ *
+ * Instead, callers can provide a clar thread error context to a thread,
+ * which will populate and return it on failure.  Callers can check the
+ * status with `cl_git_thread_check`.
+ */
+typedef struct {
+	int error;
+	const char *file;
+	int line;
+	const char *expr;
+	char error_msg[4096];
+} cl_git_thread_err;
+
+#ifdef GIT_THREADS
+# define cl_git_thread_pass(threaderr, expr) cl_git_thread_pass_(threaderr, (expr), __FILE__, __LINE__)
+#else
+# define cl_git_thread_pass(threaderr, expr) cl_git_pass(expr)
+#endif
+
+#define cl_git_thread_pass_(__threaderr, __expr, __file, __line) do { \
+	giterr_clear(); \
+	if ((((cl_git_thread_err *)__threaderr)->error = (__expr)) != 0) { \
+		const git_error *_last = giterr_last(); \
+		((cl_git_thread_err *)__threaderr)->file = __file; \
+		((cl_git_thread_err *)__threaderr)->line = __line; \
+		((cl_git_thread_err *)__threaderr)->expr = "Function call failed: " #__expr; \
+		p_snprintf(((cl_git_thread_err *)__threaderr)->error_msg, 4096, "thread 0x%" PRIxZ " - error %d - %s", \
+			git_thread_currentid(), ((cl_git_thread_err *)__threaderr)->error, \
+			_last ? _last->message : "<no message>"); \
+		git_thread_exit(__threaderr); \
+	} \
+	} while (0)
+
+static void cl_git_thread_check(void *data)
+{
+	cl_git_thread_err *threaderr = (cl_git_thread_err *)data;
+	if (threaderr->error != 0)
+		clar__assert(0, threaderr->file, threaderr->line, threaderr->expr, threaderr->error_msg, 1);
+}
+
 void cl_git_report_failure(int, const char *, int, const char *);
 
 #define cl_assert_at_line(expr,file,line) \
diff --git a/tests/core/init.c b/tests/core/init.c
index cd90b378d..a8cbd930b 100644
--- a/tests/core/init.c
+++ b/tests/core/init.c
@@ -39,14 +39,14 @@ void test_core_init__concurrent_init_succeeds(void)
 	git_thread threads[10];
 	unsigned i;
 
-	cl_assert_equal_i(0, git_libgit2_shutdown());
+	cl_assert_equal_i(2, git_libgit2_init());
 
 	for (i = 0; i < ARRAY_SIZE(threads); i++)
 		git_thread_create(&threads[i], reinit, NULL);
 	for (i = 0; i < ARRAY_SIZE(threads); i++)
 		git_thread_join(&threads[i], NULL);
 
-	cl_assert_equal_i(1, git_libgit2_init());
+	cl_assert_equal_i(1, git_libgit2_shutdown());
 	cl_sandbox_set_search_path_defaults();
 #else
 	cl_skip();
diff --git a/tests/threads/basic.c b/tests/threads/basic.c
index 9c342bc42..a9310bbd4 100644
--- a/tests/threads/basic.c
+++ b/tests/threads/basic.c
@@ -48,3 +48,36 @@ void test_threads_basic__set_error(void)
 {
 	run_in_parallel(1, 4, set_error, NULL, NULL);
 }
+
+#ifdef GIT_THREADS
+static void *return_normally(void *param)
+{
+	return param;
+}
+
+static void *exit_abruptly(void *param)
+{
+	git_thread_exit(param);
+	return NULL;
+}
+#endif
+
+void test_threads_basic__exit(void)
+{
+#ifndef GIT_THREADS
+	clar__skip();
+#else
+	git_thread thread;
+	void *result;
+
+	/* Ensure that the return value of the threadproc is returned. */
+	cl_git_pass(git_thread_create(&thread, return_normally, (void *)424242));
+	cl_git_pass(git_thread_join(&thread, &result));
+	cl_assert_equal_sz(424242, (size_t)result);
+
+	/* Ensure that the return value of `git_thread_exit` is returned. */
+	cl_git_pass(git_thread_create(&thread, return_normally, (void *)232323));
+	cl_git_pass(git_thread_join(&thread, &result));
+	cl_assert_equal_sz(232323, (size_t)result);
+#endif
+}
diff --git a/tests/threads/refdb.c b/tests/threads/refdb.c
index 5484b71d6..e2f7563c7 100644
--- a/tests/threads/refdb.c
+++ b/tests/threads/refdb.c
@@ -22,6 +22,7 @@ void test_threads_refdb__cleanup(void)
 #define NREFS 10
 
 struct th_data {
+	cl_git_thread_err error;
 	int id;
 	const char *path;
 };
@@ -34,11 +35,11 @@ static void *iterate_refs(void *arg)
 	int count = 0, error;
 	git_repository *repo;
 
-	cl_git_pass(git_repository_open(&repo, data->path));
+	cl_git_thread_pass(data, git_repository_open(&repo, data->path));
 	do {
 		error = git_reference_iterator_new(&i, repo);
 	} while (error == GIT_ELOCKED);
-	cl_git_pass(error);
+	cl_git_thread_pass(data, error);
 
 	for (count = 0; !git_reference_next(&ref, i); ++count) {
 		cl_assert(ref != NULL);
@@ -64,26 +65,27 @@ static void *create_refs(void *arg)
 	git_reference *ref[NREFS];
 	git_repository *repo;
 
-	cl_git_pass(git_repository_open(&repo, data->path));
+	cl_git_thread_pass(data, git_repository_open(&repo, data->path));
 
 	do {
 		error = git_reference_name_to_id(&head, repo, "HEAD");
 	} while (error == GIT_ELOCKED);
-	cl_git_pass(error);
+	cl_git_thread_pass(data, error);
 
 	for (i = 0; i < NREFS; ++i) {
 		p_snprintf(name, sizeof(name), "refs/heads/thread-%03d-%02d", data->id, i);
 		do {
 			error = git_reference_create(&ref[i], repo, name, &head, 0, NULL);
 		} while (error == GIT_ELOCKED);
-		cl_git_pass(error);
+		cl_git_thread_pass(data, error);
 
 		if (i == NREFS/2) {
 			git_refdb *refdb;
-			cl_git_pass(git_repository_refdb(&refdb, repo));
+			cl_git_thread_pass(data, git_repository_refdb(&refdb, repo));
 			do {
 				error = git_refdb_compress(refdb);
 			} while (error == GIT_ELOCKED);
+			cl_git_thread_pass(data, error);
 			git_refdb_free(refdb);
 		}
 	}
@@ -105,7 +107,7 @@ static void *delete_refs(void *arg)
 	char name[128];
 	git_repository *repo;
 
-	cl_git_pass(git_repository_open(&repo, data->path));
+	cl_git_thread_pass(data, git_repository_open(&repo, data->path));
 
 	for (i = 0; i < NREFS; ++i) {
 		p_snprintf(
@@ -119,17 +121,17 @@ static void *delete_refs(void *arg)
 			if (error == GIT_ENOTFOUND)
 				error = 0;
 
-			cl_git_pass(error);
+			cl_git_thread_pass(data, error);
 			git_reference_free(ref);
 		}
 
 		if (i == NREFS/2) {
 			git_refdb *refdb;
-			cl_git_pass(git_repository_refdb(&refdb, repo));
+			cl_git_thread_pass(data, git_repository_refdb(&refdb, repo));
 			do {
 				error = git_refdb_compress(refdb);
 			} while (error == GIT_ELOCKED);
-			cl_git_pass(error);
+			cl_git_thread_pass(data, error);
 			git_refdb_free(refdb);
 		}
 	}
@@ -194,6 +196,7 @@ void test_threads_refdb__edit_while_iterate(void)
 #ifdef GIT_THREADS
 	for (t = 0; t < THREADS; ++t) {
 		cl_git_pass(git_thread_join(&th[t], NULL));
+		cl_git_thread_check(&th_data[t]);
 	}
 
 	memset(th, 0, sizeof(th));
@@ -205,6 +208,7 @@ void test_threads_refdb__edit_while_iterate(void)
 
 	for (t = 0; t < THREADS; ++t) {
 		cl_git_pass(git_thread_join(&th[t], NULL));
+		cl_git_thread_check(&th_data[t]);
 	}
 #endif
 }