Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : native: Rework handling of pthread carrier thread

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : native: Rework handling of pthread carrier thread

[Gilles Chanteperdrix]

GIT version control wrote:
> Module: xenomai-jki
> Branch: for-upstream
> Commit: 0352b068600bd4ef3172c8a42416badbcdad32ca
> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=0352b068600bd4ef3172c8a42416badbcdad32ca
> 
> Author: Jan Kiszka <jan.kiszka@domain.hid>
> Date:   Wed Apr 28 15:08:11 2010 +0200
> 
> native: Rework handling of pthread carrier thread
> 
> This patch improves two pitfalls of libnative's interaction with
> underlying pthreads:
> 
> First, it tries to detect double deletions (cancellations or joinings)
> of pthreads and report them via an error code. This reduces the risk to
> trigger a SIGSEGV accessing meanwhile released pthread objects. And
> second, it properly detaches joinable pthreads when they are deleted
> instead. This properly releases the pthread resources.

I really do not understand that. What is the point of creating a
joinable thread if you do not want to join it once it has been canceled?

-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : native: Rework handling of pthread carrier thread

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1365 bytes --]

Gilles Chanteperdrix wrote:
> GIT version control wrote:
>> Module: xenomai-jki
>> Branch: for-upstream
>> Commit: 0352b068600bd4ef3172c8a42416badbcdad32ca
>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=0352b068600bd4ef3172c8a42416badbcdad32ca
>>
>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>> Date:   Wed Apr 28 15:08:11 2010 +0200
>>
>> native: Rework handling of pthread carrier thread
>>
>> This patch improves two pitfalls of libnative's interaction with
>> underlying pthreads:
>>
>> First, it tries to detect double deletions (cancellations or joinings)
>> of pthreads and report them via an error code. This reduces the risk to
>> trigger a SIGSEGV accessing meanwhile released pthread objects. And
>> second, it properly detaches joinable pthreads when they are deleted
>> instead. This properly releases the pthread resources.
> 
> I really do not understand that. What is the point of creating a
> joinable thread if you do not want to join it once it has been canceled?
> 

Keep in mind that there is no pthread_cancel equivalent in the native
API. Moreover, even POSIX has pthread_detach so that you are not forced
to join every joinable thread. I could imagine that one may want to use
this fot error cleanups. But first of all this is about improving the
API consistency and fault tolerance.

Jan


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

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : native: Rework handling of pthread carrier thread

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> GIT version control wrote:
>>> Module: xenomai-jki
>>> Branch: for-upstream
>>> Commit: 0352b068600bd4ef3172c8a42416badbcdad32ca
>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=0352b068600bd4ef3172c8a42416badbcdad32ca
>>>
>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>> Date:   Wed Apr 28 15:08:11 2010 +0200
>>>
>>> native: Rework handling of pthread carrier thread
>>>
>>> This patch improves two pitfalls of libnative's interaction with
>>> underlying pthreads:
>>>
>>> First, it tries to detect double deletions (cancellations or joinings)
>>> of pthreads and report them via an error code. This reduces the risk to
>>> trigger a SIGSEGV accessing meanwhile released pthread objects. And
>>> second, it properly detaches joinable pthreads when they are deleted
>>> instead. This properly releases the pthread resources.
>> I really do not understand that. What is the point of creating a
>> joinable thread if you do not want to join it once it has been canceled?
>>
> 
> Keep in mind that there is no pthread_cancel equivalent in the native
> API. Moreover, even POSIX has pthread_detach so that you are not forced
> to join every joinable thread. I could imagine that one may want to use
> this fot error cleanups. But first of all this is about improving the
> API consistency and fault tolerance.

I have no strong opinion on that matter, but it looks to me like the
"joinable" stuff in the native API is made to look like the posix
equivalent, following the principle of least surprise. So, since posix
requires joining a thread after canceling, it should be necessary to
join a thread after deleting it.

