Re: pthread_{suspend,resume}_np broken?
In article [EMAIL PROTECTED], Daniel M. Eischen [EMAIL PROTECTED] wrote: Here's a quick fix. Thanks! It seems to fix my test program. I'll leave it up to you and Jordan whether to commit this before 4.0. It would be nice to have, but I don't have any immediate need for it. John -- John Polstra [EMAIL PROTECTED] John D. Polstra Co., Inc.Seattle, Washington USA "Disappointment is a good sign of basic intelligence." -- Chögyam Trungpa To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: pthread_{suspend,resume}_np broken?
On Tue, 29 Feb 2000, John Polstra wrote: In article [EMAIL PROTECTED], Daniel Eischen [EMAIL PROTECTED] wrote: On Tue, 29 Feb 2000, John Polstra wrote: Shouldn't the test against PS_SUSPENDED be "==" instead of "!="? Yes, it should be "==" instead of "!=". Go ahead and fix it if you want :-) Thanks. I'll ask Jordan if I may commit the fix. What about the other part of my question? I still don't understand why in my test program pthread_suspend_np() isn't suspending the thread. I think it's a separate bug from the pthread_resume_np() bug. Sorry, it was very late here and I missed that part. I see the problem. pthread_suspend_np is broke in that it only will work if the thread is running (PS_RUNNING). Your program is always trying to suspend a thread that is sleeping (PS_SLEEP_WAIT) so changing the state with PTHREAD_NEW_STATE fails. The fix isn't as easy as just correctly setting the threads state. Potentially, the suspended thread could be waiting on a mutex or condition variable and be in another queue. The correct fix is probably to save the threads old state and then set the threads state to PS_SUSPENDED. Resuming should restore the saved thread state. I'll see if I can come up with a correct fix for this. Dan Eischen [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: pthread_{suspend,resume}_np broken?
Daniel Eischen wrote: On Tue, 29 Feb 2000, John Polstra wrote: In article [EMAIL PROTECTED], Daniel Eischen [EMAIL PROTECTED] wrote: On Tue, 29 Feb 2000, John Polstra wrote: Shouldn't the test against PS_SUSPENDED be "==" instead of "!="? Yes, it should be "==" instead of "!=". Go ahead and fix it if you want :-) Thanks. I'll ask Jordan if I may commit the fix. What about the other part of my question? I still don't understand why in my test program pthread_suspend_np() isn't suspending the thread. I think it's a separate bug from the pthread_resume_np() bug. Sorry, it was very late here and I missed that part. I see the problem. pthread_suspend_np is broke in that it only will work if the thread is running (PS_RUNNING). Your program is always trying to suspend a thread that is sleeping (PS_SLEEP_WAIT) so changing the state with PTHREAD_NEW_STATE fails. The fix isn't as easy as just correctly setting the threads state. Potentially, the suspended thread could be waiting on a mutex or condition variable and be in another queue. The correct fix is probably to save the threads old state and then set the threads state to PS_SUSPENDED. Resuming should restore the saved thread state. I'll see if I can come up with a correct fix for this. Here's a quick fix. It also includes a simple fix for pthread_kill that src/libc_r/uthread/test/sigwait/sigwait.c will expose. I haven't run any other regression tests. I'll do that when I get some more time. Jason, can you also take a look at these changes and run some tests on them? Thanks, Dan Eischen [EMAIL PROTECTED] Index: pthread_private.h === RCS file: /opt/b/CVS/src/lib/libc_r/uthread/pthread_private.h,v retrieving revision 1.36 diff -c -r1.36 pthread_private.h *** pthread_private.h 2000/01/20 21:53:58 1.36 --- pthread_private.h 2000/03/01 12:49:27 *** *** 576,581 --- 576,583 #define PTHREAD_CANCEL_NEEDED 0x0010 int cancelflags; + int suspended; + thread_continuation_t continuation; /* Index: uthread_cancel.c === RCS file: /opt/b/CVS/src/lib/libc_r/uthread/uthread_cancel.c,v retrieving revision 1.3 diff -c -r1.3 uthread_cancel.c *** uthread_cancel.c2000/01/19 07:04:45 1.3 --- uthread_cancel.c2000/03/01 13:44:57 *** *** 37,42 --- 37,51 pthread-cancelflags |= PTHREAD_CANCELLING; break; + case PS_SUSPENDED: + /* +* This thread isn't in any scheduling +* queues; just change it's state: +*/ + pthread-cancelflags |= PTHREAD_CANCELLING; + PTHREAD_SET_STATE(pthread, PS_RUNNING); + break; + case PS_SPINBLOCK: case PS_FDR_WAIT: case PS_FDW_WAIT: *** *** 52,58 case PS_WAIT_WAIT: case PS_SIGSUSPEND: case PS_SIGWAIT: - case PS_SUSPENDED: /* Interrupt and resume: */ pthread-interrupted = 1; pthread-cancelflags |= PTHREAD_CANCELLING; --- 61,66 Index: uthread_cond.c === RCS file: /opt/b/CVS/src/lib/libc_r/uthread/uthread_cond.c,v retrieving revision 1.22 diff -c -r1.22 uthread_cond.c *** uthread_cond.c 2000/01/27 23:06:59 1.22 --- uthread_cond.c 2000/03/01 13:03:46 *** *** 282,289 break; } ! if (interrupted != 0 _thread_run-continuation != NULL) ! _thread_run-continuation((void *) _thread_run); _thread_leave_cancellation_point(); } --- 282,292 break; } ! if (interrupted != 0) { ! if (_thread_run-continuation != NULL) ! _thread_run-continuation((void *) _thread_run); ! rval = EINTR; ! } _thread_leave_cancellation_point(); } *** *** 449,456 break; } ! if (interrupted != 0 _thread_run-continuation != NULL) ! _thread_run-continuation((void *) _thread_run); _thread_leave_cancellation_point(); } --- 452,462 break; } ! if (interrupted !=
Re: pthread_{suspend,resume}_np broken?
On Wed, Mar 01, 2000 at 09:30:43AM -0500, Daniel M. Eischen wrote: I haven't run any other regression tests. I'll do that when I get some more time. Jason, can you also take a look at these changes and run some tests on them? I just moved a couple of days ago and all my test boxes are still in boxes. It will be at least a couple of days before I can test this properly. Jason To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: pthread_{suspend,resume}_np broken?
On Tue, 29 Feb 2000, John Polstra wrote: Either pthread_suspend_np() and pthread_resume_np() are broken in -current or I don't understand them. The attached program (cc -pthread suspend.c) starts two background threads. Each thread loops outputting a character ('1' or '2' according to which thread it is) and then sleeping for a second. Meanwhile, the main thread reads keypresses from the standard input. On each keypress it toggles background thread 1 between suspended and resumed. [...] Shouldn't the test against PS_SUSPENDED be "==" instead of "!="? I would think we'd want to do something if the thread was suspended, and skip it if the thread wasn't suspended -- exactly the opposite of what the current code does. Yes, it should be "==" instead of "!=". Go ahead and fix it if you want :-) Dan Eischen [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: pthread_{suspend,resume}_np broken?
In article [EMAIL PROTECTED], Daniel Eischen [EMAIL PROTECTED] wrote: On Tue, 29 Feb 2000, John Polstra wrote: Shouldn't the test against PS_SUSPENDED be "==" instead of "!="? Yes, it should be "==" instead of "!=". Go ahead and fix it if you want :-) Thanks. I'll ask Jordan if I may commit the fix. What about the other part of my question? I still don't understand why in my test program pthread_suspend_np() isn't suspending the thread. I think it's a separate bug from the pthread_resume_np() bug. John -- John Polstra [EMAIL PROTECTED] John D. Polstra Co., Inc.Seattle, Washington USA "Disappointment is a good sign of basic intelligence." -- Chögyam Trungpa To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message