Module: xenomai-forge
Branch: master
Commit: 412a1286b42f368417027302d4017c89f138d922
URL:
http://git.xenomai.org/?p=xenomai-forge.git;a=commit;h=412a1286b42f368417027302d4017c89f138d922
Author: Philippe Gerum <r...@xenomai.org>
Date: Sun May 12 11:37:41 2013 +0200
copperplate/threadobj: defer TCB release until wait on barrier is over
(valgrind)
This clears a valgrind warning which may occur in the following
situation:
Thread A Thread B
threadobj_lock(B)
threadobj_start(B)
/* wait for B active */
threadobj_prologue(B)
/* set B active */
pthread_exit()
threadobj_free(B)
pthread_cond_wait(&B->barrier)
threadobj_unlock(B)
In such a case, thread A would tread on free memory while waking up on
the start barrier.
The thread finalizer now refrains from releasing the current TCB if
threadobj_start() is still ongoing, waiting for the latter to reclaim
the resources safely.
---
include/copperplate/threadobj.h | 13 +++--
lib/alchemy/task.c | 13 ++++-
lib/copperplate/threadobj.c | 115 ++++++++++++++++++++++++++-------------
lib/psos/task.c | 5 +-
lib/vxworks/taskLib.c | 6 ++-
5 files changed, 100 insertions(+), 52 deletions(-)
diff --git a/include/copperplate/threadobj.h b/include/copperplate/threadobj.h
index 8e2c235..01a4010 100644
--- a/include/copperplate/threadobj.h
+++ b/include/copperplate/threadobj.h
@@ -118,16 +118,17 @@ void threadobj_save_timeout(struct threadobj_corespec
*corespec,
#define __THREAD_S_LOCKED (1 << 5) /* threadobj_lock() granted
(debug only). */
#define __THREAD_S_ACTIVE (1 << 6) /* Running user code. */
#define __THREAD_S_SUSPENDED (1 << 7) /* Suspended via
threadobj_suspend(). */
-#define __THREAD_S_DEBUG (1 << 15) /* Debug mode enabled. */
+#define __THREAD_S_SAFE (1 << 8) /* TCB release
deferred. */
+#define __THREAD_S_DEBUG (1 << 31) /* Debug mode enabled. */
/*
* threadobj->run_state, locklessly updated by "current", merged
* with ->status bits by threadobj_get_status().
*/
#define __THREAD_S_RUNNING 0
-#define __THREAD_S_DORMANT (1 << 8)
-#define __THREAD_S_WAIT (1 << 9)
-#define __THREAD_S_TIMEDWAIT (1 << 10)
-#define __THREAD_S_DELAYED (1 << 11)
+#define __THREAD_S_DORMANT (1 << 16)
+#define __THREAD_S_WAIT (1 << 17)
+#define __THREAD_S_TIMEDWAIT (1 << 18)
+#define __THREAD_S_DELAYED (1 << 19)
/* threadobj mode bits */
#define __THREAD_M_LOCK (1 << 0) /* Toggle scheduler lock. */
@@ -275,7 +276,7 @@ static inline void threadobj_free(struct threadobj *thobj)
void threadobj_init(struct threadobj *thobj,
struct threadobj_init_data *idata);
-void threadobj_start(struct threadobj *thobj);
+int threadobj_start(struct threadobj *thobj);
void threadobj_shadow(struct threadobj *thobj);
diff --git a/lib/alchemy/task.c b/lib/alchemy/task.c
index 00cf278..da09d51 100644
--- a/lib/alchemy/task.c
+++ b/lib/alchemy/task.c
@@ -544,7 +544,7 @@ int rt_task_start(RT_TASK *task,
{
struct alchemy_task *tcb;
struct service svc;
- int ret = 0;
+ int ret;
COPPERPLATE_PROTECT(svc);
@@ -554,8 +554,15 @@ int rt_task_start(RT_TASK *task,
tcb->entry = entry;
tcb->arg = arg;
- threadobj_start(&tcb->thobj);
- put_alchemy_task(tcb);
+ ret = threadobj_start(&tcb->thobj);
+ if (ret == -EIDRM)
+ /*
+ * The started thread has run then exited, tcb->thobj
+ * is stale: don't touch it anymore.
+ */
+ ret = 0;
+ else
+ put_alchemy_task(tcb);
out:
COPPERPLATE_PROTECT(svc);
diff --git a/lib/copperplate/threadobj.c b/lib/copperplate/threadobj.c
index 4bbf06e..0ec95d7 100644
--- a/lib/copperplate/threadobj.c
+++ b/lib/copperplate/threadobj.c
@@ -797,6 +797,19 @@ void threadobj_init(struct threadobj *thobj,
threadobj_init_corespec(thobj);
}
+static void destroy_thread(struct threadobj *thobj)
+{
+ __RT(pthread_cond_destroy(&thobj->barrier));
+ __RT(pthread_mutex_destroy(&thobj->lock));
+ threadobj_cleanup_corespec(thobj);
+}
+
+void threadobj_destroy(struct threadobj *thobj) /* thobj->lock free */
+{
+ assert((thobj->status & (__THREAD_S_STARTED|__THREAD_S_ACTIVE)) == 0);
+ destroy_thread(thobj);
+}
+
/*
* NOTE: to spare us the need for passing the equivalent of a
* syncstate argument to each thread locking operation, we hold the
@@ -815,7 +828,7 @@ void threadobj_init(struct threadobj *thobj,
* call these services, and these have no threadobj descriptor.
*/
-static int start_sync(struct threadobj *thobj, int mask)
+static int wait_on_barrier(struct threadobj *thobj, int mask)
{
int oldstate, status;
@@ -833,27 +846,51 @@ static int start_sync(struct threadobj *thobj, int mask)
return status;
}
-void threadobj_start(struct threadobj *thobj) /* thobj->lock held. */
+int threadobj_start(struct threadobj *thobj) /* thobj->lock held. */
{
struct threadobj *current = threadobj_current();
+ int ret = 0, oldstate;
__threadobj_check_locked(thobj);
if (thobj->status & __THREAD_S_STARTED)
- return;
+ return 0;
- thobj->status |= __THREAD_S_STARTED;
+ thobj->status |= __THREAD_S_STARTED|__THREAD_S_SAFE;
__RT(pthread_cond_signal(&thobj->barrier));
if (current && thobj->priority <= current->priority)
- return;
+ return 0;
/*
* Caller needs synchronization with the thread being started,
* which has higher priority. We shall wait until that thread
* enters the user code, or aborts prior to reaching that
* point, whichever comes first.
+ *
+ * We must not exit until the synchronization has fully taken
+ * place, disable cancellability until then.
+ */
+ pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldstate);
+
+ wait_on_barrier(thobj, __THREAD_S_ACTIVE);
+ thobj->status &= ~__THREAD_S_SAFE;
+
+ /*
+ * If the started thread has exited before we woke up from the
+ * barrier, its TCB was not reclaimed, to prevent us from
+ * treading on stale memory. Reclaim it now, and tell the
+ * caller to forget about it as well.
*/
- start_sync(thobj, __THREAD_S_ACTIVE);
+ if (thobj->run_state == __THREAD_S_DORMANT) {
+ threadobj_unlock(thobj);
+ destroy_thread(thobj);
+ threadobj_free(thobj);
+ ret = -EIDRM;
+ }
+
+ pthread_setcancelstate(oldstate, NULL);
+
+ return ret;
}
void threadobj_shadow(struct threadobj *thobj)
@@ -870,7 +907,7 @@ void threadobj_wait_start(void) /* current->lock free. */
int status;
threadobj_lock(current);
- status = start_sync(current, __THREAD_S_STARTED|__THREAD_S_ABORTED);
+ status = wait_on_barrier(current,
__THREAD_S_STARTED|__THREAD_S_ABORTED);
threadobj_unlock(current);
/*
@@ -950,6 +987,23 @@ int threadobj_prologue(struct threadobj *thobj, const char
*name)
return 0;
}
+int threadobj_sleep(struct timespec *ts)
+{
+ struct threadobj *current = threadobj_current();
+ int ret;
+
+ /*
+ * clock_nanosleep() returns -EINTR upon threadobj_unblock()
+ * with both Cobalt and Mercury cores.
+ */
+ current->run_state = __THREAD_S_DELAYED;
+ threadobj_save_timeout(¤t->core, ts);
+ ret = -__RT(clock_nanosleep(CLOCK_COPPERPLATE, TIMER_ABSTIME, ts,
NULL));
+ current->run_state = __THREAD_S_RUNNING;
+
+ return ret;
+}
+
static void cancel_sync(struct threadobj *thobj) /* thobj->lock held */
{
pthread_t tid = thobj->tid;
@@ -1019,23 +1073,6 @@ static void cancel_sync(struct threadobj *thobj) /*
thobj->lock held */
}
}
-int threadobj_sleep(struct timespec *ts)
-{
- struct threadobj *current = threadobj_current();
- int ret;
-
- /*
- * clock_nanosleep() returns -EINTR upon threadobj_unblock()
- * with both Cobalt and Mercury cores.
- */
- current->run_state = __THREAD_S_DELAYED;
- threadobj_save_timeout(¤t->core, ts);
- ret = -__RT(clock_nanosleep(CLOCK_COPPERPLATE, TIMER_ABSTIME, ts,
NULL));
- current->run_state = __THREAD_S_RUNNING;
-
- return ret;
-}
-
/* thobj->lock held on entry, released on return */
int threadobj_cancel(struct threadobj *thobj)
{
@@ -1058,13 +1095,6 @@ int threadobj_cancel(struct threadobj *thobj)
return 0;
}
-static void destroy_thread(struct threadobj *thobj)
-{
- __RT(pthread_cond_destroy(&thobj->barrier));
- __RT(pthread_mutex_destroy(&thobj->lock));
- threadobj_cleanup_corespec(thobj);
-}
-
static void finalize_thread(void *p) /* thobj->lock free */
{
struct threadobj *thobj = p;
@@ -1094,15 +1124,22 @@ static void finalize_thread(void *p) /* thobj->lock
free */
/* Release the killer from threadobj_cancel(). */
__STD(sem_post)(thobj->cancel_sem);
- destroy_thread(thobj);
- threadobj_free(thobj);
- threadobj_set_current(NULL);
-}
+ thobj->run_state = __THREAD_S_DORMANT;
-void threadobj_destroy(struct threadobj *thobj) /* thobj->lock free */
-{
- assert((thobj->status & (__THREAD_S_STARTED|__THREAD_S_ACTIVE)) == 0);
- destroy_thread(thobj);
+ /*
+ * Do not reclaim the TCB core resources if another thread is
+ * waiting for us to start, pending on
+ * wait_on_barrier(). Instead, hand it over to this thread.
+ */
+ threadobj_lock(thobj);
+ if ((thobj->status & __THREAD_S_SAFE) == 0) {
+ threadobj_unlock(thobj);
+ destroy_thread(thobj);
+ threadobj_free(thobj);
+ } else
+ threadobj_unlock(thobj);
+
+ threadobj_set_current(NULL);
}
int threadobj_unblock(struct threadobj *thobj) /* thobj->lock held */
diff --git a/lib/psos/task.c b/lib/psos/task.c
index 27accef..0a0ef40 100644
--- a/lib/psos/task.c
+++ b/lib/psos/task.c
@@ -351,8 +351,9 @@ u_long t_start(u_long tid,
task->args.arg3 = 0;
}
task->mode = mode;
- threadobj_start(&task->thobj);
- put_psos_task(task);
+ ret = threadobj_start(&task->thobj);
+ if (ret != -EIDRM)
+ put_psos_task(task);
return SUCCESS;
}
diff --git a/lib/vxworks/taskLib.c b/lib/vxworks/taskLib.c
index ffd9ce2..6096694 100644
--- a/lib/vxworks/taskLib.c
+++ b/lib/vxworks/taskLib.c
@@ -436,14 +436,16 @@ out:
STATUS taskActivate(TASK_ID tid)
{
struct wind_task *task;
+ int ret;
task = get_wind_task(tid);
if (task == NULL)
return ERROR;
task->tcb->status &= ~WIND_SUSPEND;
- threadobj_start(&task->thobj);
- put_wind_task(task);
+ ret = threadobj_start(&task->thobj);
+ if (ret != -EIDRM)
+ put_wind_task(task);
return OK;
}
_______________________________________________
Xenomai-git mailing list
Xenomai-git@xenomai.org
http://www.xenomai.org/mailman/listinfo/xenomai-git
