This is the mail archive of the
ecos-discuss@sources.redhat.com
mailing list for the eCos project.
Re: Some bugs in ECOS kernel
- From: Jonathan Larmour <jifl at eCosCentric dot com>
- To: Кондратенко Антон Ал ександров <kondratenko at tecon dot ru>
- Cc: ecos-discuss at sources dot redhat dot com,Nick Garnett <nickg at ecosCentric dot com>
- Date: Thu, 04 Mar 2004 02:55:15 +0000
- Subject: Re: [ECOS] Some bugs in ECOS kernel
- References: <4ACB265B18E9DD42A37EC8D656BA13C532BD58@mail.teconpc.local>
Кондратенко Антон Александров wrote:
Hello, all:
I think, i found a few bugs in ECOS kernel.
1.
Please take look at folowing source code:
"packages\kernel\current\src\common\kapi.cxx"
externC cyg_bool_t cyg_thread_delete( cyg_handle_t thread )
{
Cyg_Thread *th = (Cyg_Thread *)thread;
if( th->get_state() != Cyg_Thread::EXITED )
th->kill(); // encourage it to terminate
if( th->get_state() != Cyg_Thread::EXITED )
return false; // it didn't run yet, leave it up to the app to fix
th->~Cyg_Thread();
return true;
}
At first in ECOS Docs, this function described as returning void.
And second is it:
when we trying to delete not EXITED thread, 90% that th->kill() will not switch thread to EXITED state
immediately (whereas if thread in SLEEPING state, kill() only wake up thread and exit ), and thread will not deleted.
So, if we wrote code in accordance with ECOS docs, we'll have a troubles if after deleting thread, we freeing thread resources.
You mean there's an omission in the documentation, not a bug in the kernel
:-). So I've corrected the documentation, thanks.
2.
see "packages\kernel\current\src\common\thread.cxx"
Function Cyg_Thread::kill(), didn't check a state of thread before check "sleep_reason". So if thread is running but "sleep_reason" not NONE (whereas after last sleeping sleep_reason not resetting to NONE), kill() will not really kill a running thread. This is a big problem if we want to delete thread in function of thread destructor ( via new thread created in destructor). See also bug 1.
The thread will be put into the EXIT state and be woken up (although in
your case it already is, but the point is that its state will then be
cleared of SLEEPING) and if you check the return value of cyg_thread_delete
you'll see the thread delete failed - if the new thread is lower priority
than the thread to be killed then the thread to be killed will complete the
exit() function and end up in the EXITED state. If the new thread is higher
priority than the thread to be killed then the thread to be killed wouldn't
be able to run to call exit() anyway so the thread delete would fail anyway.
3.
see "packages\kernel\current\src\common\thread.cxx"
Cyg_Thread::kill().
Try to kill thread that in RUNNING state, will not invoke thread destructors whereas destructors calling in Cyg_Thread::exit(), but kill() invoke exit() only if thread was in SLEEPING state ( via "wake_reason=EXIT")
Hmm... I'm not decided about this one. It's true the behaviour isn't very
symmetric, but loosely part of the point is that a kill is very much
*forcing* a thread to exit. As the comment about Cyg_Thread::kill says:
// Kill thread. Force the thread into EXITED state externally, or
// make it wake up and call exit().
And the documentation says:
"However killing a thread
is generally rather dangerous because no attempt is made to unlock any
synchronization primitives currently owned by that thread or release
any other resources that thread may have claimed. Therefore use of
this function should be avoided, and cyg_thread_exit is preferred."
I would consider thread destructors to be part and parcel of that. At the
same time the assymetry irks me as the caller arguably won't know if the
thread is sleeping or not and therefore whether the thread destructors get
called or not is non-deterministic. I've CC'd Nick for his opinion on this,
but I don't see any easy solution that would make the thread destructors be
called in the context of the killed thread. Short of setting the PC to
&Cyg_Thread::exit anyway! Therefore I'm tempted to go along with the
documentation saying that resources may not be released and this function
is dangerous and to be avoided.
If you would care to propose a simple deterministic solution then I'm sure
we'd consider it.
Please fix me if I wrong.
I don't think I've ever patched a human before ;-).
And fix ECOS otherwise.
I've fixed the doc issue anyway.
Jifl
--
eCosCentric http://www.eCosCentric.com/ The eCos and RedBoot experts
>>>>> Visit us in booth 2527 at the Embedded Systems Conference 2004 <<<<<
March 30 - April 1, San Francisco http://www.esconline.com/electronicaUSA/
--["No sense being pessimistic, it wouldn't work anyway"]-- Opinions==mine
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss