Re: [PATCH 5/4] run-command: implement abort_async for pthreads

[Erik Faye-Lund <kusmabite@gmail.com>]

On Fri, Apr 1, 2011 at 7:27 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> 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 */