From owner-freebsd-hackers@FreeBSD.ORG Sun Jul 13 21:09:24 2008 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 7B8031065676 for ; Sun, 13 Jul 2008 21:09:24 +0000 (UTC) (envelope-from ioplex@gmail.com) Received: from nf-out-0910.google.com (nf-out-0910.google.com [64.233.182.185]) by mx1.freebsd.org (Postfix) with ESMTP id C75368FC1C for ; Sun, 13 Jul 2008 21:09:23 +0000 (UTC) (envelope-from ioplex@gmail.com) Received: by nf-out-0910.google.com with SMTP id h3so1669686nfh.33 for ; Sun, 13 Jul 2008 14:09:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from:to :subject:cc:in-reply-to:mime-version:content-type :content-transfer-encoding:content-disposition:references; bh=g3IN1yIeGhjoLszUDpHn9j3O/bkahMaKMtEj9fHAXNk=; b=dBg0ZdBTJUlEuaGK64Fk4TAMZpfOfRfCv/HZ4yb8rxzbDleGpDTKc/gLRI/51a1q+G 9z2KMG0YMop0pFflgEdLpG9aCnghGV3GitPIdbmqZJaLUXY5VWUr5u8L4boSj3l00d6G +6iED/HgQ8anm8kP0LDmqgb2//sm2+U5kOwW0= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:to:subject:cc:in-reply-to:mime-version :content-type:content-transfer-encoding:content-disposition :references; b=w6kCQchBikBylBkjPTd36cj+jRLEqpQ1wSZvXZIb1QwJt9YruIeN+MxbPmRE6FIywy 7+e3nnp/4G7S8AAlDgfrt8JZYokHiAPlYFWlYj6ueIVBkE9YiGugthK9u7VgaHFzYeG6 A3e5ygTObaeSZsGWsQxBfYnNkqrHldxcMQBGk= Received: by 10.210.34.2 with SMTP id h2mr8386826ebh.110.1215983360203; Sun, 13 Jul 2008 14:09:20 -0700 (PDT) Received: by 10.210.139.1 with HTTP; Sun, 13 Jul 2008 14:09:20 -0700 (PDT) Message-ID: <78c6bd860807131409v6e34478ameda1390b9eb2333f@mail.gmail.com> Date: Sun, 13 Jul 2008 17:09:20 -0400 From: "Michael B Allen" To: "=?ISO-8859-1?Q?Mikko_Ty=F6l=E4j=E4rvi?=" In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline References: <78c6bd860807130813p4c28f930g1f3094241aa1f1b1@mail.gmail.com> Cc: freebsd-hackers Subject: Re: Pls sanity check my semtimedop(2) implementation X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 13 Jul 2008 21:09:24 -0000 On 7/13/08, Mikko Ty=F6l=E4j=E4rvi 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 ap= p > > 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 =3D _timeout->tv_sec; > > timeout.tv_usec =3D _timeout->tv_nsec / 1000; > > > > if (gettimeofday(&before, NULL) =3D=3D -1) { > > return -1; > > } > > > > memset(&value, 0, sizeof value); > > value.it_value =3D timeout; > > > > memset(&sa, 0, sizeof sa); > > /* signal_print writes the signal info to a log file > > */ > > sa.sa_sigaction =3D signal_print; > > sa.sa_flags =3D SA_SIGINFO; > > sigemptyset(&sa.sa_mask); > > sigaction(SIGALRM, &sa, &osa); > > > > if (setitimer(ITIMER_REAL, &value, &ovalue) =3D=3D -1) { > > sigaction(SIGALRM, &osa, NULL); > > return -1; > > } > > } > > > > ret =3D semop(semid, array, nops); > > > > if (_timeout) { > > sigaction(SIGALRM, &osa, NULL); > > > > if (setitimer(ITIMER_REAL, &ovalue, NULL) =3D=3D -1) { > > return -1; > > } > > } > > > > if (ret =3D=3D -1) { > > if (_timeout) { > > struct timeval elapsed; > > > > if (gettimeofday(&after, NULL) =3D=3D -1) { > > return -1; > > } > > > > _timeval_diff(&after, &before, &elapsed); > > > > if (timercmp(&elapsed, &timeout, >=3D)) > > errno =3D EAGAIN; > > } > > > > return -1; > > } > > > > return 0; > > }