As for pthread_detach, its main justification is a corner case: being
able to avoid leaks when canceling a thread which was blocked in a call
to pthread_join by calling pthread_detach in the cancelation cleanup
handler.

The reason for the way the native API is, is that you may delete a
thread which is in fact running in another process. And I am not sure
the patch you sent addresses this issue.

> 
> Jan
> 


-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : native: Rework handling of pthread carrier thread

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2684 bytes --]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> GIT version control wrote:
>>>> Module: xenomai-jki
>>>> Branch: for-upstream
>>>> Commit: 0352b068600bd4ef3172c8a42416badbcdad32ca
>>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=0352b068600bd4ef3172c8a42416badbcdad32ca
>>>>
>>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>>> Date:   Wed Apr 28 15:08:11 2010 +0200
>>>>
>>>> native: Rework handling of pthread carrier thread
>>>>
>>>> This patch improves two pitfalls of libnative's interaction with
>>>> underlying pthreads:
>>>>
>>>> First, it tries to detect double deletions (cancellations or joinings)
>>>> of pthreads and report them via an error code. This reduces the risk to
>>>> trigger a SIGSEGV accessing meanwhile released pthread objects. And
>>>> second, it properly detaches joinable pthreads when they are deleted
>>>> instead. This properly releases the pthread resources.
>>> I really do not understand that. What is the point of creating a
>>> joinable thread if you do not want to join it once it has been canceled?
>>>
>> Keep in mind that there is no pthread_cancel equivalent in the native
>> API. Moreover, even POSIX has pthread_detach so that you are not forced
>> to join every joinable thread. I could imagine that one may want to use
>> this fot error cleanups. But first of all this is about improving the
>> API consistency and fault tolerance.
> 
> I have no strong opinion on that matter, but it looks to me like the
> "joinable" stuff in the native API is made to look like the posix
> equivalent, following the principle of least surprise. So, since posix
> requires joining a thread after canceling, it should be necessary to
> join a thread after deleting it.

The problem is that there is no direct equivalent to rt_task_delete with
pthread. What comes closest is pthread_cancel + pthread_detach (where
required). Still, rt_task_delete is synchronous, pthread_cancel not.

> 
> As for pthread_detach, its main justification is a corner case: being
> able to avoid leaks when canceling a thread which was blocked in a call
> to pthread_join by calling pthread_detach in the cancelation cleanup
> handler.
> 
> The reason for the way the native API is, is that you may delete a
> thread which is in fact running in another process. And I am not sure
> the patch you sent addresses this issue.

I does not. And thinking about it, I guess we should simply deny remote
deletion of joinable tasks as there is no reasonable way to cleanly
release the remote pthread resources. I could post an update for code
and docs on top of this.

Jan


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

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : native: Rework handling of pthread carrier thread

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> GIT version control wrote:
>>>>> Module: xenomai-jki
>>>>> Branch: for-upstream
>>>>> Commit: 0352b068600bd4ef3172c8a42416badbcdad32ca
>>>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=0352b068600bd4ef3172c8a42416badbcdad32ca
>>>>>
>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>>>> Date:   Wed Apr 28 15:08:11 2010 +0200
>>>>>
>>>>> native: Rework handling of pthread carrier thread
>>>>>
>>>>> This patch improves two pitfalls of libnative's interaction with
>>>>> underlying pthreads:
>>>>>
>>>>> First, it tries to detect double deletions (cancellations or joinings)
>>>>> of pthreads and report them via an error code. This reduces the risk to
>>>>> trigger a SIGSEGV accessing meanwhile released pthread objects. And
>>>>> second, it properly detaches joinable pthreads when they are deleted
>>>>> instead. This properly releases the pthread resources.
>>>> I really do not understand that. What is the point of creating a
>>>> joinable thread if you do not want to join it once it has been canceled?
>>>>
>>> Keep in mind that there is no pthread_cancel equivalent in the native
>>> API. Moreover, even POSIX has pthread_detach so that you are not forced
>>> to join every joinable thread. I could imagine that one may want to use
>>> this fot error cleanups. But first of all this is about improving the
>>> API consistency and fault tolerance.
>> I have no strong opinion on that matter, but it looks to me like the
>> "joinable" stuff in the native API is made to look like the posix
>> equivalent, following the principle of least surprise. So, since posix
>> requires joining a thread after canceling, it should be necessary to
>> join a thread after deleting it.
> 
> The problem is that there is no direct equivalent to rt_task_delete with
> pthread. What comes closest is pthread_cancel + pthread_detach (where
> required). Still, rt_task_delete is synchronous, pthread_cancel not.

