Bug 27035

Summary:	libc_r aborts pppctl(8)
Product:	Base System	Reporter:	Brian Somers <brian>
Component:	bin	Assignee:	Daniel Eischen <deischen>
Status:	Closed FIXED
Severity:	Affects Only Me	CC:	deischen
Priority:	Normal
Version:	5.0-CURRENT
Hardware:	Any
OS:	Any

Description Brian Somers 2001-05-02 18:20:00 UTC

	pppctl aborts with

	Fatal error '_waitq_remove: Not in queue' at line 317 in file /usr/src/lib/libc_r/uthread/uthread_priority_queue.c (errno = 35)

Fix: 

No idea, I know little about the pthreads implementation.  It's
	always possible to use telnet instead of pppctl though, so this
	problem can't really be described as critical...
How-To-Repeat: 	Start up two windows.

	Window 1:
		# ppp
		Working in interactive mode
		Using interface: tun0
		# ppp ON machine> set server /tmp/s ""

	Window 2:
		# pppctl /tmp/s
		# ppp ON machine> set log local command

	Window 1:
		# ppp ON machine> show who
		.......

	You may need to do the ``show who'' more than once - on one occasion
	here I needed to do it twice.

	Window 2 eventually shows:
		ppp ON machine> Command: /dev/tty: show who
		Fatal error '_waitq_remove: Not in queue' at line 317 in file /usr/src/lib/libc_r/uthread/uthread_priority_queue.c (errno = 35)
		Abort trap (core dumped)
		#

Comment 1 Brian Somers freebsd_committer

2001-05-02 18:22:06 UTC

Responsible Changed
From-To: freebsd-bugs->deischen

I think Daniel looks after the pthreads stuff....?

Comment 2 eischen 2001-05-03 04:22:59 UTC

You want to try this fix?

-- 
Dan Eischen


Index: uthread_kern.c
===================================================================
RCS file: /opt/b/CVS/src/lib/libc_r/uthread/uthread_kern.c,v
retrieving revision 1.37
diff -u -r1.37 uthread_kern.c
--- uthread_kern.c	2001/01/24 13:03:34	1.37
+++ uthread_kern.c	2001/05/03 03:17:03
@@ -206,12 +206,6 @@
 			PANIC("Unable to restore alternate signal stack");
 	}
 
-	/* Are there pending signals for this thread? */
-	if (curthread->check_pending != 0) {
-		curthread->check_pending = 0;
-		_thread_sig_check_pending(curthread);
-	}
-
 	/*
 	 * Enter a scheduling loop that finds the next thread that is
 	 * ready to run. This loop completes when there are no more threads
@@ -335,6 +329,12 @@
 				PTHREAD_WORKQ_INSERT(curthread);
 				break;
 			}
+		}
+
+		/* Are there pending signals for this thread? */
+		if (curthread->check_pending != 0) {
+			curthread->check_pending = 0;
+			_thread_sig_check_pending(curthread);
 		}
 
 		/*

Comment 3 Brian Somers 2001-05-03 11:11:50 UTC

Hi,

Yes, this gets it past the problem.  However, with the same setup, 
typing ``quit'' at the original ppp command prompt makes pppctl drop 
a core.  Also, once pppctl's Receive() thread has read data, it doesn't 
seem to accept keyboard input any more.

> You want to try this fix?
> 
> -- 
> Dan Eischen
> 
> 
> Index: uthread_kern.c
> ===================================================================
> RCS file: /opt/b/CVS/src/lib/libc_r/uthread/uthread_kern.c,v
> retrieving revision 1.37
> diff -u -r1.37 uthread_kern.c
> --- uthread_kern.c	2001/01/24 13:03:34	1.37
> +++ uthread_kern.c	2001/05/03 03:17:03
> @@ -206,12 +206,6 @@
>  			PANIC("Unable to restore alternate signal stack");
>  	}
>  
> -	/* Are there pending signals for this thread? */
> -	if (curthread->check_pending != 0) {
> -		curthread->check_pending = 0;
> -		_thread_sig_check_pending(curthread);
> -	}
> -
>  	/*
>  	 * Enter a scheduling loop that finds the next thread that is
>  	 * ready to run. This loop completes when there are no more threads
> @@ -335,6 +329,12 @@
>  				PTHREAD_WORKQ_INSERT(curthread);
>  				break;
>  			}
> +		}
> +
> +		/* Are there pending signals for this thread? */
> +		if (curthread->check_pending != 0) {
> +			curthread->check_pending = 0;
> +			_thread_sig_check_pending(curthread);
>  		}
>  
>  		/*
> 

-- 
Brian <brian@Awfulhak.org>                        <brian@[uk.]FreeBSD.org>
      <http://www.Awfulhak.org>                   <brian@[uk.]OpenBSD.org>
Don't _EVER_ lose your sense of humour !

Comment 4 eischen 2001-05-03 21:26:01 UTC

On Thu, 3 May 2001, Brian Somers wrote:
> Hi,
> 
> Yes, this gets it past the problem.  However, with the same setup, 
> typing ``quit'' at the original ppp command prompt makes pppctl drop 
> a core.  Also, once pppctl's Receive() thread has read data, it doesn't 
> seem to accept keyboard input any more.

Hmm, I tried a similar patch to 4.3-stable and can't reproduce this
new problem.  If you can reproduce this under -stable also, let me know.
I'll try -current later tonight.

-- 
Dan Eischen

Comment 5 eischen 2001-05-04 15:45:14 UTC

This patch seems to fix both the original problem and the followup
problem.  Please try it and see if there are any other bogosities
that you can find.

Thanks!

Index: uthread_kern.c
===================================================================
RCS file: /opt/b/CVS/src/lib/libc_r/uthread/uthread_kern.c,v
retrieving revision 1.37
diff -u -r1.37 uthread_kern.c
--- uthread_kern.c	2001/01/24 13:03:34	1.37
+++ uthread_kern.c	2001/05/04 14:28:07
@@ -206,12 +206,6 @@
 			PANIC("Unable to restore alternate signal stack");
 	}
 
-	/* Are there pending signals for this thread? */
-	if (curthread->check_pending != 0) {
-		curthread->check_pending = 0;
-		_thread_sig_check_pending(curthread);
-	}
-
 	/*
 	 * Enter a scheduling loop that finds the next thread that is
 	 * ready to run. This loop completes when there are no more threads
@@ -334,6 +328,21 @@
 				/* Insert into the work queue: */
 				PTHREAD_WORKQ_INSERT(curthread);
 				break;
