[ECOS] Bug in timed message box get function ?

[ECOS] Bug in timed message box get function ?

[Alex Brown]

I think I have found a bug in the timed mbox get function.

The symptom is that it only accurately times out the first time it is
called. Further calls will always return immediately.

The problem (from what I can see), is that the alarm routines used in
mboxt2.inl take absolute times yet they are always passed in a delay.

I made a small change to mbox.cxx that solved the problem for me:

New:

Cyg_Mbox::get( cyg_tick_count timeout )
{
    void * p;
    if ( ! m.get( p, Cyg_Clock::real_time_clock->current_value()+timeout ) )
        return NULL;
    return p;
}

Old:

Cyg_Mbox::get( cyg_tick_count timeout )
{
    void * p;
    if ( ! m.get( p, timeout ) )
        return NULL;
    return p;
}

Is this a bug ? or am I doing something wrong ?

Regards,

Alex Brown
agb@cea.com.au

Re: [ECOS] Bug in timed message box get function ?

[Hugo Tyson]

"Alex Brown" <agb@cea.com.au> writes:
> I think I have found a bug in the timed mbox get function.
> 
> The symptom is that it only accurately times out the first time it is
> called. Further calls will always return immediately.
> 
> The problem (from what I can see), is that the alarm routines used in
> mboxt2.inl take absolute times yet they are always passed in a delay.
> 
> I made a small change to mbox.cxx that solved the problem for me:
> 
> New:
> 
> Cyg_Mbox::get( cyg_tick_count timeout )
> {
>     void * p;
>     if ( ! m.get( p, Cyg_Clock::real_time_clock->current_value()+timeout ) )
>         return NULL;
>     return p;
> }
> 
> Old:
> 
> Cyg_Mbox::get( cyg_tick_count timeout )
> {
>     void * p;
>     if ( ! m.get( p, timeout ) )
>         return NULL;
>     return p;
> }
> 
> Is this a bug ? or am I doing something wrong ?

Documentation error. ;-)

Kernel functions use an absolute time for timeout arguments.  This moves
the problem of atomically adding "now" to the increment that the programmer
is thinking about, into application (or compatibility layer) code.

You can make a relative time API from an absolute time API trivially.
It is not possible to make a correct absolute time API from a relative time
API.  So we provide an absolute time API.

So yes, I guess, you should be writing

	Cyg_Clock::real_time_clock->current_value()+timeout

in your application, where you make the call, rather than within the
kernel.

More analysis: suppose you want to timeout in 5 seconds.  So you make the
call at time 100 seconds.  Your thread is pre-empted by more important
stuff, so it gets descheduled for 1 second.

With your suggested implementation, you might get woken up at time 105S or
106S depending on whether the deschedule occurs before or after reading
"now".  Some might call *that* a bug.

With our implementation, if you write code that goes:

  int donetime = Cyg_Clock::real_time_clock->current_value()+timeout;

  printf( "Wakeup at %d\n", donetime );
  Cyg_Mbox::get( donetime );

You know for certain what you asked for, and you know for certain what the
system will do.  The API is unabiguous.  If you get descheduled until after
donetime the call returns immediately.  I hope you see the advantages now?

HTH,
	- Huge


PS Of course, you can still write code with bugs in if you want:  ;-)

  printf( "Wakeup at %d\n",
	Cyg_Clock::real_time_clock->current_value()+timeout );
  Cyg_Mbox::get(
	Cyg_Clock::real_time_clock->current_value()+timeout );

that code can lie to you about when the wakeup is - and with a *relative*
time API, it's not possible to prevent that error occurring within the
kernel call itself.