rt_task_delete looks to me like really equivalent to pthread_cancel.
I do not think rt_task_delete is supposed to be synchronous (it looks to
me it is not if you delete a thread running on another cpu than the
current one). And pthread_detach is not synchronous either. The only way
to wait for a thread deletion is to use pthread_join/rt_task_join.

-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : native: Rework handling of pthread carrier thread

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 3301 bytes --]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Gilles Chanteperdrix wrote:
>>>>> GIT version control wrote:
>>>>>> Module: xenomai-jki
>>>>>> Branch: for-upstream
>>>>>> Commit: 0352b068600bd4ef3172c8a42416badbcdad32ca
>>>>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=0352b068600bd4ef3172c8a42416badbcdad32ca
>>>>>>
>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>>>>> Date:   Wed Apr 28 15:08:11 2010 +0200
>>>>>>
>>>>>> native: Rework handling of pthread carrier thread
>>>>>>
>>>>>> This patch improves two pitfalls of libnative's interaction with
>>>>>> underlying pthreads:
>>>>>>
>>>>>> First, it tries to detect double deletions (cancellations or joinings)
>>>>>> of pthreads and report them via an error code. This reduces the risk to
>>>>>> trigger a SIGSEGV accessing meanwhile released pthread objects. And
>>>>>> second, it properly detaches joinable pthreads when they are deleted
>>>>>> instead. This properly releases the pthread resources.
>>>>> I really do not understand that. What is the point of creating a
>>>>> joinable thread if you do not want to join it once it has been canceled?
>>>>>
>>>> Keep in mind that there is no pthread_cancel equivalent in the native
>>>> API. Moreover, even POSIX has pthread_detach so that you are not forced
>>>> to join every joinable thread. I could imagine that one may want to use
>>>> this fot error cleanups. But first of all this is about improving the
>>>> API consistency and fault tolerance.
>>> I have no strong opinion on that matter, but it looks to me like the
>>> "joinable" stuff in the native API is made to look like the posix
>>> equivalent, following the principle of least surprise. So, since posix
>>> requires joining a thread after canceling, it should be necessary to
>>> join a thread after deleting it.
>> The problem is that there is no direct equivalent to rt_task_delete with
>> pthread. What comes closest is pthread_cancel + pthread_detach (where
>> required). Still, rt_task_delete is synchronous, pthread_cancel not.
> 
> rt_task_delete looks to me like really equivalent to pthread_cancel.

rt_task_delete was once designed to sweep a task object, not just to
terminate its runnable context (thought that's hard - if not impossible
- to maintain across process boundaries).

While you can join a canceled pthread, you can't do this reliably with a
deleted task. It happened to work if you were already holding a (then
outdated) handle. But, officially, the lifetime of a tasks ends when
rt_task_delete returns. No service is supposed to accept any further
requests regarding this object afterwards. Making rt_task_join an
exception may raise confusion.

> I do not think rt_task_delete is supposed to be synchronous (it looks to
> me it is not if you delete a thread running on another cpu than the
> current one). And pthread_detach is not synchronous either. The only way
> to wait for a thread deletion is to use pthread_join/rt_task_join.
> 

It is synchronous /wrt the validity of the task object which includes
the schedulable part in kernel space. The carrier thread is in pthread
hands, but Xenomai is out of the game when rt_task_delete returned.

