Re: [ECOS] Wake select() with a signal

[Roland Caßebohm <roland dot cassebohm at visionsystems dot de>]

On Freitag, 8. November 2002 00:00, Nick Garnett wrote:
> Roland Caßebohm <roland.cassebohm@visionsystems.de> writes:
> > On Mittwoch, 6. November 2002 18:13, Nick Garnett wrote:
> > > In Unix I suppose one way of fixing this would have been to add a pipe
> > > to the set of read FDs and have the other thread write to that to wake
> > > the select() -- you probably don't need to use a signal at all. We do
> > > not have pipes in eCos, but a loop-back TCP socket would probably do
> > > the same thing.
> > >
> > > The POSIX-200X standard has added a pselect() call, which takes an
> > > additional signal mask argument, specifically to allow this race
> > > condition to be eliminated.
> >
> > You are right, this will be a general problem for me. Till now I never
> > had the race condition if the signal comes just before calling select(),
> > but I think this is only, because my application waits most of the time
> > in the select(). To be sure that my application work always, I have to
> > use one of the solutions you have written.
> >
> > I'm thinking of implementing a pselect() call. Where should this function
> > be located, in packages/compat/posix or in packages/io/fileio?
>
> It should go in the FILEIO package. The best approach would be to
> convert the current select() into pselect() and then add a new
> select() that just calls pselect() with a NULL sigmask argument. And
> pselect() takes a struct timespec rather than a struct timeval for the
> timeout, so that would have to be converted too. Here are the
> declarations for comparison:
>
> int pselect(int nfds, fd_set *readfds, fd_set *writefds,
>         fd_set *errorfds, const struct timespec *timeout,
>         const sigset_t *sigmask);
>
> int select(int nfds, fd_set *restrict readfds,
>         fd_set *restrict writefds, fd_set *restrict errorfds,
>         struct timeval *restrict timeout);
>
> As for where to set the new mask, that is a bit more complex. Simply
> calling pthread_sigmask() is not the right thing to do. And the code
> in select() would need some restructuring.
>
> Hmm, this is not looking as simple as it did when I started this
> message. I'll need to think about this a while. In the meantime you
> may want to investigate the socket option.

I have just tried it, but I have written a extra _pselect() function which 
take a timeval structure, so for the normal and more used select() the 
timeout don't have to be converted.

//==========================================================================
// Select API function

//roland
# define TIMESPEC_TO_TIMEVAL(tv, ts) {                                   \
        (tv)->tv_sec = (ts)->tv_sec;                                    \
        (tv)->tv_usec = (ts)->tv_nsec / 1000;                           \
}

//roland
extern sigset_t sig_pending;
extern Cyg_Mutex signal_mutex;

__externC int
_pselect(int nfd, fd_set *in, fd_set *out, fd_set *ex,
	struct timeval *tv, const sigset_t *sigmask)
{
    FILEIO_ENTRY();

    int error = ENOERR;
    int fd, mode, num;
    cyg_file *fp;
    fd_set in_res, out_res, ex_res;  // Result sets
    fd_set *selection[3], *result[3];
    cyg_tick_count ticks;
    int mode_type[] = {CYG_FREAD, CYG_FWRITE, 0};
    cyg_uint32 wake_count;

    //
    sigset_t oldsigmask;

    FD_ZERO(&in_res);
    FD_ZERO(&out_res);
    FD_ZERO(&ex_res);

    // Set up sets
    selection[0] = in;   result[0] = &in_res;
    selection[1] = out;  result[1] = &out_res;
    selection[2] = ex;   result[2] = &ex_res;

    // Compute end time
    if (tv)
        ticks = cyg_timeval_to_ticks( tv );
    else ticks = 0;

    // Lock the mutex
    select_mutex.lock();

    //roland
    pthread_info *self = pthread_self_info();
    oldsigmask=self->sigmask;

