* Re: [PATCH v2 2/2] pthread: implement try/timed join variants
2015-10-04 19:55 ` [PATCH v2 2/2] pthread: implement try/timed join variants Bobby Bingham
@ 2015-10-12 2:30 ` Rich Felker
2015-10-16 3:15 ` Rich Felker
2016-05-07 18:48 ` [PATCH v3] " Bobby Bingham
1 sibling, 1 reply; 5+ messages in thread
From: Rich Felker @ 2015-10-12 2:30 UTC (permalink / raw)
To: musl
On Sun, Oct 04, 2015 at 02:55:30PM -0500, Bobby Bingham wrote:
> ---
> include/pthread.h | 2 ++
> src/thread/pthread_join.c | 29 ++++++++++++++++++++++-------
> 2 files changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/include/pthread.h b/include/pthread.h
> index af70b73..3d2e0c4 100644
> --- a/include/pthread.h
> +++ b/include/pthread.h
> @@ -214,6 +214,8 @@ struct cpu_set_t;
> int pthread_getaffinity_np(pthread_t, size_t, struct cpu_set_t *);
> int pthread_setaffinity_np(pthread_t, size_t, const struct cpu_set_t *);
> int pthread_getattr_np(pthread_t, pthread_attr_t *);
> +int pthread_tryjoin_np(pthread_t, void **);
> +int pthread_timedjoin_np(pthread_t, void **, const struct timespec *);
> #endif
>
> #ifdef __cplusplus
> diff --git a/src/thread/pthread_join.c b/src/thread/pthread_join.c
> index 694d377..b041ff8 100644
> --- a/src/thread/pthread_join.c
> +++ b/src/thread/pthread_join.c
> @@ -5,18 +5,33 @@ int __munmap(void *, size_t);
> void __pthread_testcancel(void);
> int __pthread_setcancelstate(int, int *);
>
> -int __pthread_join(pthread_t t, void **res)
> +int __pthread_tryjoin_np(pthread_t t, void **res)
> {
> - int tmp, cs;
> - __pthread_testcancel();
> - __pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs);
> - if (cs == PTHREAD_CANCEL_ENABLE) __pthread_setcancelstate(cs, 0);
> - while ((tmp = t->tid)) __timedwait_cp(&t->tid, tmp, 0, 0, 0);
> - __pthread_setcancelstate(cs, 0);
> + if (t->tid) return EBUSY;
> a_barrier();
> if (res) *res = t->result;
> if (t->map_base) __munmap(t->map_base, t->map_size);
> return 0;
> }
This can spuriously fail when lack of memory synchronization causes
the caller not to see that the target thread has exited. However this
condition does not seem observable; calling pthread_tryjoin_np from
multiple threads where the calls could race is certainly UB, and
otherwise there should be no way to distinguish between the state of
just-before-exiting and already-exited. So I _think_ this is okay.
> +int __pthread_timedjoin_np(pthread_t t, void **res, const struct timespec *at)
> +{
> + int tmp, cs, r = 0;
> + __pthread_testcancel();
> + __pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs);
> + if (cs == PTHREAD_CANCEL_ENABLE) __pthread_setcancelstate(cs, 0);
> + while ((tmp = t->tid) && r != ETIMEDOUT && r != EINVAL)
> + r = __timedwait_cp(&t->tid, tmp, CLOCK_REALTIME, at, 0);
> + __pthread_setcancelstate(cs, 0);
> + if (r == ETIMEDOUT || r == EINVAL) return r;
> + return __pthread_tryjoin_np(t, res);
> +}
> +
> +int __pthread_join(pthread_t t, void **res)
> +{
> + return __pthread_timedjoin_np(t, res, 0);
> +}
> +
> +weak_alias(__pthread_tryjoin_np, pthread_tryjoin_np);
> +weak_alias(__pthread_timedjoin_np, pthread_timedjoin_np);
> weak_alias(__pthread_join, pthread_join);
This all looks okay to me. I'll probably commit the patch as-is, as
long as nobody thinks the reasoning on the tryjoin issue above is
wrong.
Rich
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v3] pthread: implement try/timed join variants
2015-10-04 19:55 ` [PATCH v2 2/2] pthread: implement try/timed join variants Bobby Bingham
2015-10-12 2:30 ` Rich Felker
@ 2016-05-07 18:48 ` Bobby Bingham
1 sibling, 0 replies; 5+ messages in thread
From: Bobby Bingham @ 2016-05-07 18:48 UTC (permalink / raw)
To: musl
---
In the interest of unstalling this, here's a new, less invasive version.
In the previous patch, timejoin waited for the thread to exit, and tail
called to tryjoin which cleaned up after the thread and retrieved the
thread result. The regular join was just a timedjoin with an infinite
timeout.
Now, timedjoin has all the logic for waiting, cleaning up after the
thread, and retrieving the result. Regular join is still just a
timedjoin with an infinite timeout. However, tryjoin now just checks if
the thread has exited and either returns EBUSY or calls join, depending.
include/pthread.h | 2 ++
src/thread/pthread_join.c | 20 +++++++++++++++++---
2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/include/pthread.h b/include/pthread.h
index af70b73..3d2e0c4 100644
--- a/include/pthread.h
+++ b/include/pthread.h
@@ -214,6 +214,8 @@ struct cpu_set_t;
int pthread_getaffinity_np(pthread_t, size_t, struct cpu_set_t *);
int pthread_setaffinity_np(pthread_t, size_t, const struct cpu_set_t *);
int pthread_getattr_np(pthread_t, pthread_attr_t *);
+int pthread_tryjoin_np(pthread_t, void **);
+int pthread_timedjoin_np(pthread_t, void **, const struct timespec *);
#endif
#ifdef __cplusplus
diff --git a/src/thread/pthread_join.c b/src/thread/pthread_join.c
index 694d377..5211148 100644
--- a/src/thread/pthread_join.c
+++ b/src/thread/pthread_join.c
@@ -5,18 +5,32 @@ int __munmap(void *, size_t);
void __pthread_testcancel(void);
int __pthread_setcancelstate(int, int *);
-int __pthread_join(pthread_t t, void **res)
+int __pthread_timedjoin_np(pthread_t t, void **res, const struct timespec *at)
{
- int tmp, cs;
+ int tmp, cs, r = 0;
__pthread_testcancel();
__pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs);
if (cs == PTHREAD_CANCEL_ENABLE) __pthread_setcancelstate(cs, 0);
- while ((tmp = t->tid)) __timedwait_cp(&t->tid, tmp, 0, 0, 0);
+ while ((tmp = t->tid) && r != ETIMEDOUT && r != EINVAL)
+ r = __timedwait_cp(&t->tid, tmp, CLOCK_REALTIME, at, 0);
__pthread_setcancelstate(cs, 0);
+ if (r == ETIMEDOUT || r == EINVAL) return r;
a_barrier();
if (res) *res = t->result;
if (t->map_base) __munmap(t->map_base, t->map_size);
return 0;
}
+int __pthread_join(pthread_t t, void **res)
+{
+ return __pthread_timedjoin_np(t, res, 0);
+}
+
+int __pthread_tryjoin_np(pthread_t t, void **res)
+{
+ return t->tid ? EBUSY : __pthread_join(t, res);
+}
+
+weak_alias(__pthread_tryjoin_np, pthread_tryjoin_np);
+weak_alias(__pthread_timedjoin_np, pthread_timedjoin_np);
weak_alias(__pthread_join, pthread_join);
--
2.8.1
^ permalink raw reply [flat|nested] 5+ messages in thread