Skip to content

Commit 7626550

Browse files
committed
Merge branch 'js/async-thread'
* js/async-thread: fast-import: die_nicely() back to vsnprintf (reverts part of ebaa79f) Enable threaded async procedures whenever pthreads is available Dying in an async procedure should only exit the thread, not the process. Reimplement async procedures using pthreads Windows: more pthreads functions Fix signature of fcntl() compatibility dummy Make report() from usage.c public as vreportf() and use it. Modernize t5530-upload-pack-error. Conflicts: http-backend.c
2 parents 8d676d8 + 3e33303 commit 7626550

10 files changed

Lines changed: 117 additions & 41 deletions

File tree

Documentation/technical/api-run-command.txt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,8 +231,9 @@ The function pointer in .proc has the following signature:
231231

232232

233233
There are serious restrictions on what the asynchronous function can do
234-
because this facility is implemented by a pipe to a forked process on
235-
UNIX, but by a thread in the same address space on Windows:
234+
because this facility is implemented by a thread in the same address
235+
space on most platforms (when pthreads is available), but by a pipe to
236+
a forked process otherwise:
236237

237238
. It cannot change the program's state (global variables, environment,
238239
etc.) in a way that the caller notices; in other words, .in and .out

compat/mingw.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ static inline int getuid()
8989
{ return 1; }
9090
static inline struct passwd *getpwnam(const char *name)
9191
{ return NULL; }
92-
static inline int fcntl(int fd, int cmd, long arg)
92+
static inline int fcntl(int fd, int cmd, ...)
9393
{
9494
if (cmd == F_GETFD || cmd == F_SETFD)
9595
return 0;

compat/win32/pthread.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
static unsigned __stdcall win32_start_routine(void *arg)
1717
{
1818
pthread_t *thread = arg;
19+
thread->tid = GetCurrentThreadId();
1920
thread->arg = thread->start_routine(thread->arg);
2021
return 0;
2122
}
@@ -49,6 +50,13 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr)
4950
}
5051
}
5152

53+
pthread_t pthread_self(void)
54+
{
55+
pthread_t t = { 0 };
56+
t.tid = GetCurrentThreadId();
57+
return t;
58+
}
59+
5260
int pthread_cond_init(pthread_cond_t *cond, const void *unused)
5361
{
5462
cond->waiters = 0;

compat/win32/pthread.h

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ typedef struct {
5858
HANDLE handle;
5959
void *(*start_routine)(void*);
6060
void *arg;
61+
DWORD tid;
6162
} pthread_t;
6263

6364
extern int pthread_create(pthread_t *thread, const void *unused,
@@ -71,4 +72,28 @@ extern int pthread_create(pthread_t *thread, const void *unused,
7172

7273
extern int win32_pthread_join(pthread_t *thread, void **value_ptr);
7374

75+
#define pthread_equal(t1, t2) ((t1).tid == (t2).tid)
76+
extern pthread_t pthread_self(void);
77+
78+
static inline int pthread_exit(void *ret)
79+
{
80+
ExitThread((DWORD)ret);
81+
}
82+
83+
typedef DWORD pthread_key_t;
84+
static inline int pthread_key_create(pthread_key_t *keyp, void (*destructor)(void *value))
85+
{
86+
return (*keyp = TlsAlloc()) == TLS_OUT_OF_INDEXES ? EAGAIN : 0;
87+
}
88+
89+
static inline int pthread_setspecific(pthread_key_t key, const void *value)
90+
{
91+
return TlsSetValue(key, (void *)value) ? 0 : EINVAL;
92+
}
93+
94+
static inline void *pthread_getspecific(pthread_key_t key)
95+
{
96+
return TlsGetValue(key);
97+
}
98+
7499
#endif /* PTHREAD_H */

git-compat-util.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ extern char *gitbasename(char *);
200200
#include "compat/bswap.h"
201201

202202
/* General helper functions */
203+
extern void vreportf(const char *prefix, const char *err, va_list params);
203204
extern NORETURN void usage(const char *err);
204205
extern NORETURN void usagef(const char *err, ...) __attribute__((format (printf, 1, 2)));
205206
extern NORETURN void die(const char *err, ...) __attribute__((format (printf, 1, 2)));

http-backend.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -488,14 +488,12 @@ static NORETURN void die_webcgi(const char *err, va_list params)
488488
static int dead;
489489

490490
if (!dead) {
491-
char buffer[1000];
492491
dead = 1;
493-
494-
vsnprintf(buffer, sizeof(buffer), err, params);
495-
fprintf(stderr, "fatal: %s\n", buffer);
496492
http_status(500, "Internal Server Error");
497493
hdr_nocache();
498494
end_headers();
495+
496+
vreportf("fatal: ", err, params);
499497
}
500498
exit(0); /* we successfully reported a failure ;-) */
501499
}

run-command.c

Lines changed: 57 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,14 @@ static NORETURN void die_child(const char *err, va_list params)
8484
unused = write(child_err, "\n", 1);
8585
exit(128);
8686
}
87+
#endif
8788

8889
static inline void set_cloexec(int fd)
8990
{
9091
int flags = fcntl(fd, F_GETFD);
9192
if (flags >= 0)
9293
fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
9394
}
94-
#endif
9595

9696
static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure)
9797
{
@@ -449,11 +449,35 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const
449449
return run_command(&cmd);
450450
}
451451