    // Scan sets for possible I/O until something found, timeout or error.
    while (!error)
    {
        wake_count = selwake_count;

        num = 0;  // Total file descriptors "ready"

        for (mode = 0;  !error && mode < 3;  mode++)
        {
            if (selection[mode]) {
                for (fd = 0;  !error && fd < nfd;  fd++)
                {
                    if (FD_ISSET(fd, selection[mode]))
                    {
                        fp = cyg_fp_get( fd );
                        if( fp == NULL )
                        {
                            error = EBADF;
                            break;
                        }

                        if ((*fp->f_ops->fo_select)(fp, mode_type[mode], 0))
                        {
                            FD_SET(fd, result[mode]);
                            num++;
                        }

                        cyg_fp_free( fp );
                    }
                }
            }
        }

        if (num)
        {
            // Found something, update user's sets
            if (in)  FD_COPY( &in_res, in );
            if (out) FD_COPY( &out_res, out );
            if (ex)  FD_COPY( &ex_res, ex );
            select_mutex.unlock();
            FILEIO_RETURN_VALUE(num);
        }

        Cyg_Scheduler::lock();

	//roland
	if (~*sigmask & (self->sigpending|sig_pending))
        {
                diag_printf("sigpending!!!!\n");
		if (sigmask)
	    	    self->sigmask = *sigmask;
                error = EINTR;
                break;
        }

        if( wake_count == selwake_count )
        {
            // Nothing found, see if we want to wait
            if (tv)
            {
                if (ticks == 0)
                {
                    // Special case of "poll"
                    select_mutex.unlock();
                    Cyg_Scheduler::unlock();
                    FILEIO_RETURN_VALUE(0);
                }

                ticks += Cyg_Clock::real_time_clock->current_value();

		//roland
		if (sigmask)
		    self->sigmask = *sigmask;

                if( !selwait.wait( ticks ) )
                {
	            // A non-standard wakeup, if the current time is equal to
                    // or past the timeout, return zero. Otherwise return
                    // EINTR, since we have been released.

                    if( Cyg_Clock::real_time_clock->current_value() >= ticks )
                    {

			//FIXME: maybe don't deliver signals here? There can't be one
			error = 0;
                    }
                    else
			error = EINTR;
		    break;
                }

		//roland
		if (sigmask)
		    self->sigmask = oldsigmask;

                ticks -= Cyg_Clock::real_time_clock->current_value();
            }
            else
            {
                // Wait forever (until something happens)

		//roland
		if (sigmask)
		    self->sigmask = *sigmask;

                if( !selwait.wait() )
		{
                    error = EINTR;
		    break;
		}

		//roland
		if (sigmask)
		    self->sigmask = oldsigmask;
            }
        }

        Cyg_Scheduler::unlock();

    } // while(!error)

    //roland
    signal_mutex.lock();
    Cyg_Scheduler::unlock();
    cyg_deliver_signals();
    if (sigmask)
        self->sigmask = oldsigmask;
    signal_mutex.unlock();

    select_mutex.unlock();

    FILEIO_RETURN(error);
}

__externC int
select(int nfd, fd_set *in, fd_set *out, fd_set *ex, struct timeval *tv)
{
	return _pselect(nfd,in,out,ex,tv,NULL);
}

__externC int
pselect(int nfd, fd_set *in, fd_set *out, fd_set *ex,
	const struct timespec *ts, const sigset_t *sigmask)
{
	struct timeval tv;

	if (ts != NULL)
		TIMESPEC_TO_TIMEVAL (&tv, ts);
	return _pselect(nfd,in,out,ex,&tv,sigmask);
}

//==========================================================================

In my first impression it seems to work.
The prototype of pthread_self_info() is in the private headerfile pprivate.h 
in compat/posix/current/src, which I can't access from select.cxx, so I have 
to move this somewhere else.
Furthermore I have to include some #ifdef if signals are not implemented.

What do you think about my advice?

Roland


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