Module: xenomai-forge
Branch: next
Commit: 75cc5dac959c06bac3a5730ae279ae7a0a4ad271
URL:    
http://git.xenomai.org/?p=xenomai-forge.git;a=commit;h=75cc5dac959c06bac3a5730ae279ae7a0a4ad271

Author: Philippe Gerum <r...@xenomai.org>
Date:   Mon Aug  4 21:38:33 2014 +0200

copperplate: work-around glibc race in cancel state handling

This is a SMP race which seems to affect NPTL-based glibc releases
(2.9 and 2.18 checked, other releases are likely concerned as
well). The issue requires SMP setups to pop up, when
--enable-async-cancel is mentioned on the Xenomai build configuration
line. It was detected over the Mercury core, not checked on the Cobalt
core. The work-around will fit both.

The bug creates a situation where
pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, ...) does not actually
prevent subsequent asynchronous cancellation of the caller, in the
following scenario:

threadB has its cancellability set to PTHREAD_CANCEL_ENABLE,
PTHREAD_CANCEL_ASYNCHRONOUS on entry:

threadA[CPU0]: pthread_cancel(threadB)
                (SIGCANCEL queued to threadB[CPU1])
threadB[CPU1]: pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL)
                (SIGCANCEL delivered to threadB[CPU1])
                <sigcancel_handler>
                        if (cancel_type(threadB) == PTHREAD_CANCEL_ASYNCHRONOUS)
                                __do_cancel();
                        ...

As illustrated, the source of this issue is in glibc's SIGCANCEL
handler, which only (re-)checks the cancellability TYPE prior to
entering the thread deletion process on the target CPU, omitting the
cancellability STATE from this check. High concurrency may therefore
cause the target CPU to receive an asynchronous cancellation signal
out of sync, which contradicts the current cancellability state of the
recipient thread, due to the delayed signal propagation induced by SMP
activities.

Considering this, the cancellability state seems only safe for
disabling the internal cancellation points, but can't be relied on for
blocking asynchronous requests in SMP configurations.

The work-around is to make sure to turn the cancellability type to
PTHREAD_CANCEL_DEFERRED in addition to disabling cancellation with
PTHREAD_CANCEL_DISABLE.

Copperplate, and all Copperplate-based APIs are affected by the
change.

---

 include/boilerplate/lock.h      |    5 +-
 include/copperplate/threadobj.h |    3 +-
 lib/alchemy/task.c              |   41 ++++++++----
 lib/alchemy/testsuite/heap-1.c  |    6 +-
 lib/alchemy/testsuite/mq-3.c    |    1 +
 lib/boilerplate/ancillaries.c   |    9 +--
 lib/copperplate/init.c          |   16 +++--
 lib/copperplate/registry.c      |   25 +++++--
 lib/copperplate/threadobj.c     |   45 +++++++------
 lib/copperplate/traceobj.c      |   51 +++++++++++----
 lib/psos/task.c                 |  137 ++++++++++++++++++++++++++-------------
 lib/psos/testsuite/tm-3.c       |    1 +
 lib/trank/native.c              |    1 +
 lib/vxworks/errnoLib.c          |   27 ++++++--
 lib/vxworks/kernLib.c           |    2 +-
 lib/vxworks/taskInfo.c          |   46 +++++++++----
 lib/vxworks/taskLib.c           |   84 ++++++++++++++++--------
 lib/vxworks/taskLib.h           |    4 +-
 18 files changed, 350 insertions(+), 154 deletions(-)

