Race condition in TimerTask KillThread test

[David Holmes]

Hi Gary,

On 10/11/2011 5:36 AM, Gary Adams wrote:
> Here's a revised diff for the KillThread timing problem :
> - added current CR# to @bug tag

I don't think that is correct. AFAIK the @bug indicates what bug this 
test is testing the fix for, not which bugs modified the test. (Ditto 
for your other test fixes).

Alan: can you confirm? Thanks.

> public class KillThread {
> + static Thread tdThread;

Must be volatile as its value is being communicated from the timer 
thread to the main thread.

> public static void main (String[] args) throws Exception {
> + tdThread = null;

Not necessary - static field is null by default.

> Timer t = new Timer();
>
> // Start a mean event that kills the timer thread
> t.schedule(new TimerTask() {
>   public void run() {
> +   tdThread = Thread.currentThread();
>    throw new ThreadDeath();
>   }
> }, 0);
>
> // Wait for mean event to do the deed and thread to die.
> try {
> + do {
> Thread.sleep(100);
> + } while(tdThread == null);
> } catch(InterruptedException e) {
> }

Minor nit: You didn't introduce this, but catching IE is unnecessary as 
main can let it propagate. With the updated code in the unlikely event 
it occurred you might then go and invoke join() on a null reference.

Aside: it would be good if whomever is going to do the commits for you 
could assist with publishing webrevs for these changes.

Thanks,
David
-----

> + tdThread.join();
>
> // Try to start another event
> try {
>
>
> On 11/ 6/11 08:46 PM, David Holmes wrote:
>> On 5/11/2011 8:37 AM, Alan Bateman wrote:
>>> On 04/11/2011 15:52, Gary Adams wrote:
>>>> :
>>>>
>>>> I'll attempt a simple fix for the KillThread case by replacing :
>>>>
>>>> Thread.sleep(100);
>>>>
>>>> with a simple loop
>>>>
>>>> do {
>>>> Thread.sleep(100);
>>>> } while (waiting);
>>>>
>>>> where the TimerTask runnable will clear the flag
>>>> with "waiting = false " once it has been launched.
>>>>
>>> I don't think this will guarantee that the timer thread will have
>>> terminated before the main thread schedules the second event. I think
>>> this test would be robust if you capture a reference to the timer thread
>>> in the first task and have the main thread wait or poll until it has a
>>> reference to the thread. Once it has a reference to the thread then it
>>> can wait for it to terminate before attempting to schedule the second
>>> task.
>>
>> Agreed. As soon as waiting is set we can switch back to the main
>> thread and proceed to the next phase of the test - but the timer
>> thread is still alive at that point. If the issue is checking for
>> thread termination then we must track that directly.
>>
>> David
>> -----
>>
>>> -Alan.
>