Subject: Re: Defered signal handling in SA libpthread
To: None <tech-userlevel@netbsd.org>
From: Bill Stouder-Studenmund <wrstuden@netbsd.org>
List: tech-userlevel
Date: 09/08/2007 23:33:09
--reI/iBAAp9kzkmX4
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sat, Sep 08, 2007 at 09:45:33PM -0700, Bill Stouder-Studenmund wrote:
> I have coded most of the change to detect deferred signals, and I'd=20
> appreciate comments. I'm attaching it below.

It needs a minor change. See below.

> Index: pthread_sig.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> RCS file: /cvsroot/src/lib/libpthread/Attic/pthread_sig.c,v
> retrieving revision 1.47
> diff -u -p -r1.47 pthread_sig.c
> --- pthread_sig.c	24 Nov 2006 19:46:58 -0000	1.47
> +++ pthread_sig.c	9 Sep 2007 04:37:07 -0000
> @@ -266,6 +266,9 @@ __sigsuspend14(const sigset_t *sigmask)
>  		pthread_spinunlock(self, &pt_sigsuspended_lock);
>  		pthread_exit(PTHREAD_CANCELED);
>  	}
> +	if (pthread_check_defsig(self)) {
> +		/* WRS Hmmm... how to handle... */
> +	}
>  	pthread_sigmask(SIG_SETMASK, sigmask, &oldmask);
> =20
>  	self->pt_state =3D PT_STATE_BLOCKED_QUEUE;
> @@ -399,6 +402,9 @@ sigtimedwait(const sigset_t * __restrict
> =20
>   block:
>  		pthread_spinlock(self, &self->pt_statelock);
> +		if (pthread_check_defsig(self)) {
> +			/* WRS Hmmm... how to handle... */
> +		}
>  		self->pt_state =3D PT_STATE_BLOCKED_QUEUE;
>  		self->pt_sleepobj =3D &pt_sigwaiting_cond;
>  		self->pt_sleepq =3D &pt_sigwaiting;
> @@ -842,6 +848,9 @@ pthread__kill(pthread_t self, pthread_t=20
>  		 * make the target runnable but do not deliver the signal.
>  		 * Otherwise record the signal for later delivery.
>  		 * XXX not MPsafe.
> +		 * WRS - how is it not MP safe?
> +		 * Hmm... We probably should hold the target's pt_statelock,
> +		 * not our own.
>  		 */
>  		pthread_spinlock(self, &self->pt_statelock);
>  		if (target->pt_state =3D=3D PT_STATE_BLOCKED_QUEUE &&
> @@ -892,8 +901,11 @@ pthread__kill(pthread_t self, pthread_t=20
>  		 * from its torpor, then mark the signal for
>  		 * later processing.
>  		 */
> +		pthread_spinlock(self, &target->pt_siglock);
>  		__sigaddset14(&target->pt_sigblocked, si->si_signo);
>  		__sigaddset14(&target->pt_sigmask, si->si_signo);
> +		pthread_spinunlock(self, &target->pt_siglock);
> +

Don't add those. We already hold the spinlock, so this will cause us to=20
spin & deadlock. :-(

>  		pthread_spinlock(self, &target->pt_flaglock);
>  		target->pt_flags |=3D PT_FLAG_SIGDEFERRED;
>  		pthread_spinunlock(self, &target->pt_flaglock);
> @@ -1021,7 +1033,9 @@ pthread__signal_deferred(pthread_t self,
>  		si.si_signo =3D i;
>  		pthread__deliver_signal(self, t, &si);
>  	}
> +	pthread_spinlock(self, &t->pt_flaglock);
>  	t->pt_flags &=3D ~PT_FLAG_SIGDEFERRED;
> +	pthread_spinunlock(self, &t->pt_flaglock);
> =20
>  	pthread_spinunlock(self, &t->pt_siglock);
>  }

Take care,

Bill

--reI/iBAAp9kzkmX4
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (NetBSD)

iD8DBQFG45OlWz+3JHUci9cRAvVPAJ9tkWbGQxtJOXhgZttN+3Lj3O5J/gCbB43t
Bh9jaMYyzZO+q/W72znNuV0=
=QCt+
-----END PGP SIGNATURE-----

--reI/iBAAp9kzkmX4--