[ECOS] Re: Thread destructors

[ECOS] Re: Thread destructors

[Ivan Horvat]

Hi Chris,

> Thanks for the fast response. I hope you've had a happy 
> easter.


Thanks you for the wishes, I hope you had happy easter too (they say 
it's never late). Sorry for the delay,
I'm having some problems with my email-server...

> I didn't grasp this at first, but I guess that what you are 
> saying is that even though I have exited the thread for which 
> I would like to free up memory I can't just do that in the 
> reaper thread as it still "being used" by the scheduler. So 
> in the reaper thread I have to call cyg_thread_delete before 
> doing any clean-up (ie stack, thread object and handle)? Does 
> this also mean that if I wanted I could restart the thread by 
> calling the cyg_thread_resume? 

It is actually safe to free the stack in "reaper" even *before* deleting 
the thread (as the reaper has the lower priority
and the exited thread won't run any more - it is in  EXITED state). 
Thread handle may be freed at any time,
but the thread object can be cleaned-up *only after* deleting the thread.

After the thread calls the cyg_thread_exit(), there is still a "zombie" 
thread object that  never again gets the
RUNNING state (it is now in state EXITED), but it is still "hanging 
arround" in scheduler thread tables.
The bad thing is that after this "zombie" descriptor is used last time 
in Cyg_Scheduler::unlock_inner() in call to
HAL_THREAD_SWITCH_CONTEXT(&c, &n), it still contains *vital* information 
for scheduler in form of
linked-list pointers to other thread objects. The 
cyg_thread_delete(thread) actually calls the th->~Cyg_Thread(),
where the associated thread descrptor is obtained as:

   Cyg_Thread *th = (Cyg_Thread *)thread;

The Cyg_Thread::~Cyg_Thread() calls Cyg_Thread::remove_from_list(), 
which actually removes the object from
the scheduler's list and only now it is safe to free the space occupied 
by the thread descriptor. But of course, the
function cyg_thread_delete() cannot be called by the thread to delete 
itself, as it calls the Cyg_Thread::kill()
which in turn checks if it is the current thread that is about to be 
"killed":

   if( this == Cyg_Scheduler::get_current_thread() )
      exit();

So, cyg_thread_delete() must be used from another thread (context) e.g. 
"reaper" thread.
I think exited threads cannot be restarted with cyg_thread_resume(), but 
as far as I know, it is safe not to free the
space of thread object and pass it to another cyg_thread_create(). This 
"reuse" doesn't seem to harm the scheduler...


Your implementation of ReaperThread (under the condition it has lower 
priority than any other thread it can
possibly reap) can be safely rewritten as:

void ReaperThread(cyg_addrword_t pParam)
{

   // pParam is a pointer to our message box
   cyg_handle_t hMsgBox = *((cyg_handle_t *)pParam);

   while(1) {
      // Wait for an exiting thread to send us a message
      // The message contains the address to its data
      ThreadData *ptp = cyg_mbox_get(hMsgBox);
      // Delete the threads allocated stack   // MOVED BEFORE DELETE()
      delete [] ptp->pStack;   // MOVED BEFORE DELETE()
      // "Disconnect" thread for the scheduler
      cyg_thread_delete(ptp->hThread);

      // Delete rest of its data (handle,object, etc..)
      delete ptp;
   }

}

However, this is not necessary, and I will use your original 
implementation of ReaperThread()...


Best Regards,
Ivan.




-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

[ECOS] Re: Thread destructors

[Chris Nimmers]

Hi Ivan,

Thank you for the in-depth info. It is now crystal clear! :)

Regards,
Chris 

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

[ECOS] Re: Thread destructors

[Chris Nimmers]

Hi Ivan,

Thanks for the fast response. I hope you've had a happy 
easter.

>> I have read somewhere the suggestion of using message 
boxes. 
>> Does this mean that I can can put a message in a message 
box 
>> with for example the stack address and then let another 
>> thread do the clean up? Will this work as long as the 
clean-
>> up thread has a lower priority than the thread exiting?
>
>This would work, under exactly these conditions you already 
mentioned:
>1) the cleaning is done from *another* context (i.e. thread)
>2) the other thread that performs cleaning *must* have lower 
priority 
>than any other thread it is supposed to clean
>
>Yet, this is incomplete... The thread whose resources are to 
be 
>deallocated must first perform cyg_thread_exit()
>(of course, otherwise the lower priority thread wouldn't get 
chance to 
>run at all). But, remember the
>cyg_thread_create() function and its last parameter? It is a 
pointer to 
>"cyg_thread" object, that is handled as a
>"black-box" structure. However, this structure is also often 
allocated 
>by malloc(), and *cannot* just be freed in the
>cleaner (or usually called reaper) thread, as the scheduler 
depends on 
>the link pointers in that structure in order to
>operate properly. If this space is just freed, then other 
thread may 
>obtain it and corrupt the pointers causing the
>scheduler to crash. Thus, to fully clean the thread, you 
must first call 
>cyg_thread_delete() with the "cyg_handle_t"
>parameter associated to the "cyg_thread" object in question, 
which would 
>remove that "cyg_thread" object from
>the scheduler's list. Only now, it is "safe" to free the 
space occupied 
>by the "cyg_thread" object.
>
> From my experience: if you don't perform the 
cyg_thread_delete() then 
>the "cyg_thread" object can be "reused", but
>only for the purposes of creating the new thread (i.e. 
passing it to the 
>cyg_thread_create() as its last argument).
>Any other use (usually) leads to imminent scheduler crash...
>

