Re: [PATCH 5/4] run-command: implement abort_async for pthreads
- From
Erik Faye-Lund <kusmabite@gmail.com>
- Date
- Apr 1, 2011, 19:26 UTC
- Message-ID
- <AANLkTi=8+V3w-1uQgekjntM1qREGZYWSTE_ra1NbDxG8@mail.gmail.com>
- In-Reply-To
- <201104011927.03366.j6t@kdbg.org>
On Fri, Apr 1, 2011 at 7:27 PM, Johannes Sixt <j6t@kdbg.org> wrote:
Show 25 quoted lines
> On Freitag, 1. April 2011, Erik Faye-Lund wrote: >> On Fri, Apr 1, 2011 at 11:41 AM, Erik Faye-Lund <kusmabite@gmail.com> wrote: >> > On Thu, Mar 31, 2011 at 8:45 PM, Jeff King <peff@peff.net> wrote: >> >> kill(async->pid, 15); >> >> #else >> >> - /* no clue */ >> >> + pthread_cancel(async->tid); >> > >> > My worry about terminating a thread that's currently holding a mutex >> > (implicitly through the CRT?) still applies though... >> >> OK, I've read up on thread-cancellation, and this code seems correct. >> pthread_cancel doesn't kill the thread right away, it just signals a >> cancellation-event, which is checked for at certain >> cancellation-points. A lot of the CRT functions are defined as >> cancellation points, so it'll be a matter for us Win32-guys to >> implement pthread_testcancel() and inject that into the >> function-wrappers of the CRT functions that are marked as >> cancellation-points. > > That's not going to happen. We cannot implement pthread_cancel() on Windows > because it would have to be able to interrupt blocking system calls. > (TerminateThread() is a no-no, given all the caveats about leaking system > resources that are mentioned in the manual.) >
Did you read my suggestion? I was talking about implementing cancellation-points, just like on other platforms. This should not lead to TerminateThread, but instead a conditional ExitThread from the thread in question.
Something like this (I've only added a cancellation-point at close, just to illustrate what I mean):
diff --git a/compat/mingw.c b/compat/mingw.c index 878b1de..253be14 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -302,6 +302,14 @@ int mingw_open (const char *filename, int oflags, ...) return fd; } +#undef close +int mingw_close(int fd) +{ + int ret = close(fd); + pthread_testcancel(); + return ret; +} + #undef write ssize_t mingw_write(int fd, const void *buf, size_t count) { diff --git a/compat/mingw.h b/compat/mingw.h index 62eccd3..3e904c8 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -175,6 +175,9 @@ int mingw_rmdir(const char *path); int mingw_open (const char *filename, int oflags, ...); #define open mingw_open +int mingw_close(int fd); +#define close mingw_close + ssize_t mingw_write(int fd, const void *buf, size_t count); #define write mingw_write diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c index 010e875..47d620b 100644 --- a/compat/win32/pthread.c +++ b/compat/win32/pthread.c @@ -18,6 +18,7 @@ static unsigned __stdcall win32_start_routine(void *arg) pthread_t *thread = arg; thread->tid = GetCurrentThreadId(); thread->arg = thread->start_routine(thread->arg); + CloseHandle(thread->cancel_event); return 0; } @@ -26,6 +27,9 @@ int pthread_create(pthread_t *thread, const void *unused, { thread->arg = arg; thread->start_routine = start_routine; + thread->cancel_event = CreateEvent(NULL, FALSE, FALSE, NULL); + if (!thread->cancel_event) + die("failed to create event"); thread->handle = (HANDLE) _beginthreadex(NULL, 0, win32_start_routine, thread, 0, NULL); diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h index 2e20548..7fce39d 100644 --- a/compat/win32/pthread.h +++ b/compat/win32/pthread.h @@ -56,6 +56,7 @@ extern int pthread_cond_broadcast(pthread_cond_t *cond); */ typedef struct { HANDLE handle; + HANDLE cancel_event; void *(*start_routine)(void*); void *arg; DWORD tid; @@ -96,4 +97,13 @@ static inline void *pthread_getspecific(pthread_key_t key) return TlsGetValue(key); } +#define pthread_cancel(a) SetEvent(a.cancel_event) + +static inline void pthread_testcancel(void) +{ + pthread_t thread = pthread_self(); + if (WaitForSingleObject(thread.cancel_event, 0)) + pthread_exit(NULL); +} + #endif /* PTHREAD_H */