Jan


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

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : native: Rework handling of pthread carrier thread

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> Jan Kiszka wrote:
>>>>> Gilles Chanteperdrix wrote:
>>>>>> GIT version control wrote:
>>>>>>> Module: xenomai-jki
>>>>>>> Branch: for-upstream
>>>>>>> Commit: 0352b068600bd4ef3172c8a42416badbcdad32ca
>>>>>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=0352b068600bd4ef3172c8a42416badbcdad32ca
>>>>>>>
>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>>>>>> Date:   Wed Apr 28 15:08:11 2010 +0200
>>>>>>>
>>>>>>> native: Rework handling of pthread carrier thread
>>>>>>>
>>>>>>> This patch improves two pitfalls of libnative's interaction with
>>>>>>> underlying pthreads:
>>>>>>>
>>>>>>> First, it tries to detect double deletions (cancellations or joinings)
>>>>>>> of pthreads and report them via an error code. This reduces the risk to
>>>>>>> trigger a SIGSEGV accessing meanwhile released pthread objects. And
>>>>>>> second, it properly detaches joinable pthreads when they are deleted
>>>>>>> instead. This properly releases the pthread resources.
>>>>>> I really do not understand that. What is the point of creating a
>>>>>> joinable thread if you do not want to join it once it has been canceled?
>>>>>>
>>>>> Keep in mind that there is no pthread_cancel equivalent in the native
>>>>> API. Moreover, even POSIX has pthread_detach so that you are not forced
>>>>> to join every joinable thread. I could imagine that one may want to use
>>>>> this fot error cleanups. But first of all this is about improving the
>>>>> API consistency and fault tolerance.
>>>> I have no strong opinion on that matter, but it looks to me like the
>>>> "joinable" stuff in the native API is made to look like the posix
>>>> equivalent, following the principle of least surprise. So, since posix
>>>> requires joining a thread after canceling, it should be necessary to
>>>> join a thread after deleting it.
>>> The problem is that there is no direct equivalent to rt_task_delete with
>>> pthread. What comes closest is pthread_cancel + pthread_detach (where
>>> required). Still, rt_task_delete is synchronous, pthread_cancel not.
>> rt_task_delete looks to me like really equivalent to pthread_cancel.
> 
> rt_task_delete was once designed to sweep a task object, not just to
> terminate its runnable context (thought that's hard - if not impossible
> - to maintain across process boundaries).
> 
> While you can join a canceled pthread, you can't do this reliably with a
> deleted task. It happened to work if you were already holding a (then
> outdated) handle. But, officially, the lifetime of a tasks ends when
> rt_task_delete returns. No service is supposed to accept any further
> requests regarding this object afterwards. Making rt_task_join an
> exception may raise confusion.
> 
>> I do not think rt_task_delete is supposed to be synchronous (it looks to
>> me it is not if you delete a thread running on another cpu than the
>> current one). And pthread_detach is not synchronous either. The only way
>> to wait for a thread deletion is to use pthread_join/rt_task_join.
>>
> 
> It is synchronous /wrt the validity of the task object which includes
> the schedulable part in kernel space. The carrier thread is in pthread
> hands, but Xenomai is out of the game when rt_task_delete returned.

Not being able to join a deleted task undermines seriously the utility
of rt_task_join... What is it useful for, then?