diff --git a/include/boilerplate/lock.h b/include/boilerplate/lock.h
index 6f0218c..3738fa2 100644
--- a/include/boilerplate/lock.h
+++ b/include/boilerplate/lock.h
@@ -105,8 +105,8 @@ int __check_cancel_type(const char *locktype);
 
 #define __do_lock_nocancel(__lock, __type, __op)                       \
        ({                                                              \
-               int __ret = __bt(__check_cancel_type(#__type));         \
-               __ret ?: __do_lock(__lock, __op);                       \
+               __bt(__check_cancel_type(#__op "_nocancel"));           \
+               __do_lock(__lock, __op);                                \
        })
 
 #define __do_unlock(__lock)                                    \
@@ -167,6 +167,7 @@ int __check_cancel_type(const char *locktype);
 #define __do_lock_safe(__lock, __state, __op)                          \
        ({                                                              \
                int __ret, __oldstate;                                  \
+               __bt(__check_cancel_type(#__op "_safe"));               \
                pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &__oldstate); \
                __ret = -__RT(pthread_mutex_##__op(__lock));            \
                if (__ret)                                              \
diff --git a/include/copperplate/threadobj.h b/include/copperplate/threadobj.h
index 73eb791..3a707e5 100644
--- a/include/copperplate/threadobj.h
+++ b/include/copperplate/threadobj.h
@@ -295,7 +295,8 @@ int threadobj_init(struct threadobj *thobj,
 
 int threadobj_start(struct threadobj *thobj) __must_check;
 
-void threadobj_shadow(struct threadobj *thobj);
+int threadobj_shadow(struct threadobj *thobj,
+                    const char *name);
 
 int threadobj_prologue(struct threadobj *thobj,
                       const char *name);
diff --git a/lib/alchemy/task.c b/lib/alchemy/task.c
index 18ceb35..e8ac1cf 100644
--- a/lib/alchemy/task.c
+++ b/lib/alchemy/task.c
@@ -206,36 +206,41 @@ static int task_prologue_1(void *arg)
 
 static int task_prologue_2(struct alchemy_task *tcb)
 {
-       struct service svc;
        int ret;
 
-       CANCEL_DEFER(svc);
-
        threadobj_wait_start();
-
        threadobj_lock(&tcb->thobj);
        ret = threadobj_set_mode(0, tcb->mode, NULL);
        threadobj_unlock(&tcb->thobj);
 
-       CANCEL_RESTORE(svc);
-
        return ret;
 }
 
 static void *task_entry(void *arg)
 {
        struct alchemy_task *tcb = arg;
+       struct service svc;
        int ret;
 
+       CANCEL_DEFER(svc);
+
        ret = __bt(task_prologue_2(tcb));
-       if (ret)
+       if (ret) {
+               CANCEL_RESTORE(svc);
                return (void *)(long)ret;
+       }
 
        threadobj_notify_entry();
+
+       CANCEL_RESTORE(svc);
+
        tcb->entry(tcb->arg);
+
+       CANCEL_DEFER(svc);
        threadobj_lock(&tcb->thobj);
        threadobj_set_magic(&tcb->thobj, ~task_magic);
        threadobj_unlock(&tcb->thobj);
+       CANCEL_RESTORE(svc);
 
        return NULL;
 }
@@ -740,12 +745,14 @@ int rt_task_shadow(RT_TASK *task, const char *name, int 
prio, int mode)
        if (ret)
                goto out;
 
-       threadobj_shadow(&tcb->thobj);  /* We won't wait in prologue. */
+       CANCEL_RESTORE(svc);
 
-       ret = threadobj_prologue(&tcb->thobj, tcb->name);
+       ret = threadobj_shadow(&tcb->thobj, tcb->name);
        if (ret)
                goto undo;
 
+       CANCEL_DEFER(svc);
+
        ret = task_prologue_2(tcb);
        if (ret)
                goto undo;
@@ -1247,7 +1254,13 @@ int rt_task_set_priority(RT_TASK *task, int prio)
        policy = prio ? SCHED_FIFO : SCHED_OTHER;
        param_ex.sched_priority = prio;
        ret = threadobj_set_schedparam(&tcb->thobj, policy, &param_ex);
-       put_alchemy_task(tcb);
+       switch (ret) {
+       case -EIDRM:
+               ret = 0;
+               break;
+       default:
+               put_alchemy_task(tcb);
+       }
 out:
        CANCEL_RESTORE(svc);
 
@@ -1376,7 +1389,13 @@ int rt_task_slice(RT_TASK *task, RTIME quantum)
                policy = param_ex.sched_priority ? SCHED_FIFO : SCHED_OTHER;
 
        ret = threadobj_set_schedparam(&tcb->thobj, policy, &param_ex);
-       put_alchemy_task(tcb);
+       switch (ret) {
+       case -EIDRM:
+               ret = 0;
+               break;
+       default:
+               put_alchemy_task(tcb);
+       }
 out:
        CANCEL_RESTORE(svc);
 
diff --git a/lib/alchemy/testsuite/heap-1.c b/lib/alchemy/testsuite/heap-1.c
index d7667b3..c116f1f 100644
--- a/lib/alchemy/testsuite/heap-1.c
+++ b/lib/alchemy/testsuite/heap-1.c
@@ -90,13 +90,13 @@ int main(int argc, char *const argv[])
 
        traceobj_init(&trobj, argv[0], sizeof(tseq) / sizeof(int));
 
-       ret = rt_task_create(&t_bgnd, "BGND", 0,  20, 0);
+       ret = rt_task_create(&t_fgnd, "FGND", 0,  21, 0);
        traceobj_assert(&trobj, ret == 0);
 
-       ret = rt_task_start(&t_bgnd, background_task, NULL);
+       ret = rt_task_create(&t_bgnd, "BGND", 0,  20, 0);
        traceobj_assert(&trobj, ret == 0);
 
-       ret = rt_task_create(&t_fgnd, "FGND", 0,  21, 0);
+       ret = rt_task_start(&t_bgnd, background_task, NULL);
        traceobj_assert(&trobj, ret == 0);
 
        ret = rt_task_start(&t_fgnd, foreground_task, NULL);
diff --git a/lib/alchemy/testsuite/mq-3.c b/lib/alchemy/testsuite/mq-3.c
index 3a66eec..dc1ec96 100644
--- a/lib/alchemy/testsuite/mq-3.c
+++ b/lib/alchemy/testsuite/mq-3.c
@@ -82,6 +82,7 @@ static void peer_task(void *arg)
 
        traceobj_mark(&trobj, 9);
 
+       /* Valgrind will bark at this one, this is expected. */
        ret = rt_queue_read(&q, &msg, sizeof(msg), TM_INFINITE);
        traceobj_assert(&trobj, ret == -EINVAL);
 
diff --git a/lib/boilerplate/ancillaries.c b/lib/boilerplate/ancillaries.c
index 09b88d9..e4cd735 100644
--- a/lib/boilerplate/ancillaries.c
+++ b/lib/boilerplate/ancillaries.c
@@ -225,19 +225,14 @@ __weak void *__main_heap = NULL;
 
 int __check_cancel_type(const char *locktype)
 {
-       int oldtype, oldstate;
+       int oldtype;
 
        pthread_setcanceltype(PTHREAD_CANCEL_DEFERRED, &oldtype);
        if (oldtype == PTHREAD_CANCEL_DEFERRED)
                return 0;
 
-       pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldstate);
-       if (oldstate == PTHREAD_CANCEL_DISABLE)
-               return 0;
-
+       warning("%s() section is NOT cancel-safe", locktype);
        abort();
-       warning("%s_nocancel() section is NOT cancel-safe", locktype);
-       pthread_setcancelstate(oldstate, NULL);
 
        return __bt(-EINVAL);
 }
diff --git a/lib/copperplate/init.c b/lib/copperplate/init.c
index 10889e8..665e6f4 100644
--- a/lib/copperplate/init.c
+++ b/lib/copperplate/init.c
@@ -496,6 +496,7 @@ void copperplate_init(int *argcp, char *const **argvp)
        struct option *options;
        static int init_done;
        char **uargv = NULL;
+       struct service svc;
 
        if (init_done) {
                warning("duplicate call to %s() ignored", __func__);
@@ -604,12 +605,19 @@ void copperplate_init(int *argcp, char *const **argvp)
 
        free(options);
 
+       CANCEL_DEFER(svc);
+
        pvlist_for_each_entry(skin, &skins, __reserved.next) {
                ret = skin->init();
-               if (ret) {
-                       warning("skin %s won't initialize", skin->name);
-                       goto fail;
-               }
+               if (ret)
+                       break;
+       }
+
+       CANCEL_RESTORE(svc);
+
+       if (ret) {
+               warning("skin %s won't initialize", skin->name);
+               goto fail;
        }
 
 #ifdef CONFIG_XENO_DEBUG
diff --git a/lib/copperplate/registry.c b/lib/copperplate/registry.c
index 03035dd..f37162f 100644
--- a/lib/copperplate/registry.c
+++ b/lib/copperplate/registry.c
@@ -367,6 +367,7 @@ static int regfs_open(const char *path, struct 
fuse_file_info *fi)
        struct regfs_data *p = regfs_get_context();
        struct pvhashobj *hobj;
        struct fsobj *fsobj;
+       struct service svc;
        int ret = 0;
        void *priv;
 
@@ -395,8 +396,11 @@ static int regfs_open(const char *path, struct 
fuse_file_info *fi)
                priv = NULL;
 
        fi->fh = (uintptr_t)priv;
-       if (fsobj->ops->open)
+       if (fsobj->ops->open) {
+               CANCEL_DEFER(svc);
                ret = __bt(fsobj->ops->open(fsobj, priv));
+               CANCEL_RESTORE(svc);
+       }
 done:
        read_unlock(&p->lock);
        pop_cleanup_lock(&p->lock);
@@ -409,6 +413,7 @@ static int regfs_release(const char *path, struct 
fuse_file_info *fi)
        struct regfs_data *p = regfs_get_context();
        struct pvhashobj *hobj;
        struct fsobj *fsobj;
+       struct service svc;
        int ret = 0;
        void *priv;
 
@@ -423,8 +428,11 @@ static int regfs_release(const char *path, struct 
fuse_file_info *fi)
 
        fsobj = container_of(hobj, struct fsobj, hobj);
        priv = (void *)(uintptr_t)fi->fh;
-       if (fsobj->ops->release)
+       if (fsobj->ops->release) {
+               CANCEL_DEFER(svc);
                ret = __bt(fsobj->ops->release(fsobj, priv));
+               CANCEL_RESTORE(svc);
+       }
        if (priv)
                __STD(free(priv));
 done:
@@ -440,6 +448,7 @@ static int regfs_read(const char *path, char *buf, size_t 
size, off_t offset,
        struct regfs_data *p = regfs_get_context();
        struct pvhashobj *hobj;
        struct fsobj *fsobj;
+       struct service svc;
        void *priv;
        int ret;
 
@@ -461,7 +470,9 @@ static int regfs_read(const char *path, char *buf, size_t 
size, off_t offset,
        read_lock(&fsobj->lock);
        read_unlock(&p->lock);
        priv = (void *)(uintptr_t)fi->fh;
+       CANCEL_DEFER(svc);
        ret = fsobj->ops->read(fsobj, buf, size, offset, priv);
+       CANCEL_RESTORE(svc);
        read_unlock(&fsobj->lock);
        pop_cleanup_lock(&fsobj->lock);
 
@@ -474,6 +485,7 @@ static int regfs_write(const char *path, const char *buf, 
size_t size, off_t off
        struct regfs_data *p = regfs_get_context();
        struct pvhashobj *hobj;
        struct fsobj *fsobj;
+       struct service svc;
        void *priv;
        int ret;
 
@@ -495,7 +507,9 @@ static int regfs_write(const char *path, const char *buf, 
size_t size, off_t off
        read_lock(&fsobj->lock);
        read_unlock(&p->lock);
        priv = (void *)(uintptr_t)fi->fh;
+       CANCEL_DEFER(svc);
        ret = fsobj->ops->write(fsobj, buf, size, offset, priv);
+       CANCEL_RESTORE(svc);
        read_unlock(&fsobj->lock);
        pop_cleanup_lock(&fsobj->lock);
 
@@ -902,10 +916,12 @@ static int collect_wait_list(struct fsobstack *o,
        struct threadobj *thobj;
        struct syncstate syns;
        struct obstack cache;
+       struct service svc;
        int count, ret;
        void *p, *e;
 
        obstack_init(&cache);
+       CANCEL_DEFER(svc);
 redo:
        smp_rmb();
        count = *wait_count;
@@ -919,8 +935,8 @@ redo:
 
        ret = syncobj_lock(sobj, &syns);
        if (ret) {
-               obstack_free(&cache, NULL);
-               return ret;
+               count = ret;
+               goto out;
        }
 
        /* Re-validate the previous item count under lock. */
@@ -955,6 +971,7 @@ redo:
                p += ops->format_data(o, p);
        while (p < e);
 out:
+       CANCEL_RESTORE(svc);
        obstack_free(&cache, NULL);
 
        return count;
diff --git a/lib/copperplate/threadobj.c b/lib/copperplate/threadobj.c
index 870e81e..f3780df 100644
--- a/lib/copperplate/threadobj.c
+++ b/lib/copperplate/threadobj.c
@@ -698,7 +698,7 @@ int __threadobj_lock_sched(struct threadobj *current) /* 
current->lock held */
        current->core.schedparam_unlocked = current->schedparam;
        current->core.policy_unlocked = current->policy;
        param_ex.sched_priority = threadobj_lock_prio;
-       ret = __bt(threadobj_set_schedparam(current, SCHED_FIFO, &param_ex));
+       ret = threadobj_set_schedparam(current, SCHED_FIFO, &param_ex);
        if (ret)
                return __bt(ret);
 done:
@@ -868,10 +868,15 @@ static int request_setschedparam(struct threadobj *thobj, 
int policy,
         * operation, as libcobalt may switch us to secondary mode
         * when doing so (i.e. libc call to reflect the new priority
         * on the linux side).
+        *
+        * If we can't relock the target thread, this must mean that
+        * it vanished in the meantime: return -EIDRM for the caller
+        * to handle this case specifically.
         */
        threadobj_unlock(thobj);
-       ret = __bt(copperplate_renice_local_thread(thobj->ptid, policy, 
param_ex));
-       threadobj_lock(thobj);
+       ret = copperplate_renice_local_thread(thobj->ptid, policy, param_ex);
+       if (threadobj_lock(thobj))
+               ret = -EIDRM;
 
        return ret;
 }
@@ -1097,14 +1102,6 @@ int threadobj_start(struct threadobj *thobj)     /* 
thobj->lock held. */
        return ret;
 }
 
-void threadobj_shadow(struct threadobj *thobj)
-{
-       assert(thobj != threadobj_current());
-       threadobj_lock(thobj);
-       thobj->status |= __THREAD_S_STARTED|__THREAD_S_ACTIVE;
-       threadobj_unlock(thobj);
-}
-
 void threadobj_wait_start(void) /* current->lock free. */
 {
        struct threadobj *current = threadobj_current();
@@ -1135,12 +1132,14 @@ void threadobj_notify_entry(void) /* current->lock 
free. */
        threadobj_unlock(current);
 }
 
-/* thobj->lock free, cancellation disabled. */
+/* thobj->lock free. */
 int threadobj_prologue(struct threadobj *thobj, const char *name)
 {
        struct threadobj *current = threadobj_current();
        int ret;
 
+       pthread_setcanceltype(PTHREAD_CANCEL_DEFERRED, NULL);
+
        /*
         * Check whether we overlay the default main TCB we set in
         * main_overlay(), releasing it if so.
@@ -1154,8 +1153,7 @@ int threadobj_prologue(struct threadobj *thobj, const 
char *name)
                assert(current->magic == 0);
                sysgroup_remove(thread, &current->memspec);
                finalize_thread(current);
-       } else
-               pthread_setcanceltype(PTHREAD_CANCEL_DEFERRED, NULL);
+       }
 
        if (name)
                namecpy(thobj->name, name);
@@ -1196,6 +1194,17 @@ int threadobj_prologue(struct threadobj *thobj, const 
char *name)
        return 0;
 }
 
+int threadobj_shadow(struct threadobj *thobj, const char *name)
+{
+       assert(thobj != threadobj_current());
+       threadobj_lock(thobj);
+       assert((thobj->status & (__THREAD_S_STARTED|__THREAD_S_ACTIVE)) == 0);
+       thobj->status |= __THREAD_S_STARTED|__THREAD_S_ACTIVE;
+       threadobj_unlock(thobj);
+
+       return __bt(threadobj_prologue(thobj, name));
+}
+
 /*
  * Most traditional RTOSes guarantee that the task/thread delete
  * operation is strictly synchronous, i.e. the deletion service
@@ -1498,7 +1507,7 @@ int threadobj_set_schedparam(struct threadobj *thobj, int 
policy,
         */
        ret = request_setschedparam(thobj, _policy, param_ex);
        if (ret)
-               return __bt(ret);
+               return ret;
 
        /*
         * XXX: only local threads may switch to SCHED_RR since both
@@ -1527,7 +1536,7 @@ int threadobj_set_schedparam(struct threadobj *thobj, int 
policy,
 int threadobj_set_schedprio(struct threadobj *thobj, int priority)
 {                              /* thobj->lock held */
        struct sched_param_ex param_ex;
-       int policy, ret;
+       int policy;
 
        __threadobj_check_locked(thobj);
 
@@ -1538,9 +1547,7 @@ int threadobj_set_schedprio(struct threadobj *thobj, int 
priority)
        if (policy == SCHED_RR)
                param_ex.sched_rr_quantum = thobj->tslice;
 
-       ret = __bt(threadobj_set_schedparam(thobj, policy, &param_ex));
-
-       return ret;
+       return threadobj_set_schedparam(thobj, policy, &param_ex);
 }
 
 static inline int main_overlay(void)
diff --git a/lib/copperplate/traceobj.c b/lib/copperplate/traceobj.c
index ed05111..280bf4c 100644
--- a/lib/copperplate/traceobj.c
+++ b/lib/copperplate/traceobj.c
@@ -114,6 +114,9 @@ static void compare_marks(struct traceobj *trobj, int 
tseq[], int nr_seq) /* loc
 void traceobj_verify(struct traceobj *trobj, int tseq[], int nr_seq)
 {
        int end_mark, mark, state;
+       struct service svc;
+
+       CANCEL_DEFER(svc);
 
        read_lock_safe(&trobj->lock, state);
 
@@ -135,6 +138,9 @@ void traceobj_verify(struct traceobj *trobj, int tseq[], 
int nr_seq)
        }
 out:
        read_unlock_safe(&trobj->lock, state);
+
+       CANCEL_RESTORE(svc);
+
        return;
 
 fail:
@@ -146,6 +152,9 @@ fail:
        warning("mismatching execution sequence detected");
        compare_marks(trobj, tseq, nr_seq);
        read_unlock_safe(&trobj->lock, state);
+
+       CANCEL_RESTORE(svc);
+
 #ifdef CONFIG_XENO_MERCURY
        /*
         * The Mercury core does not force any affinity, which may
@@ -189,12 +198,18 @@ static void dump_marks(struct traceobj *trobj) /* lock 
held */
 void __traceobj_assert_failed(struct traceobj *trobj,
                              const char *file, int line, const char *cond)
 {
+       struct service svc;
+
+       CANCEL_DEFER(svc);
+
        push_cleanup_lock(&trobj->lock);
        read_lock(&trobj->lock);
        dump_marks(trobj);
        read_unlock(&trobj->lock);
        pop_cleanup_lock(&trobj->lock);
 
+       CANCEL_RESTORE(svc);
+
        panic("trace assertion failed:\n%s:%d => \"%s\"", file, line, cond);
 }
 
@@ -202,8 +217,11 @@ void __traceobj_mark(struct traceobj *trobj,
                     const char *file, int line, int mark)
 {
        struct tracemark *tmk;
+       struct service svc;
        int cur_mark;
 
+       CANCEL_DEFER(svc);
+
        pthread_testcancel();
        push_cleanup_lock(&trobj->lock);
        write_lock(&trobj->lock);
@@ -222,41 +240,46 @@ void __traceobj_mark(struct traceobj *trobj,
 
        write_unlock(&trobj->lock);
        pop_cleanup_lock(&trobj->lock);
+
+       CANCEL_RESTORE(svc);
 }
 
 void traceobj_enter(struct traceobj *trobj)
 {
        struct threadobj *current = threadobj_current();
+       struct service svc;
 
-       if (current) {
-               threadobj_lock(current);
+       if (current)
                current->tracer = trobj;
-               threadobj_unlock(current);
-       }
 
-       /*
-        * Our caller is usually out of any protected section, so push
-        * a cleanup routine.
-        */
-       push_cleanup_lock(&trobj->lock);
-       write_lock(&trobj->lock);
+       CANCEL_DEFER(svc);
+
+       write_lock_nocancel(&trobj->lock);
+
        if (++trobj->nr_threads == 0)
                trobj->nr_threads = 1;
+
        write_unlock(&trobj->lock);
-       pop_cleanup_lock(&trobj->lock);
+
+       CANCEL_RESTORE(svc);
 }
 
 /* May be directly called from finalizer. */
 void traceobj_unwind(struct traceobj *trobj)
 {
+       struct service svc;
        int state;
 
+       CANCEL_DEFER(svc);
+
        write_lock_safe(&trobj->lock, state);
 
        if (--trobj->nr_threads <= 0)
                __RT(pthread_cond_signal(&trobj->join));
 
        write_unlock_safe(&trobj->lock, state);
+
+       CANCEL_RESTORE(svc);
 }
 
 void traceobj_exit(struct traceobj *trobj)
@@ -271,6 +294,10 @@ void traceobj_exit(struct traceobj *trobj)
 
 void traceobj_join(struct traceobj *trobj)
 {
+       struct service svc;
+
+       CANCEL_DEFER(svc);
+
        push_cleanup_lock(&trobj->lock);
        read_lock(&trobj->lock);
 
@@ -279,4 +306,6 @@ void traceobj_join(struct traceobj *trobj)
 
        read_unlock(&trobj->lock);
        pop_cleanup_lock(&trobj->lock);
+
+       CANCEL_RESTORE(svc);
 }
diff --git a/lib/psos/task.c b/lib/psos/task.c
index 89ba917..2c0cbca 100644
--- a/lib/psos/task.c
+++ b/lib/psos/task.c
@@ -186,9 +186,7 @@ static void *task_trampoline(void *arg)
        struct service svc;
 
        CANCEL_DEFER(svc);
-
        threadobj_wait_start();
-
        threadobj_lock(&task->thobj);
 
        if (task->mode & T_TSLICE) {
@@ -201,14 +199,16 @@ static void *task_trampoline(void *arg)
                __threadobj_lock_sched(&task->thobj);
 
        threadobj_unlock(&task->thobj);
-
+       threadobj_notify_entry();
        CANCEL_RESTORE(svc);
 
-       threadobj_notify_entry();
        args->entry(args->arg0, args->arg1, args->arg2, args->arg3);
+
+       CANCEL_DEFER(svc);
        threadobj_lock(&task->thobj);
        threadobj_set_magic(&task->thobj, ~task_magic);
        threadobj_unlock(&task->thobj);
+       CANCEL_RESTORE(svc);
 
        return NULL;
 }
@@ -365,11 +365,14 @@ u_long t_start(u_long tid,
               u_long args[])
 {
        struct psos_task *task;
+       struct service svc;
        int ret;
 
+       CANCEL_DEFER(svc);
+
        task = get_psos_task(tid, &ret);
        if (task == NULL)
-               return ret;
+               goto out;
 
        task->args.entry = entry;
        if (args) {
@@ -385,10 +388,19 @@ u_long t_start(u_long tid,
        }
        task->mode = mode;
        ret = threadobj_start(&task->thobj);
-       if (ret != -EIDRM)
+       switch (ret) {
+       case -EIDRM:
+               ret = SUCCESS;
+               break;
+       default:
+               ret = ERR_OBJDEL;
+       case 0: /* == SUCCESS */
                put_psos_task(task);
+       }
+ out:
+       CANCEL_RESTORE(svc);
 
-       return SUCCESS;
+       return ret;
 }
 
 u_long t_suspend(u_long tid)
@@ -397,19 +409,21 @@ u_long t_suspend(u_long tid)
        struct service svc;
        int ret;
 
+       CANCEL_DEFER(svc);
+
        task = get_psos_task_or_self(tid, &ret);
        if (task == NULL)
-               return ret;
+               goto out;
 
-       CANCEL_DEFER(svc);
        ret = threadobj_suspend(&task->thobj);
-       CANCEL_RESTORE(svc);
-       put_psos_task(task);
-
        if (ret)
-               return ERR_OBJDEL;
+               ret = ERR_OBJDEL;
 
-       return SUCCESS;
+       put_psos_task(task);
+out:
+       CANCEL_RESTORE(svc);
+
+       return ret;
 }
 
 u_long t_resume(u_long tid)
@@ -418,52 +432,65 @@ u_long t_resume(u_long tid)
        struct service svc;
        int ret;
 
+       CANCEL_DEFER(svc);
+
        task = get_psos_task(tid, &ret);
        if (task == NULL)
-               return ret;
+               goto out;
 
-       CANCEL_DEFER(svc);
        ret = threadobj_resume(&task->thobj);
-       CANCEL_RESTORE(svc);
-       put_psos_task(task);
-
        if (ret)
-               return ERR_OBJDEL;
+               ret = ERR_OBJDEL;
 
-       return SUCCESS;
+       put_psos_task(task);
+out:
+       CANCEL_RESTORE(svc);
+
+       return ret;
 }
 
 u_long t_setpri(u_long tid, u_long newprio, u_long *oldprio_r)
 {
+       int policy, ret = SUCCESS, cprio = 1;
        struct sched_param_ex param_ex;
-       int policy, ret, cprio = 1;
        struct psos_task *task;
+       struct service svc;
+
+       CANCEL_DEFER(svc);
 
        task = get_psos_task_or_self(tid, &ret);
        if (task == NULL)
-               return ret;
+               goto out;
 
        *oldprio_r = psos_task_get_priority(task);
 
-       if (newprio == 0) { /* Only inquires for the task priority. */
-               put_psos_task(task);
-               return SUCCESS;
-       }
+       if (newprio == 0) /* Only inquires for the task priority. */
+               goto done;
 
        ret = check_task_priority(newprio, &cprio);
        if (ret) {
-               put_psos_task(task);
-               return ERR_SETPRI;
+               ret = ERR_SETPRI;
+               goto done;
        }
 
        policy = cprio ? SCHED_FIFO : SCHED_OTHER;
        param_ex.sched_priority = cprio;
        ret = threadobj_set_schedparam(&task->thobj, policy, &param_ex);
+       switch (ret) {
+       case -EIDRM:
+               ret = SUCCESS;
+               goto out;
+       default:
+               ret = ERR_OBJDEL;
+       case 0: /* == SUCCESS */
+               break;
+       }
+done:
        put_psos_task(task);
-       if (ret)
-               return ERR_OBJDEL;
+out:
+       CANCEL_RESTORE(svc);
 
-       return SUCCESS;
+       return ret;
 }
 
 u_long t_delete(u_long tid)
@@ -472,17 +499,19 @@ u_long t_delete(u_long tid)
        struct service svc;
        int ret;
 
+       CANCEL_DEFER(svc);
+
        task = get_psos_task_or_self(tid, &ret);
        if (task == NULL)
-               return ret;
+               goto out;
 
-       CANCEL_DEFER(svc);
        ret = threadobj_cancel(&task->thobj);
-       CANCEL_RESTORE(svc);
        if (ret)
-               return ERR_OBJDEL;
+               ret = ERR_OBJDEL;
+out:
+       CANCEL_RESTORE(svc);
 
-       return SUCCESS;
+       return ret;
 }
 
 u_long t_ident(const char *name, u_long node, u_long *tid_r)
@@ -530,48 +559,61 @@ out:
 u_long t_getreg(u_long tid, u_long regnum, u_long *regvalue_r)
 {
        struct psos_task *task;
-       int ret;
+       struct service svc;
+       int ret = SUCCESS;
 
        if (regnum >= PSOSTASK_NR_REGS)
                return ERR_REGNUM;
 
+       CANCEL_DEFER(svc);
+
        task = get_psos_task_or_self(tid, &ret);
        if (task == NULL)
-               return ret;
+               goto out;
 
        *regvalue_r = task->notepad[regnum];
        put_psos_task(task);
+out:
+       CANCEL_RESTORE(svc);
 
-       return SUCCESS;
+       return ret;
 }
 
 u_long t_setreg(u_long tid, u_long regnum, u_long regvalue)
 {
        struct psos_task *task;
-       int ret;
+       struct service svc;
+       int ret = SUCCESS;
 
        if (regnum >= PSOSTASK_NR_REGS)
                return ERR_REGNUM;
 
+       CANCEL_DEFER(svc);
+
        task = get_psos_task_or_self(tid, &ret);
        if (task == NULL)
-               return ret;
+               goto out;
 
        task->notepad[regnum] = regvalue;
        put_psos_task(task);
+out:
+       CANCEL_RESTORE(svc);
 
-       return SUCCESS;
+       return ret;
 }
 
 u_long t_mode(u_long mask, u_long newmask, u_long *oldmode_r)
 {
        struct sched_param_ex param_ex;
+       int policy, ret = SUCCESS;
        struct psos_task *task;
-       int policy, ret;
+       struct service svc;
+
+       CANCEL_DEFER(svc);
 
        task = get_psos_task_or_self(0, &ret);
        if (task == NULL)
-               return ret;
+               goto out;
 
        *oldmode_r = task->mode;
 
@@ -597,11 +639,14 @@ u_long t_mode(u_long mask, u_long newmask, u_long 
*oldmode_r)
        } else
                policy = param_ex.sched_priority ? SCHED_FIFO : SCHED_OTHER;
 
+       /* Working on self, so -EIDRM can't happen. */
        threadobj_set_schedparam(&task->thobj, policy, &param_ex);
 done:
        put_psos_task(task);
+out:
+       CANCEL_RESTORE(svc);
 
-       return SUCCESS;
+       return ret;
 }
 
 static int collect_events(struct psos_task *task,
diff --git a/lib/psos/testsuite/tm-3.c b/lib/psos/testsuite/tm-3.c
index 9be419e..260a131 100644
--- a/lib/psos/testsuite/tm-3.c
+++ b/lib/psos/testsuite/tm-3.c
@@ -45,6 +45,7 @@ static void task(u_long a0, u_long a1, u_long a2, u_long a3)
        traceobj_assert(&trobj, ret == ERR_TIMEOUT);
        traceobj_mark(&trobj, 6);
 
+       /* Valgrind will bark at this one, this is expected. */
        ret = tm_cancel(timer_id);
        traceobj_assert(&trobj, ret == ERR_BADTMID);
 
diff --git a/lib/trank/native.c b/lib/trank/native.c
index 1f44368..a13eb38 100644
--- a/lib/trank/native.c
+++ b/lib/trank/native.c
@@ -173,6 +173,7 @@ int rt_alarm_wait(RT_ALARM *alarm)
        prio = threadobj_get_priority(current);
        if (prio != threadobj_irq_prio) {
                param_ex.sched_priority = threadobj_irq_prio;
+               /* Working on self, so -EIDRM can't happen. */
                threadobj_set_schedparam(current, SCHED_FIFO, &param_ex);
        }
        threadobj_unlock(current);
diff --git a/lib/vxworks/errnoLib.c b/lib/vxworks/errnoLib.c
index 2ef5dd5..cfb73f2 100644
--- a/lib/vxworks/errnoLib.c
+++ b/lib/vxworks/errnoLib.c
@@ -91,28 +91,43 @@ void printErrno(int status)
 STATUS errnoOfTaskSet(TASK_ID task_id, int status)
 {
        struct wind_task *task;
+       struct service svc;
+       STATUS ret = OK;
+
+       CANCEL_DEFER(svc);
 
        task = get_wind_task_or_self(task_id);
-       if (task == NULL)
-               return ERROR;
+       if (task == NULL) {
+               ret = ERROR;
+               goto out;
+       }
 
        *task->thobj.errno_pointer = status;
        put_wind_task(task);
+out:
+       CANCEL_RESTORE(svc);
 
-       return OK;
+       return ret;
 }
 
 STATUS errnoOfTaskGet(TASK_ID task_id)
 {
        struct wind_task *task;
-       STATUS status;
+       struct service svc;
+       STATUS status = OK;
+
+       CANCEL_DEFER(svc);
 
        task = get_wind_task_or_self(task_id);
-       if (task == NULL)
-               return ERROR;
+       if (task == NULL) {
+               status = ERROR;
+               goto out;
+       }
 
        status = *task->thobj.errno_pointer;
        put_wind_task(task);
+out:
+       CANCEL_RESTORE(svc);
 
        return status;
 }
diff --git a/lib/vxworks/kernLib.c b/lib/vxworks/kernLib.c
index 80580fb..4f3bd82 100644
--- a/lib/vxworks/kernLib.c
+++ b/lib/vxworks/kernLib.c
@@ -34,7 +34,7 @@ static int switch_slicing(struct threadobj *thobj, struct 
timespec *quantum)
        } else
                policy = param_ex.sched_priority ? SCHED_FIFO : SCHED_OTHER;
 
-       return __bt(threadobj_set_schedparam(thobj, policy, &param_ex));
+       return threadobj_set_schedparam(thobj, policy, &param_ex);
 }
 
 STATUS kernelTimeSlice(int ticks)
diff --git a/lib/vxworks/taskInfo.c b/lib/vxworks/taskInfo.c
index 8e33644..7989468 100644
--- a/lib/vxworks/taskInfo.c
+++ b/lib/vxworks/taskInfo.c
@@ -26,18 +26,24 @@
 const char *taskName(TASK_ID task_id)
 {
        struct wind_task *task;
+       struct service svc;
        const char *name;
 
+       CANCEL_DEFER(svc);
+
        task = get_wind_task_or_self(task_id);
-       if (task == NULL)
-               return NULL;
+       if (task == NULL) {
+               name = NULL;
+               goto out;
+       }
 
        name = task->name;
        put_wind_task(task);
-
+out:
+       CANCEL_RESTORE(svc);
        /*
-        * This is unsafe, but this service is terminally flawed by
-        * design anyway.
+        * This is racy and unsafe, but this service is terminally
+        * flawed by design anyway.
         */
        return name;
 }
@@ -72,14 +78,19 @@ TASK_ID taskNameToId(const char *name)
 BOOL taskIsReady(TASK_ID task_id)
 {
        struct wind_task *task;
-       int status;
+       struct service svc;
+       int status = 0;
+
+       CANCEL_DEFER(svc);
 
        task = get_wind_task(task_id);
        if (task == NULL)
-               return 0;
+               goto out;
 
        status = get_task_status(task);
        put_wind_task(task);
+out:
+       CANCEL_RESTORE(svc);
 
        return status == WIND_READY;
 }
@@ -87,15 +98,19 @@ BOOL taskIsReady(TASK_ID task_id)
 BOOL taskIsSuspended(TASK_ID task_id)
 {
        struct wind_task *task;
-       int status;
+       struct service svc;
+       int status = 0;
+
+       CANCEL_DEFER(svc);
 
        task = get_wind_task(task_id);
        if (task == NULL)
-               return 0;
+               goto out;
 
        status = threadobj_get_status(&task->thobj);
-
        put_wind_task(task);
+out:
+       CANCEL_RESTORE(svc);
 
        return (status & __THREAD_S_SUSPENDED) != 0;
 }
@@ -106,13 +121,18 @@ STATUS taskGetInfo(TASK_ID task_id, TASK_DESC *desc)
        struct wind_task *task;
        struct WIND_TCB *tcb;
        pthread_attr_t attr;
+       STATUS status = OK;
+       struct service svc;
        size_t stacksize;
        void *stackbase;
 
+       CANCEL_DEFER(svc);
+
        task = get_wind_task(task_id);
        if (task == NULL) {
                errno = S_objLib_OBJ_ID_ERROR;
-               return ERROR;
+               status = ERROR;
+               goto out;
        }
 
        tcb = task->tcb;
@@ -147,6 +167,8 @@ STATUS taskGetInfo(TASK_ID task_id, TASK_DESC *desc)
                        /* Stack grows downward. */
                        desc->td_pStackEnd = (caddr_t)stackbase - stacksize;
        }
+out:
+       CANCEL_RESTORE(svc);
 
-       return OK;
+       return status;
 }
diff --git a/lib/vxworks/taskLib.c b/lib/vxworks/taskLib.c
index 34faedc..946e58d 100644
--- a/lib/vxworks/taskLib.c
+++ b/lib/vxworks/taskLib.c
@@ -268,16 +268,19 @@ static void *task_trampoline(void *arg)
                threadobj_unlock(&task->thobj);
        }
 
+       threadobj_notify_entry();
+
        CANCEL_RESTORE(svc);
 
-       threadobj_notify_entry();
        args->entry(args->arg0, args->arg1, args->arg2, args->arg3,
                    args->arg4, args->arg5, args->arg6, args->arg7,
                    args->arg8, args->arg9);
 
+       CANCEL_DEFER(svc);
        threadobj_lock(&task->thobj);
        threadobj_set_magic(&task->thobj, ~task_magic);
        threadobj_unlock(&task->thobj);
+       CANCEL_RESTORE(svc);
 
        return NULL;
 }
@@ -461,18 +464,32 @@ out:
 STATUS taskActivate(TASK_ID tid)
 {
        struct wind_task *task;
+       struct service svc;
        int ret;
 
+       CANCEL_DEFER(svc);
+
        task = get_wind_task(tid);
-       if (task == NULL)
-               return ERROR;
+       if (task == NULL) {
+               ret = ERROR;
+               goto out;
+       }
 
        task->tcb->status &= ~WIND_SUSPEND;
        ret = threadobj_start(&task->thobj);
-       if (ret != -EIDRM)
+       switch (ret) {
+       case -EIDRM:
+               ret = OK;
+               break;
+       default:
+               ret = ERROR;
+       case 0: /* == OK */
                put_wind_task(task);
+       }
+out:
+       CANCEL_RESTORE(svc);
 
-       return OK;
+       return ret;
 }
 
 TASK_ID taskSpawn(const char *name,
@@ -531,6 +548,7 @@ TASK_ID taskSpawn(const char *name,
 static STATUS __taskDelete(TASK_ID tid, int force)
 {
        struct wind_task *task;
+       struct service svc;
        int ret;
 
        if (threadobj_irq_p()) {
@@ -545,6 +563,8 @@ static STATUS __taskDelete(TASK_ID tid, int force)
                return ERROR;
        }
 
+       CANCEL_DEFER(svc);
+
        /*
         * We always attempt to grab the thread safe lock first, then
         * make sure nobody (including the target task itself) will be
@@ -557,10 +577,6 @@ static STATUS __taskDelete(TASK_ID tid, int force)
         * object lock afterwards, therefore, _never_ call
         * __taskDelete() directly or indirectly while holding the
         * thread object lock. You have been warned.
-        *
-        * We traverse no cancellation point below until
-        * threadobj_cancel() is invoked, so we don't need any
-        * CANCEL_DEFER() section.
         */
        if (force)      /* Best effort only. */
                force = __RT(pthread_mutex_trylock(&task->safelock));
@@ -575,6 +591,8 @@ static STATUS __taskDelete(TASK_ID tid, int force)
        if (ret == 0)
                ret = threadobj_cancel(&task->thobj);
 
+       CANCEL_RESTORE(svc);
+
        if (ret)
                goto objid_error;
 
@@ -628,22 +646,24 @@ STATUS taskSuspend(TASK_ID tid)
        struct service svc;
        int ret;
 
+       CANCEL_DEFER(svc);
+
        task = get_wind_task(tid);
        if (task == NULL)
                goto objid_error;
 
-       CANCEL_DEFER(svc);
        ret = threadobj_suspend(&task->thobj);
-       CANCEL_RESTORE(svc);
        put_wind_task(task);
 
        if (ret) {
        objid_error:
                errno = S_objLib_OBJ_ID_ERROR;
-               return ERROR;
+               ret = ERROR;
        }
 
-       return OK;
+       CANCEL_RESTORE(svc);
+
+       return ret;
 }
 
 STATUS taskResume(TASK_ID tid)
@@ -652,22 +672,24 @@ STATUS taskResume(TASK_ID tid)
        struct service svc;
        int ret;
 
+       CANCEL_DEFER(svc);
+
        task = get_wind_task(tid);
        if (task == NULL)
                goto objid_error;
 
-       CANCEL_DEFER(svc);
        ret = threadobj_resume(&task->thobj);
-       CANCEL_RESTORE(svc);
        put_wind_task(task);
 
        if (ret) {
        objid_error:
                errno = S_objLib_OBJ_ID_ERROR;
-               return ERROR;
+               ret = ERROR;
        }
 
-       return OK;
+       CANCEL_RESTORE(svc);
+
+       return ret;
 }
 
 STATUS taskSafe(void)
@@ -743,6 +765,8 @@ STATUS taskPrioritySet(TASK_ID tid, int prio)
        int ret, policy, cprio;
        struct service svc;
 
+       CANCEL_DEFER(svc);
+
        task = get_wind_task(tid);
        if (task == NULL)
                goto objid_error;
@@ -751,24 +775,25 @@ STATUS taskPrioritySet(TASK_ID tid, int prio)
        if (ret) {
                put_wind_task(task);
                errno = ret;
-               return ERROR;
+               ret = ERROR;
+               goto out;
        }
 
-       CANCEL_DEFER(svc);
        policy = cprio ? SCHED_FIFO : SCHED_OTHER;
        param_ex.sched_priority = cprio;
        ret = threadobj_set_schedparam(&task->thobj, policy, &param_ex);
-       CANCEL_RESTORE(svc);
-
-       put_wind_task(task);
+       if (ret != -EIDRM)
+               put_wind_task(task);
 
        if (ret) {
        objid_error:
                errno = S_objLib_OBJ_ID_ERROR;
-               return ERROR;
+               ret = ERROR;
        }
+out:
+       CANCEL_RESTORE(svc);
 
-       return OK;
+       return ret;
 }
 
 int wind_task_get_priority(struct wind_task *task)
@@ -781,17 +806,24 @@ int wind_task_get_priority(struct wind_task *task)
 STATUS taskPriorityGet(TASK_ID tid, int *priop)
 {
        struct wind_task *task;
+       struct service svc;
+       int ret = OK;
+
+       CANCEL_DEFER(svc);
 
        task = get_wind_task(tid);
        if (task == NULL) {
                errno = S_objLib_OBJ_ID_ERROR;
-               return ERROR;
+               ret = ERROR;
+               goto out;
        }
 
        *priop = wind_task_get_priority(task);
        put_wind_task(task);
+out:
+       CANCEL_RESTORE(svc);
 
-       return OK;
+       return ret;
 }
 
 STATUS taskLock(void)
diff --git a/lib/vxworks/taskLib.h b/lib/vxworks/taskLib.h
index 1d75e12..dc01025 100644
--- a/lib/vxworks/taskLib.h
+++ b/lib/vxworks/taskLib.h
@@ -59,7 +59,9 @@ struct wind_task {
                if (!pvlist_empty(&wind_task_list))                     \
                        pvlist_for_each_entry(__task, &wind_task_list, next) { \
                                threadobj_lock(&(__task)->thobj);       \
-                               __ret = __action;                       \
+                               __ret = (__action);                     \
+                               if (__ret == -EIDRM)                    \
+                                       continue;                       \
                                threadobj_unlock(&(__task)->thobj);     \
                                if (__ret)                              \
                                        goto out;                       \


_______________________________________________
Xenomai-git mailing list
Xenomai-git@xenomai.org
http://www.xenomai.org/mailman/listinfo/xenomai-git

Reply via email to