Wow Gary, this is much better ( no thanks to my comments ;-) )
Nearly there now, please bear with us. I'll try again to be constructive
(but you may need to wait for David to come back online to confirm my
comments below :-) )
1) 'first' also needs to be volatile since it is set in one thread and
accessed in another (outside of a sync block, or without any barrier).
2) I wouldn't just swallow the InterruptedException. jtreg uses thread
interrupts to kill/interrupt a test that has used more than it's
allotted time. I think it best to just throw them up the stack where
possible, and wrap them in an Error or RuntimeException in run(). But
either way allow the thread to be interruptible and complete execution.
Thanks again for taking this issue,
-Chris.
On 11/ 8/11 01:12 PM, Gary Adams wrote:
Let's see if this captures all the comments and
is a bit more robust in the face of the original
race conditions mentioned.
- threads are started before they are recorded in local variables
- second is now volatile
- stop thread group is triggered by first thread once second thread is
started
- main thread checks for second thread death after thread group is stopped
- left in the sleep delays to yield to other threads
- using wait/notify for handshake with main thread
- using try/finally to ensure main thread does not check too soon
after thread group stop
31 public class Stop implements Runnable {
32 private static Thread first=null;
33 private static volatile Thread second=null;
34 private static ThreadGroup group = new ThreadGroup("");
35 private static boolean groupStopped =false ;
36 private static final Object lock = new Object();
37
38 Stop() {
39 Thread thread = new Thread(group, this);
40 thread.start();
41 // Record the threads after they have been started
42 if (first == null)
43 first = thread;
44 else
45 second = thread;
46 }
47
48 public void run() {
49 while (true) {
50 try {
51 // Give the other thread a chance to start
52 Thread.sleep(1000);
53 } catch(InterruptedException e) {
54 }
55
56 // When the first thread sees the second thread has been started
57 // stop the thread group.
58 if ((Thread.currentThread() == first)
59 && (second != null)) {
60 synchronized (lock) {
61 try {
62 group.stop();
63 } finally {
64 // Signal the main thread it is time to check
65 // that the stopped thread group was successful
66 groupStopped = true;
67 lock.notifyAll();
68 }
69 }
70 }
71 }
72 }
73
74 public static void main(String[] args) throws Exception {
75 // Launch two threads as part of the same thread group
76 for (int i = 0; i < 2; i++)
77 new Stop();
78
79 // Wait for the thread group to be issued
80 synchronized(lock){
81 while (!groupStopped){
82 lock.wait();
83 try {
84 // Give the other thread a chance to stop
85 Thread.sleep(1000);
86 } catch(InterruptedException e) {
87 }
88 }
89 }
90
91 // Check that the second thread is terminated when the
92 // first thread terminates the thread group.
93 boolean failed = second.isAlive();
94
95 // Clean up any threads that may have not been terminated
96 first.stop();
97 second.stop();
98 if (failed)
99 throw new RuntimeException("Failure.");
100 }
101 }