-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : native: Rework handling of pthread carrier thread

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 3654 bytes --]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Gilles Chanteperdrix wrote:
>>>>> Jan Kiszka wrote:
>>>>>> Gilles Chanteperdrix wrote:
>>>>>>> GIT version control wrote:
>>>>>>>> Module: xenomai-jki
>>>>>>>> Branch: for-upstream
>>>>>>>> Commit: 0352b068600bd4ef3172c8a42416badbcdad32ca
>>>>>>>> URL:    http://git.xenomai.org/?p=xenomai-jki.git;a=commit;h=0352b068600bd4ef3172c8a42416badbcdad32ca
>>>>>>>>
>>>>>>>> Author: Jan Kiszka <jan.kiszka@domain.hid>
>>>>>>>> Date:   Wed Apr 28 15:08:11 2010 +0200
>>>>>>>>
>>>>>>>> native: Rework handling of pthread carrier thread
>>>>>>>>
>>>>>>>> This patch improves two pitfalls of libnative's interaction with
>>>>>>>> underlying pthreads:
>>>>>>>>
>>>>>>>> First, it tries to detect double deletions (cancellations or joinings)
>>>>>>>> of pthreads and report them via an error code. This reduces the risk to
>>>>>>>> trigger a SIGSEGV accessing meanwhile released pthread objects. And
>>>>>>>> second, it properly detaches joinable pthreads when they are deleted
>>>>>>>> instead. This properly releases the pthread resources.
>>>>>>> I really do not understand that. What is the point of creating a
>>>>>>> joinable thread if you do not want to join it once it has been canceled?
>>>>>>>
>>>>>> Keep in mind that there is no pthread_cancel equivalent in the native
>>>>>> API. Moreover, even POSIX has pthread_detach so that you are not forced
>>>>>> to join every joinable thread. I could imagine that one may want to use
>>>>>> this fot error cleanups. But first of all this is about improving the
>>>>>> API consistency and fault tolerance.
>>>>> I have no strong opinion on that matter, but it looks to me like the
>>>>> "joinable" stuff in the native API is made to look like the posix
>>>>> equivalent, following the principle of least surprise. So, since posix
>>>>> requires joining a thread after canceling, it should be necessary to
>>>>> join a thread after deleting it.
>>>> The problem is that there is no direct equivalent to rt_task_delete with
>>>> pthread. What comes closest is pthread_cancel + pthread_detach (where
>>>> required). Still, rt_task_delete is synchronous, pthread_cancel not.
>>> rt_task_delete looks to me like really equivalent to pthread_cancel.
>> rt_task_delete was once designed to sweep a task object, not just to
>> terminate its runnable context (thought that's hard - if not impossible
>> - to maintain across process boundaries).
>>
>> While you can join a canceled pthread, you can't do this reliably with a
>> deleted task. It happened to work if you were already holding a (then
>> outdated) handle. But, officially, the lifetime of a tasks ends when
>> rt_task_delete returns. No service is supposed to accept any further
>> requests regarding this object afterwards. Making rt_task_join an
>> exception may raise confusion.
>>
>>> I do not think rt_task_delete is supposed to be synchronous (it looks to
>>> me it is not if you delete a thread running on another cpu than the
>>> current one). And pthread_detach is not synchronous either. The only way
>>> to wait for a thread deletion is to use pthread_join/rt_task_join.
>>>
>> It is synchronous /wrt the validity of the task object which includes
>> the schedulable part in kernel space. The carrier thread is in pthread
>> hands, but Xenomai is out of the game when rt_task_delete returned.
> 
> Not being able to join a deleted task undermines seriously the utility
> of rt_task_join... What is it useful for, then?

See the doc: You join or delete, but not both.

Jan


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

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : native: Rework handling of pthread carrier thread

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Not being able to join a deleted task undermines seriously the utility
>> of rt_task_join... What is it useful for, then?
> 
> See the doc: You join or delete, but not both.

Looks to me like a workaround: you are not able to handle properly the
life-cycle of an RT_TASK, so you changed the rules... is there really no
other way?

-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : native: Rework handling of pthread carrier thread

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 810 bytes --]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Not being able to join a deleted task undermines seriously the utility
>>> of rt_task_join... What is it useful for, then?
>> See the doc: You join or delete, but not both.
> 
> Looks to me like a workaround: you are not able to handle properly the
> life-cycle of an RT_TASK, so you changed the rules... is there really no
> other way?

First of all, it turns a SEGV into a proper error code. Moreover,
rt_task_join now officially finalizes the object for native (it always
did technically!), as does pthread_join for POSIX. No rule changed, just
rule documentation and violation detection added. One may argue about
join after delete, but for what use case such a change of rt_task_delete
semantic?

