[Xenomai-core] [PATCH] fix pthread cancellation in native skin

[Xenomai-core] [PATCH] fix pthread cancellation in native skin

[Jan Kiszka]

[-- Attachment #1.1: Type: text/plain, Size: 753 bytes --]

Hi,

Gilles' work on cancellation for the posix skin reminded me of this
issue I once discovered in the native skin:

https://mail.gna.org/public/xenomai-core/2005-12/msg00014.html

I found out that this can easily be fixed by switching the pthread of a
native task to PTHREAD_CANCEL_ASYNCHRONOUS. See attached patch.


At this chance I discovered that calling rt_task_delete for a task that
was created and started with T_SUSP mode but was not yet resumed, locks
up the system. More precisely: it raises a fatal warning when
XENO_OPT_DEBUG is on. Might be the case that it just works on system
without this switched on. Either this is a real bug, or the warning
needs to be fixed. (Deleting a task after rt_task_suspend works.)

Jan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: native-canceltype.patch --]
[-- Type: text/x-patch; name="native-canceltype.patch", Size: 813 bytes --]

Index: src/skins/native/task.c
===================================================================
--- src/skins/native/task.c	(Revision 481)
+++ src/skins/native/task.c	(Arbeitskopie)
@@ -59,6 +59,9 @@
     param.sched_priority = sched_get_priority_max(SCHED_FIFO);
     pthread_setschedparam(pthread_self(),SCHED_FIFO,&param);
 
+    /* rt_task_delete requires asynchronous cancellation */
+    pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
+
     signal(SIGCHLD,&rt_task_sigharden);
 
     bulk.a1 = (u_long)iargs->task;
@@ -160,6 +163,9 @@
 {
     struct rt_arg_bulk bulk;
 
+    /* rt_task_delete requires asynchronous cancellation */
+    pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, NULL);
+
     signal(SIGCHLD,&rt_task_sigharden);
 
     bulk.a1 = (u_long)task;

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

Re: [Xenomai-core] [PATCH] fix pthread cancellation in native skin

[Gilles Chanteperdrix]

Jan Kiszka wrote:
 > Hi,
 > 
 > Gilles' work on cancellation for the posix skin reminded me of this
 > issue I once discovered in the native skin:
 > 
 > https://mail.gna.org/public/xenomai-core/2005-12/msg00014.html
 > 
 > I found out that this can easily be fixed by switching the pthread of a
 > native task to PTHREAD_CANCEL_ASYNCHRONOUS. See attached patch.

Applied, thanks.

-- 


					    Gilles Chanteperdrix.

Re: [Xenomai-core] [PATCH] fix pthread cancellation in native skin

[Gilles Chanteperdrix]

Jan Kiszka wrote:
 > Hi,
 > 
 > Gilles' work on cancellation for the posix skin reminded me of this
 > issue I once discovered in the native skin:
 > 
 > https://mail.gna.org/public/xenomai-core/2005-12/msg00014.html
 > 
 > I found out that this can easily be fixed by switching the pthread of a
 > native task to PTHREAD_CANCEL_ASYNCHRONOUS. See attached patch.
 > 
 > 
 > At this chance I discovered that calling rt_task_delete for a task that
 > was created and started with T_SUSP mode but was not yet resumed, locks
 > up the system. More precisely: it raises a fatal warning when
 > XENO_OPT_DEBUG is on. Might be the case that it just works on system
 > without this switched on. Either this is a real bug, or the warning
 > needs to be fixed. (Deleting a task after rt_task_suspend works.)

Actually, the fatal warning happens when starting with rt_task_start the
task which was created with the T_SUSP bit. The task needs to wake-up in
xnshadow_wait_barrier until it gets really suspended in primary mode by
the final xnshadow_harden. This situation triggers the fatal, because
the thread has the nucleus XNSUSP bit and is running in secondary mode.

This is not the only situation where a thread with a nucleus suspension
bit need to run shortly in secondary mode: it also occurs when
suspending with xnpod_suspend_thread() a thread running in secondary
mode; the thread receives the SIGCHLD signal and need to execute shortly
with the suspension bit set in order to cause a migration to primary
mode.

So, the only case when we are sure that a user-space thread can not be
scheduled by Linux seems to be when this thread does not have the
XNRELAX bit.

-- 


					    Gilles Chanteperdrix.

Re: [Xenomai-core] [PATCH] fix pthread cancellation in native skin

[Gilles Chanteperdrix]

