On Fri, Apr 1, 2011 at 9:26 PM, Erik Faye-Lund <kusmabite@xxxxxxxxx> wrote:
> On Fri, Apr 1, 2011 at 7:27 PM, Johannes Sixt <j6t@xxxxxxxx> wrote:
>> On Freitag, 1. April 2011, Erik Faye-Lund wrote:
>>> On Fri, Apr 1, 2011 at 11:41 AM, Erik Faye-Lund <kusmabite@xxxxxxxxx> wrote:
>>> > On Thu, Mar 31, 2011 at 8:45 PM, Jeff King <peff@xxxxxxxx> 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))
That should be:
- if (WaitForSingleObject(thread.cancel_event, 0))
+ if (WaitForSingleObject(thread.cancel_event, 0) == WAIT_OBJECT_0)
...Of course.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html