Jan


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

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : native: Rework handling of pthread carrier thread

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> Not being able to join a deleted task undermines seriously the utility
>>>> of rt_task_join... What is it useful for, then?
>>> See the doc: You join or delete, but not both.
>> Looks to me like a workaround: you are not able to handle properly the
>> life-cycle of an RT_TASK, so you changed the rules... is there really no
>> other way?
> 
> First of all, it turns a SEGV into a proper error code. Moreover,
> rt_task_join now officially finalizes the object for native (it always
> did technically!), as does pthread_join for POSIX. No rule changed, just
> rule documentation and violation detection added. One may argue about
> join after delete, but for what use case such a change of rt_task_delete
> semantic?

rt_task_delete, pthread_cancel, pthread_detach are not synchronous,
after one of these calls, you do not have any guarantee that the
ressources attached to a thread are free for re-use. After pthread_join,
you do.

So, the question is what is more important, to have a way to wait for
the ressources attached to a thread to be freed? Or to avoid
segmentation faults in broken application?

I do not generally mind segmentation faults in broken applications if we
can not avoid them without breaking other applications. A segmentation
fault is one of the simplest things to debug.

However, I do not have a strong opinion on this, it is just an open
question. More generally, I would like us to discuss once and for all
about the semantic of the various calls and their effect on the RT_TASK
duration, instead of changing this semantic every release and risk
breaking non-broken applications (I mean, the one which do not segfault).

-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : native: Rework handling of pthread carrier thread

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1632 bytes --]

Gilles Chanteperdrix wrote:
> However, I do not have a strong opinion on this, it is just an open
> question. More generally, I would like us to discuss once and for all
> about the semantic of the various calls and their effect on the RT_TASK
> duration, instead of changing this semantic every release and risk
> breaking non-broken applications (I mean, the one which do not segfault).

To pick up this issue again (in order to get my queue flushed):

We basically have to decide about the question what rt_task_delete
invalidates and what impact this shall have on rt_task_join. It is
already documented that rt_task_delete invalidates (and releases) the
kernel-side resources of a RT_TASK. The question is what shall happen to
the not explicitly mentioned user-side resources (ie. the pthread -
where available).

Option 1 is to decouple both and keep the user side of a joinable
RT_TASK alive until it is explicitly joined. Option 2 could be to
declare both parts invalid on rt_task_delete. Based on this decision,
the finalization logic of rt_task_delete and rt_task_join then needs to
be adjusted to deliver the right behavior, including proper error codes
instead of sporadic SEGV.

Do we expect applications to rely on this joinability after
rt_task_delete? If yes, we should make it official, document the
descriptor split and the fact that the descriptor cannot be looked up
anymore after deletion but has to be saved beforehand.

Independently, we need to clarify that cross-process join is not
supported. Trying to do this ATM will result in a SEGV (something I
missed so far).

Jan


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

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : native: Rework handling of pthread carrier thread

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> However, I do not have a strong opinion on this, it is just an open
>> question. More generally, I would like us to discuss once and for all
>> about the semantic of the various calls and their effect on the RT_TASK
>> duration, instead of changing this semantic every release and risk
>> breaking non-broken applications (I mean, the one which do not segfault).
> 
> To pick up this issue again (in order to get my queue flushed):
> 
> We basically have to decide about the question what rt_task_delete
> invalidates and what impact this shall have on rt_task_join. It is
> already documented that rt_task_delete invalidates (and releases) the
> kernel-side resources of a RT_TASK. The question is what shall happen to
> the not explicitly mentioned user-side resources (ie. the pthread -
> where available).
> 
> Option 1 is to decouple both and keep the user side of a joinable
> RT_TASK alive until it is explicitly joined. Option 2 could be to
> declare both parts invalid on rt_task_delete. Based on this decision,
> the finalization logic of rt_task_delete and rt_task_join then needs to
> be adjusted to deliver the right behavior, including proper error codes
> instead of sporadic SEGV.