452-
#ifdef WIN32
453-
static unsigned __stdcall run_thread(void *data)
452+
#ifndef NO_PTHREADS
453+
static pthread_t main_thread;
454+
static int main_thread_set;
455+
static pthread_key_t async_key;
456+
457+
static void *run_thread(void *data)
454458
{
455459
struct async *async = data;
456-
return async->proc(async->proc_in, async->proc_out, async->data);
460+
intptr_t ret;
461+
462+
pthread_setspecific(async_key, async);
463+
ret = async->proc(async->proc_in, async->proc_out, async->data);
464+
return (void *)ret;
465+
}
466+
467+
static NORETURN void die_async(const char *err, va_list params)
468+
{
469+
vreportf("fatal: ", err, params);
470+
471+
if (!pthread_equal(main_thread, pthread_self())) {
472+
struct async *async = pthread_getspecific(async_key);
473+
if (async->proc_in >= 0)
474+
close(async->proc_in);
475+
if (async->proc_out >= 0)
476+
close(async->proc_out);
477+
pthread_exit((void *)128);
478+
}
479+
480+
exit(128);
457481
}
458482
#endif
459483

@@ -499,7 +523,7 @@ int start_async(struct async *async)
499523
else
500524
proc_out = -1;
501525

502-
#ifndef WIN32
526+
#ifdef NO_PTHREADS
503527
/* Flush stdio before fork() to avoid cloning buffers */
504528
fflush(NULL);
505529