[-- Attachment #1: message body text --]
[-- Type: text/plain, Size: 768 bytes --]

Gilles Chanteperdrix wrote:
 > This is not the only situation where a thread with a nucleus suspension
 > bit need to run shortly in secondary mode: it also occurs when
 > suspending with xnpod_suspend_thread() a thread running in secondary
 > mode; the thread receives the SIGCHLD signal and need to execute shortly
 > with the suspension bit set in order to cause a migration to primary
 > mode.
 > 
 > So, the only case when we are sure that a user-space thread can not be
 > scheduled by Linux seems to be when this thread does not have the
 > XNRELAX bit.

From all the bits in XNTHREAD_BLOCK_BITS, only when the XNPEND bit is
set, a thread can not be running in secondary mode. Hence the proposed
patch.

-- 


					    Gilles Chanteperdrix.

[-- Attachment #2: fatal.diff --]
[-- Type: text/plain, Size: 1179 bytes --]

Index: ksrc/nucleus/shadow.c
===================================================================
--- ksrc/nucleus/shadow.c	(revision 507)
+++ ksrc/nucleus/shadow.c	(working copy)
@@ -1543,14 +1543,25 @@
 
 #ifdef CONFIG_XENO_OPT_DEBUG
 	{
-	xnflags_t status = threadin->status & ~XNRELAX;
+	xnflags_t status = threadin->status;
 	int sigpending = signal_pending(next);
 
-        if (!(next->ptrace & PT_PTRACED) &&
+        if (!testbits(status, XNRELAX))
+            {
+            show_stack(xnthread_user_task(threadin),NULL);
+            xnpod_fatal("Hardened thread %s[%d] running in Linux domain?! (status=0x%lx, sig=%d, prev=%s[%d])",
+			threadin->name,
+			next->pid,
+			status,
+			sigpending,
+			prev->comm,
+			prev->pid);
+	    }
+        else if (!(next->ptrace & PT_PTRACED) &&
 	    /* Allow ptraced threads to run shortly in order to
 	       properly recover from a stopped state. */
 	    testbits(status,XNSTARTED) &&
-	    testbits(status,XNTHREAD_BLOCK_BITS))
+	    testbits(status,XNPEND))
 	    {
 	    show_stack(xnthread_user_task(threadin),NULL);
             xnpod_fatal("blocked thread %s[%d] rescheduled?! (status=0x%lx, sig=%d, prev=%s[%d])",

Re: [Xenomai-core] [PATCH] fix pthread cancellation in native skin

[Philippe Gerum]

Gilles Chanteperdrix wrote:
> Gilles Chanteperdrix wrote:
>  > This is not the only situation where a thread with a nucleus suspension
>  > bit need to run shortly in secondary mode: it also occurs when
>  > suspending with xnpod_suspend_thread() a thread running in secondary
>  > mode; the thread receives the SIGCHLD signal and need to execute shortly
>  > with the suspension bit set in order to cause a migration to primary
>  > mode.
>  > 
>  > So, the only case when we are sure that a user-space thread can not be
>  > scheduled by Linux seems to be when this thread does not have the
>  > XNRELAX bit.
> 
> From all the bits in XNTHREAD_BLOCK_BITS, only when the XNPEND bit is
> set, a thread can not be running in secondary mode. Hence the proposed
> patch.
> 

Almost ok, but XNDELAY might also be set alone, indicating a purely timed wait 
state (i.e. without sync object to pend on, so XNPEND is off).

> 
> 
> ------------------------------------------------------------------------
> 
> Index: ksrc/nucleus/shadow.c
> ===================================================================
> --- ksrc/nucleus/shadow.c	(revision 507)
> +++ ksrc/nucleus/shadow.c	(working copy)
> @@ -1543,14 +1543,25 @@
>  
>  #ifdef CONFIG_XENO_OPT_DEBUG
>  	{
> -	xnflags_t status = threadin->status & ~XNRELAX;
> +	xnflags_t status = threadin->status;
>  	int sigpending = signal_pending(next);
>  
> -        if (!(next->ptrace & PT_PTRACED) &&
> +        if (!testbits(status, XNRELAX))
> +            {
> +            show_stack(xnthread_user_task(threadin),NULL);
> +            xnpod_fatal("Hardened thread %s[%d] running in Linux domain?! (status=0x%lx, sig=%d, prev=%s[%d])",
> +			threadin->name,
> +			next->pid,
> +			status,
> +			sigpending,
> +			prev->comm,
> +			prev->pid);
> +	    }
> +        else if (!(next->ptrace & PT_PTRACED) &&
>  	    /* Allow ptraced threads to run shortly in order to
>  	       properly recover from a stopped state. */
>  	    testbits(status,XNSTARTED) &&
> -	    testbits(status,XNTHREAD_BLOCK_BITS))
> +	    testbits(status,XNPEND))
>  	    {
>  	    show_stack(xnthread_user_task(threadin),NULL);
>              xnpod_fatal("blocked thread %s[%d] rescheduled?! (status=0x%lx, sig=%d, prev=%s[%d])",
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Xenomai-core mailing list
> Xenomai-core@domain.hid
> https://mail.gna.org/listinfo/xenomai-core


-- 

Philippe.

Re: [Xenomai-core] [PATCH] fix pthread cancellation in native skin

[Philippe Gerum]

Philippe Gerum wrote:
> Gilles Chanteperdrix wrote:
> 
>> Gilles Chanteperdrix wrote:
>>  > This is not the only situation where a thread with a nucleus 
>> suspension
>>  > bit need to run shortly in secondary mode: it also occurs when
>>  > suspending with xnpod_suspend_thread() a thread running in secondary
>>  > mode; the thread receives the SIGCHLD signal and need to execute 
>> shortly
>>  > with the suspension bit set in order to cause a migration to primary
>>  > mode.
>>  >  > So, the only case when we are sure that a user-space thread can 
>> not be
>>  > scheduled by Linux seems to be when this thread does not have the
>>  > XNRELAX bit.
>>
>> From all the bits in XNTHREAD_BLOCK_BITS, only when the XNPEND bit is
>> set, a thread can not be running in secondary mode. Hence the proposed
>> patch.
>>
> 
> Almost ok, but XNDELAY might also be set alone, indicating a purely 
> timed wait state (i.e. without sync object to pend on, so XNPEND is off).
> 

Forget about this: XNDELAY might also be a transient bit like XNSUSP, so you are 
right, we cannot test it there.

>>
>>
>> ------------------------------------------------------------------------
>>
>> Index: ksrc/nucleus/shadow.c
>> ===================================================================
>> --- ksrc/nucleus/shadow.c    (revision 507)
>> +++ ksrc/nucleus/shadow.c    (working copy)
>> @@ -1543,14 +1543,25 @@
>>  
>>  #ifdef CONFIG_XENO_OPT_DEBUG
>>      {
>> -    xnflags_t status = threadin->status & ~XNRELAX;
>> +    xnflags_t status = threadin->status;
>>      int sigpending = signal_pending(next);
>>  
>> -        if (!(next->ptrace & PT_PTRACED) &&
>> +        if (!testbits(status, XNRELAX))
>> +            {
>> +            show_stack(xnthread_user_task(threadin),NULL);
>> +            xnpod_fatal("Hardened thread %s[%d] running in Linux 
>> domain?! (status=0x%lx, sig=%d, prev=%s[%d])",
>> +            threadin->name,
>> +            next->pid,
>> +            status,
>> +            sigpending,
>> +            prev->comm,
>> +            prev->pid);
>> +        }
>> +        else if (!(next->ptrace & PT_PTRACED) &&
>>          /* Allow ptraced threads to run shortly in order to
>>             properly recover from a stopped state. */
>>          testbits(status,XNSTARTED) &&
>> -        testbits(status,XNTHREAD_BLOCK_BITS))
>> +        testbits(status,XNPEND))
>>          {
>>          show_stack(xnthread_user_task(threadin),NULL);
>>              xnpod_fatal("blocked thread %s[%d] rescheduled?! 
>> (status=0x%lx, sig=%d, prev=%s[%d])",
>>
>>
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> Xenomai-core mailing list
>> Xenomai-core@domain.hid
>> https://mail.gna.org/listinfo/xenomai-core
> 
> 
> 


-- 

Philippe.

Re: [Xenomai-core] [PATCH] fix pthread cancellation in native skin

[Gilles Chanteperdrix]

Philippe Gerum wrote:
 > Philippe Gerum wrote:
 > > Gilles Chanteperdrix wrote:
 > > 
 > >> Gilles Chanteperdrix wrote:
 > >>  > This is not the only situation where a thread with a nucleus 
 > >> suspension
 > >>  > bit need to run shortly in secondary mode: it also occurs when
 > >>  > suspending with xnpod_suspend_thread() a thread running in secondary
 > >>  > mode; the thread receives the SIGCHLD signal and need to execute 
 > >> shortly
 > >>  > with the suspension bit set in order to cause a migration to primary
 > >>  > mode.
 > >>  >  > So, the only case when we are sure that a user-space thread can 
 > >> not be
 > >>  > scheduled by Linux seems to be when this thread does not have the
 > >>  > XNRELAX bit.
 > >>
 > >> From all the bits in XNTHREAD_BLOCK_BITS, only when the XNPEND bit is
 > >> set, a thread can not be running in secondary mode. Hence the proposed
 > >> patch.
 > >>
 > > 
 > > Almost ok, but XNDELAY might also be set alone, indicating a purely 
 > > timed wait state (i.e. without sync object to pend on, so XNPEND is off).
 > > 
 > 
 > Forget about this: XNDELAY might also be a transient bit like XNSUSP, so you are 
 > right, we cannot test it there.

Ok. The patch is applied to svn trunk.

-- 


					    Gilles Chanteperdrix.