Relying on the contents of the RT_TASK structure to know the state of a
task is bound to fail: the RT_TASK structure may be copied around, so
changing the contents of the RT_TASK structure in rt_task_delete, to use
that information later will only work if the same RT_TASK structure is
used later. This is fragile.

> 
> Do we expect applications to rely on this joinability after
> rt_task_delete? If yes, we should make it official, document the
> descriptor split and the fact that the descriptor cannot be looked up
> anymore after deletion but has to be saved beforehand.
> 
> Independently, we need to clarify that cross-process join is not
> supported. Trying to do this ATM will result in a SEGV (something I
> missed so far).

This is a regression. At some point in the past, a NULL pthread_t opaque
pointer was used to mean that the thread was living in a different
process, and rt_task_delete would skip the pthread_cancel.

-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : native: Rework handling of pthread carrier thread

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2734 bytes --]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> However, I do not have a strong opinion on this, it is just an open
>>> question. More generally, I would like us to discuss once and for all
>>> about the semantic of the various calls and their effect on the RT_TASK
>>> duration, instead of changing this semantic every release and risk
>>> breaking non-broken applications (I mean, the one which do not segfault).
>> To pick up this issue again (in order to get my queue flushed):
>>
>> We basically have to decide about the question what rt_task_delete
>> invalidates and what impact this shall have on rt_task_join. It is
>> already documented that rt_task_delete invalidates (and releases) the
>> kernel-side resources of a RT_TASK. The question is what shall happen to
>> the not explicitly mentioned user-side resources (ie. the pthread -
>> where available).
>>
>> Option 1 is to decouple both and keep the user side of a joinable
>> RT_TASK alive until it is explicitly joined. Option 2 could be to
>> declare both parts invalid on rt_task_delete. Based on this decision,
>> the finalization logic of rt_task_delete and rt_task_join then needs to
>> be adjusted to deliver the right behavior, including proper error codes
>> instead of sporadic SEGV.
> 
> Relying on the contents of the RT_TASK structure to know the state of a
> task is bound to fail: the RT_TASK structure may be copied around, so
> changing the contents of the RT_TASK structure in rt_task_delete, to use
> that information later will only work if the same RT_TASK structure is
> used later. This is fragile.

That's true but somehow the best we can do to detect errors that remain
fuzzy otherwise. We neither have a list of all user space RT_TASK
structs nor any in-kernel object to ask after rt_task_delete or join.

> 
>> Do we expect applications to rely on this joinability after
>> rt_task_delete? If yes, we should make it official, document the
>> descriptor split and the fact that the descriptor cannot be looked up
>> anymore after deletion but has to be saved beforehand.
>>
>> Independently, we need to clarify that cross-process join is not
>> supported. Trying to do this ATM will result in a SEGV (something I
>> missed so far).
> 
> This is a regression. At some point in the past, a NULL pthread_t opaque
> pointer was used to mean that the thread was living in a different
> process, and rt_task_delete would skip the pthread_cancel.
> 

I was talking about rt_task_join on a foreign RT_TASK. And I was wrong,
it actually works with and without my patch SEGV-free. It just lacks
documentation.

But you did not address the core questions.