@@ -526,12 +550,29 @@ int start_async(struct async *async)
526550
else if (async->out)
527551
close(async->out);
528552
#else
553+
if (!main_thread_set) {
554+
/*
555+
* We assume that the first time that start_async is called
556+
* it is from the main thread.
557+
*/
558+
main_thread_set = 1;
559+
main_thread = pthread_self();
560+
pthread_key_create(&async_key, NULL);
561+
set_die_routine(die_async);
562+
}
563+
564+
if (proc_in >= 0)
565+
set_cloexec(proc_in);
566+
if (proc_out >= 0)
567+
set_cloexec(proc_out);
529568
async->proc_in = proc_in;
530569
async->proc_out = proc_out;
531-
async->tid = (HANDLE) _beginthreadex(NULL, 0, run_thread, async, 0, NULL);
532-
if (!async->tid) {
533-
error("cannot create thread: %s", strerror(errno));
534-
goto error;
570+
{
571+
int err = pthread_create(&async->tid, NULL, run_thread, async);
572+
if (err) {
573+
error("cannot create thread: %s", strerror(err));
574+
goto error;
575+
}
535576
}
536577
#endif
537578
return 0;
@@ -551,17 +592,15 @@ int start_async(struct async *async)
551592

552593
int finish_async(struct async *async)
553594
{
554-
#ifndef WIN32
555-
int ret = wait_or_whine(async->pid, "child process", 0);
595+
#ifdef NO_PTHREADS
596+
return wait_or_whine(async->pid, "child process", 0);
556597
#else
557-
DWORD ret = 0;
558-
if (WaitForSingleObject(async->tid, INFINITE) != WAIT_OBJECT_0)
559-
ret = error("waiting for thread failed: %lu", GetLastError());
560-
else if (!GetExitCodeThread(async->tid, &ret))
561-
ret = error("cannot get thread exit code: %lu", GetLastError());
562-
CloseHandle(async->tid);
598+
void *ret = (void *)(intptr_t)(-1);
599+
600+
if (pthread_join(async->tid, &ret))
601+
error("pthread_join failed");
602+
return (int)(intptr_t)ret;
563603
#endif
564-
return ret;
565604
}
566605

567606
int run_hook(const char *index_file, const char *name, ...)

run-command.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
#ifndef RUN_COMMAND_H
22
#define RUN_COMMAND_H
33

4+
#ifndef NO_PTHREADS
5+
#include <pthread.h>
6+
#endif
7+
48
struct child_process {
59
const char **argv;
610
pid_t pid;
@@ -74,10 +78,10 @@ struct async {
7478
void *data;
7579
int in; /* caller writes here and closes it */
7680
int out; /* caller reads from here and closes it */
77-
#ifndef WIN32
81+
#ifdef NO_PTHREADS
7882
pid_t pid;
7983
#else
80-
HANDLE tid;
84+
pthread_t tid;
8185
int proc_in;
8286
int proc_out;
8387
#endif

t/t5530-upload-pack-error.sh

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ test_expect_success 'fsck fails' '
3232

3333
test_expect_success 'upload-pack fails due to error in pack-objects packing' '
3434
35-
! echo "0032want $(git rev-parse HEAD)
36-
00000009done
37-
0000" | git upload-pack . > /dev/null 2> output.err &&
35+
printf "0032want %s\n00000009done\n0000" \
36+
$(git rev-parse HEAD) >input &&
37+
test_must_fail git upload-pack . <input >/dev/null 2>output.err &&
3838
grep "unable to read" output.err &&
3939
grep "pack-objects died" output.err
4040
'
@@ -51,9 +51,9 @@ test_expect_success 'fsck fails' '
5151
'
5252
test_expect_success 'upload-pack fails due to error in rev-list' '
5353
54-
! echo "0032want $(git rev-parse HEAD)
55-
0034shallow $(git rev-parse HEAD^)00000009done
56-
0000" | git upload-pack . > /dev/null 2> output.err &&
54+
printf "0032want %s\n0034shallow %s00000009done\n0000" \
55+
$(git rev-parse HEAD) $(git rev-parse HEAD^) >input &&
56+
test_must_fail git upload-pack . <input >/dev/null 2>output.err &&
5757
# pack-objects survived
5858
grep "Total.*, reused" output.err &&
5959
# but there was an error, which must have been in rev-list
@@ -62,9 +62,9 @@ test_expect_success 'upload-pack fails due to error in rev-list' '
6262

6363
test_expect_success 'upload-pack fails due to error in pack-objects enumeration' '
6464
65-
! echo "0032want $(git rev-parse HEAD)
66-
00000009done
67-
0000" | git upload-pack . > /dev/null 2> output.err &&
65+
printf "0032want %s\n00000009done\n0000" \
66+
$(git rev-parse HEAD) >input &&
67+
test_must_fail git upload-pack . <input >/dev/null 2>output.err &&
6868
grep "bad tree object" output.err &&
6969
grep "pack-objects died" output.err
7070
'

usage.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*/
66
#include "git-compat-util.h"
77

8-
static void report(const char *prefix, const char *err, va_list params)
8+
void vreportf(const char *prefix, const char *err, va_list params)
99
{
1010
char msg[4096];
1111
vsnprintf(msg, sizeof(msg), err, params);
@@ -14,24 +14,24 @@ static void report(const char *prefix, const char *err, va_list params)
1414

1515
static NORETURN void usage_builtin(const char *err, va_list params)
1616
{
17-
report("usage: ", err, params);
17+
vreportf("usage: ", err, params);
1818
exit(129);
1919
}
2020

2121
static NORETURN void die_builtin(const char *err, va_list params)
2222
{
23-
report("fatal: ", err, params);
23+
vreportf("fatal: ", err, params);
2424
exit(128);
2525
}
2626

2727
static void error_builtin(const char *err, va_list params)
2828
{
29-
report("error: ", err, params);
29+
vreportf("error: ", err, params);
3030
}
3131

3232
static void warn_builtin(const char *warn, va_list params)
3333
{
34-
report("warning: ", warn, params);
34+
vreportf("warning: ", warn, params);
3535
}
3636

3737
/* If we are in a dlopen()ed .so write to a global variable would segfault

0 commit comments

Comments
 (0)