Date: Sun, 13 Jul 2008 17:09:20 -0400 From: "Michael B Allen" <ioplex@gmail.com> To: "=?ISO-8859-1?Q?Mikko_Ty=F6l=E4j=E4rvi?=" <mbsd@pacbell.net> Cc: freebsd-hackers <freebsd-hackers@freebsd.org> Subject: Re: Pls sanity check my semtimedop(2) implementation Message-ID: <78c6bd860807131409v6e34478ameda1390b9eb2333f@mail.gmail.com> In-Reply-To: <Pine.OSX.4.64.0807131240250.594@mbook.home> References: <78c6bd860807130813p4c28f930g1f3094241aa1f1b1@mail.gmail.com> <Pine.OSX.4.64.0807131240250.594@mbook.home>
next in thread | previous in thread | raw e-mail | index | archive | help
On 7/13/08, Mikko Työläjärvi <mbsd@pacbell.net> wrote:
> On Sun, 13 Jul 2008, Michael B Allen wrote:
>
>
> > Hi,
> >
> > Below is a semtimedop(2) implementation that I'm using for FreeBSD. I
> > was hoping someone could look it over and tell me if they think the
> > implementation is sound.
> >
> > The code seems to work ok but when stressing the FreeBSD build of my app
> > I have managed to provoke errors related to concurrency (usually when a
> > SIGALRM goes off). The Linux build works flawlessesly so I'm wondering
> > about this one critical function that is different.
> >
>
> At the very least you need to check errno when semop() returns -1.
> Unless it is EINTR, you have other problems.
>
> Also, if there is any other code using the timer across this function
> call, you have race conditions between changing the signal handler and
> setting the timer. Even if there is no other use of the timer across
> this function, resetting the signal handler before disarming the timer
> leaves you open to the signal being handled by the default handler
> which will make the process exit.
Hi Mikko,
So if some other code uses setitimer(2) for whatever reason, then I
have the potential for a race. I'm not aware of any other such
instances of setitimer but my app is actually a plugin for a larger
application so I can't entirely rule out the possibility.
Is there any facility for creating a stateful timer so that I don't
run into this problem?
Can anyone provide the basis for an alternative implementation?
Should I use select(2) instead?
Mike
> > Is there any reason why I would want to use ITIMER_VIRTUAL /
> > SIGVTALRM instead of ITIMER_REAL / SIGALRM?
> >
> > Or perhaps I should be using a different implementation entirely?
> >
> > Mike
> >
> > int
> > _semtimedop(int semid,
> > struct sembuf *array,
> > size_t nops,
> > struct timespec *_timeout)
> > {
> > struct timeval timeout, before, after;
> > struct itimerval value, ovalue;
> > struct sigaction sa, osa;
> > int ret;
> >
> > if (_timeout) {
> > timeout.tv_sec = _timeout->tv_sec;
> > timeout.tv_usec = _timeout->tv_nsec / 1000;
> >
> > if (gettimeofday(&before, NULL) == -1) {
> > return -1;
> > }
> >
> > memset(&value, 0, sizeof value);
> > value.it_value = timeout;
> >
> > memset(&sa, 0, sizeof sa);
> > /* signal_print writes the signal info to a log file
> > */
> > sa.sa_sigaction = signal_print;
> > sa.sa_flags = SA_SIGINFO;
> > sigemptyset(&sa.sa_mask);
> > sigaction(SIGALRM, &sa, &osa);
> >
> > if (setitimer(ITIMER_REAL, &value, &ovalue) == -1) {
> > sigaction(SIGALRM, &osa, NULL);
> > return -1;
> > }
> > }
> >
> > ret = semop(semid, array, nops);
> >
> > if (_timeout) {
> > sigaction(SIGALRM, &osa, NULL);
> >
> > if (setitimer(ITIMER_REAL, &ovalue, NULL) == -1) {
> > return -1;
> > }
> > }
> >
> > if (ret == -1) {
> > if (_timeout) {
> > struct timeval elapsed;
> >
> > if (gettimeofday(&after, NULL) == -1) {
> > return -1;
> > }
> >
> > _timeval_diff(&after, &before, &elapsed);
> >
> > if (timercmp(&elapsed, &timeout, >=))
> > errno = EAGAIN;
> > }
> >
> > return -1;
> > }
> >
> > return 0;
> > }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?78c6bd860807131409v6e34478ameda1390b9eb2333f>