I didn't grasp this at first, but I guess that what you are 
saying is that even though I have exited the thread for which 
I would like to free up memory I can't just do that in the 
reaper thread as it still "being used" by the scheduler. So 
in the reaper thread I have to call cyg_thread_delete before 
doing any clean-up (ie stack, thread object and handle)? Does 
this also mean that if I wanted I could restart the thread by 
calling the cyg_thread_resume? 
  
void ReaperThread(cyg_addrword_t pParam) 
{

  // pParam is a pointer to our message box
  cyg_handle_t hMsgBox = *((cyg_handle_t *)pParam);

  while(1) {
    // Wait for an exiting thread to send us a message
    // The message contains the address to its data
    ThreadData *ptp = cyg_mbox_get(hMsgBox);
    // "Disconnect" thread for the scheduler
    cyg_thread_delete(ptp->hThread);
    // Delete the threads allocated stack
    delete [] ptp->pStack;
    // Delete rest of its data (handle,object, etc..)
    delete ptp;
  }

}

Regards,

Chris

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

[ECOS] Re: Thread destructors

[Ivan Horvat]

Hi Chris,

I may give you some answers, as I struggled with the same problems. 
However, this is only
from my picture of the eCos threads and related problems, so keep that 
in mind. ;-)

> I have read the suggestions here regarding thread destructors 
> and from what I understand it is illegal to do stack clean up 
> in the destructor as it is it running in its own context(?). 

I assume you are asking if it is possible for a thread that is about to 
exit, to call its own destructor.
In other words, if the thread can clean-up after itself? The answer is 
no, but there is one more issue
to be considered, besides the stack problem.
The destructor freeing the thread's stack is actually "digging the hole 
under its feet", as it runs on the
very same stack that it is about to free.
But if you assume that free() wouldn't corrupt the space it just freed, 
you may be tempted to lock the scheduler,
free the stack and than call cyg_thread_exit(). No other thread would 
get a chance to run, so the freed stack
wouldn't be corrupted (as eCos itself never uses malloc/free) and the 
scheduler would be unlocked in the method
Cyg_Scheduler::unlock_inner() called at the end of Cyg_Thread::exit().
However, this approach is *very dirty* (thus not advisable) and 
*incomplete* (as I willll try to explain)!

> I guess that this also means that pParam input to the 
> destructor is also on the stack and if it would contain the 
> address to the allocated stack and be freed this would mess 
> up the memory. Correct? 

It is actually processor-dependent where is the "pParam" variable. On 
Intel it would be on the stack, while
on e.g. SPARC or MIPS processors it will be in one of the "input" 
registers. But if it points to the stack
(context) of the thread executing the destructor, it is illegal as it 
would free the stack-memory on which the
thread is currently running. Thus, the proposed destructor:

// Illegal destructor!?
ThreadDestructor(cyg_addrword_t pParam)
{
	// Release allocated stack
        // pParam contains the stack address
	delete [] (cyg_unit8 *)pParam;
}

is illegal.

> I have read somewhere the suggestion of using message boxes. 
> Does this mean that I can can put a message in a message box 
> with for example the stack address and then let another 
> thread do the clean up? Will this work as long as the clean-
> up thread has a lower priority than the thread exiting?

This would work, under exactly these conditions you already mentioned:
1) the cleaning is done from *another* context (i.e. thread)
2) the other thread that performs cleaning *must* have lower priority 
than any other thread it is supposed to clean

Yet, this is incomplete... The thread whose resources are to be 
deallocated must first perform cyg_thread_exit()
(of course, otherwise the lower priority thread wouldn't get chance to 
run at all). But, remember the
cyg_thread_create() function and its last parameter? It is a pointer to 
"cyg_thread" object, that is handled as a
"black-box" structure. However, this structure is also often allocated 
by malloc(), and *cannot* just be freed in the
cleaner (or usually called reaper) thread, as the scheduler depends on 
the link pointers in that structure in order to
operate properly. If this space is just freed, then other thread may 
obtain it and corrupt the pointers causing the
scheduler to crash. Thus, to fully clean the thread, you must first call 
cyg_thread_delete() with the "cyg_handle_t"
parameter associated to the "cyg_thread" object in question, which would 
remove that "cyg_thread" object from
the scheduler's list. Only now, it is "safe" to free the space occupied 
by the "cyg_thread" object.

 From my experience: if you don't perform the cyg_thread_delete() then 
the "cyg_thread" object can be "reused", but
only for the purposes of creating the new thread (i.e. passing it to the 
cyg_thread_create() as its last argument).
Any other use (usually) leads to imminent scheduler crash...


I hope this helps a bit.
Any constructive flame is welcome. :-)

Regards,
Ivan.



-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss