On 7/13/08, Mikko Työläjärvi <[EMAIL PROTECTED]> 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; > > } _______________________________________________ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "[EMAIL PROTECTED]"