Jan


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

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : native: Rework handling of pthread carrier thread

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
>>>> However, I do not have a strong opinion on this, it is just an open
>>>> question. More generally, I would like us to discuss once and for all
>>>> about the semantic of the various calls and their effect on the RT_TASK
>>>> duration, instead of changing this semantic every release and risk
>>>> breaking non-broken applications (I mean, the one which do not segfault).
>>> To pick up this issue again (in order to get my queue flushed):
>>>
>>> We basically have to decide about the question what rt_task_delete
>>> invalidates and what impact this shall have on rt_task_join. It is
>>> already documented that rt_task_delete invalidates (and releases) the
>>> kernel-side resources of a RT_TASK. The question is what shall happen to
>>> the not explicitly mentioned user-side resources (ie. the pthread -
>>> where available).
>>>
>>> Option 1 is to decouple both and keep the user side of a joinable
>>> RT_TASK alive until it is explicitly joined. Option 2 could be to
>>> declare both parts invalid on rt_task_delete. Based on this decision,
>>> the finalization logic of rt_task_delete and rt_task_join then needs to
>>> be adjusted to deliver the right behavior, including proper error codes
>>> instead of sporadic SEGV.
>> Relying on the contents of the RT_TASK structure to know the state of a
>> task is bound to fail: the RT_TASK structure may be copied around, so
>> changing the contents of the RT_TASK structure in rt_task_delete, to use
>> that information later will only work if the same RT_TASK structure is
>> used later. This is fragile.
> 
> That's true but somehow the best we can do to detect errors that remain
> fuzzy otherwise. We neither have a list of all user space RT_TASK
> structs nor any in-kernel object to ask after rt_task_delete or join.
> 
>>> Do we expect applications to rely on this joinability after
>>> rt_task_delete? If yes, we should make it official, document the
>>> descriptor split and the fact that the descriptor cannot be looked up
>>> anymore after deletion but has to be saved beforehand.
>>>
>>> Independently, we need to clarify that cross-process join is not
>>> supported. Trying to do this ATM will result in a SEGV (something I
>>> missed so far).
>> This is a regression. At some point in the past, a NULL pthread_t opaque
>> pointer was used to mean that the thread was living in a different
>> process, and rt_task_delete would skip the pthread_cancel.
>>
> 
> I was talking about rt_task_join on a foreign RT_TASK. And I was wrong,
> it actually works with and without my patch SEGV-free. It just lacks
> documentation.
> 
> But you did not address the core questions.

Xenomai libraries rely on glibc services for the
creation/deletion/joining of threads. It happens that when we misuse
Xenomai services, we end up misusing glibc services, and the glibc
developers chose, in that case to have a segmentation fault. So, I would
say, the behaviour you do not like comes from glibc, not Xenomai. If
there was a simple way to workaround this behaviour I would say go for
it, but we now realize that working around it correctly requires
overkill solutions. So, no, I will not merge an half-working workaround,
if you want the issue properly fixed, fix it in the glibc. But I doubt
it will be easy to convince the glibc developers to add some code to
handle nicely a case which only happens when the libc is misused.

As for the rt_task_delete/rt_task_join question, I think we should have
to call rt_task_join after deleting  a thread, because that is the only
way to make sure that all the ressources associated to a thread are free.

> 
> Jan
> 


-- 
					    Gilles.

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : native: Rework handling of pthread carrier thread

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 580 bytes --]

Gilles Chanteperdrix wrote:
> As for the rt_task_delete/rt_task_join question, I think we should have
> to call rt_task_join after deleting  a thread, because that is the only
> way to make sure that all the ressources associated to a thread are free.

The question is not how we can release resources (that's trivial:
pthread_detach) but if we officially provide the service "join after
delete" to the user. If we do this, its limitations and implications on
the native interface need to be documented. So far this is fuzzy unless
you know our implementation.

Jan


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

Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : native: Rework handling of pthread carrier thread

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> As for the rt_task_delete/rt_task_join question, I think we should have
>> to call rt_task_join after deleting  a thread, because that is the only
>> way to make sure that all the ressources associated to a thread are free.
> 
> The question is not how we can release resources (that's trivial:
> pthread_detach) but if we officially provide the service "join after
> delete" to the user. If we do this, its limitations and implications on
> the native interface need to be documented. So far this is fuzzy unless
> you know our implementation.

No pthread_detach does not insure you that the ressources are released.
It insures you that they will be released at a later point in time. That
is the point. The only way to ensure that the ressources have been
released, or more precisely, to wait for them to be released, is to call
phtread_join. If you did not want to wait for that, then there was no
point to create the thread joinable in the first place.

Anyway, yes, I agree, this behaviour of rt_task_join should be documented.

> 
> Jan
> 


-- 
					    Gilles.