+			}
+
+			/*
+			 * Are there pending signals for this thread?
+			 *
+			 * This check has to be performed after the thread
+			 * has been placed in the queue(s) appropriate for
+			 * its state.  The process of adding pending signals
+			 * can change a threads state, which in turn will
+			 * attempt to add or remove the thread from any
+			 * scheduling queue to which it belongs.
+			 */
+			if (curthread->check_pending != 0) {
+				curthread->check_pending = 0;
+				_thread_sig_check_pending(curthread);
 			}
 		}
 
Index: uthread_sig.c
===================================================================
RCS file: /opt/b/CVS/src/lib/libc_r/uthread/uthread_sig.c,v
retrieving revision 1.35
diff -u -r1.35 uthread_sig.c
--- uthread_sig.c	2001/03/09 16:05:43	1.35
+++ uthread_sig.c	2001/05/04 14:27:56
@@ -575,6 +575,9 @@
 
 	restart = _thread_sigact[sig - 1].sa_flags & SA_RESTART;
 
+	/* Make sure this signal isn't still in the pending set: */
+	sigdelset(&pthread->sigpend, sig);
+
 	/*
 	 * Process according to thread state:
 	 */

Comment 6 Brian Somers 2001-05-04 16:47:38 UTC

Yes - this seems to make things work again.

Thanks.

> This patch seems to fix both the original problem and the followup
> problem.  Please try it and see if there are any other bogosities
> that you can find.
> 
> Thanks!
> 
> Index: uthread_kern.c
> ===================================================================
> RCS file: /opt/b/CVS/src/lib/libc_r/uthread/uthread_kern.c,v
> retrieving revision 1.37
> diff -u -r1.37 uthread_kern.c
> --- uthread_kern.c	2001/01/24 13:03:34	1.37
> +++ uthread_kern.c	2001/05/04 14:28:07
> @@ -206,12 +206,6 @@
>  			PANIC("Unable to restore alternate signal stack");
>  	}
>  
> -	/* Are there pending signals for this thread? */
> -	if (curthread->check_pending != 0) {
> -		curthread->check_pending = 0;
> -		_thread_sig_check_pending(curthread);
> -	}
> -
>  	/*
>  	 * Enter a scheduling loop that finds the next thread that is
>  	 * ready to run. This loop completes when there are no more threads
> @@ -334,6 +328,21 @@
>  				/* Insert into the work queue: */
>  				PTHREAD_WORKQ_INSERT(curthread);
>  				break;
> +			}
> +
> +			/*
> +			 * Are there pending signals for this thread?
> +			 *
> +			 * This check has to be performed after the thread
> +			 * has been placed in the queue(s) appropriate for
> +			 * its state.  The process of adding pending signals
> +			 * can change a threads state, which in turn will
> +			 * attempt to add or remove the thread from any
> +			 * scheduling queue to which it belongs.
> +			 */
> +			if (curthread->check_pending != 0) {
> +				curthread->check_pending = 0;
> +				_thread_sig_check_pending(curthread);
>  			}
>  		}
>  
> Index: uthread_sig.c
> ===================================================================
> RCS file: /opt/b/CVS/src/lib/libc_r/uthread/uthread_sig.c,v
> retrieving revision 1.35
> diff -u -r1.35 uthread_sig.c
> --- uthread_sig.c	2001/03/09 16:05:43	1.35
> +++ uthread_sig.c	2001/05/04 14:27:56
> @@ -575,6 +575,9 @@
>  
>  	restart = _thread_sigact[sig - 1].sa_flags & SA_RESTART;
>  
> +	/* Make sure this signal isn't still in the pending set: */
> +	sigdelset(&pthread->sigpend, sig);
> +
>  	/*
>  	 * Process according to thread state:
>  	 */
> 

-- 
Brian <brian@Awfulhak.org>                        <brian@[uk.]FreeBSD.org>
      <http://www.Awfulhak.org>                   <brian@[uk.]OpenBSD.org>
Don't _EVER_ lose your sense of humour !

Comment 7 Daniel Eischen freebsd_committer

2001-05-04 21:37:55 UTC

State Changed
From-To: open->closed

Fix just committed.  Thanks for the test cases Brian :-)