You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ant.apache.org by st...@apache.org on 2007/11/01 14:12:49 UTC

svn commit: r590990 - /ant/core/trunk/src/main/org/apache/tools/ant/util/WorkerAnt.java

Author: stevel
Date: Thu Nov  1 06:12:49 2007
New Revision: 590990

URL: http://svn.apache.org/viewvc?rev=590990&view=rev
Log:
read the javadocs on wait(long) to see why this change was made

Modified:
    ant/core/trunk/src/main/org/apache/tools/ant/util/WorkerAnt.java

Modified: ant/core/trunk/src/main/org/apache/tools/ant/util/WorkerAnt.java
URL: http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/util/WorkerAnt.java?rev=590990&r1=590989&r2=590990&view=diff
==============================================================================
--- ant/core/trunk/src/main/org/apache/tools/ant/util/WorkerAnt.java (original)
+++ ant/core/trunk/src/main/org/apache/tools/ant/util/WorkerAnt.java Thu Nov  1 06:12:49 2007
@@ -114,10 +114,9 @@
      */
     public void waitUntilFinished(long timeout) throws InterruptedException {
         synchronized(notify) {
-            if(finished) {
-                return;
+            while (!finished) {
+                notify.wait(timeout);
             }
-            notify.wait(timeout);
         }
     }
 



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: svn commit: r590990 - /ant/core/trunk/src/main/org/apache/tools/ant/util/WorkerAnt.java

Posted by Steve Loughran <st...@apache.org>.
Martijn Kruithof wrote:
> 
>>      public void waitUntilFinished(long timeout) throws 
>> InterruptedException {
>>          synchronized(notify) {
>> -            if(finished) {
>> -                return;
>> +            while (!finished) {
>> +                notify.wait(timeout);
>>              }
>> -            notify.wait(timeout);
>>          }
>>      }
>>   
> I hope you noticed that this is not an equivalent change, not just 
> starting to accepting "false" interrupts, but also to ignore the timeout.
> 
> to do it correctly probably it should be using something using 
> currentTimeMillis as well.

yes, I rolled it back.

the issue is, if you look at the internals of wait(), the concept of 
spurious wakeups, in which threads can be woken up early. They said put 
a while() round the test, but of course, that doesnt work in this case, 
as you need to extract the start time and keep going until the total 
test time is done. Which is just too painful to do right now. Unless I 
see evidence that spontaneous wakeups are real, I'm going to leave this 
bit alone.

-steve


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: svn commit: r590990 - /ant/core/trunk/src/main/org/apache/tools/ant/util/WorkerAnt.java

Posted by Martijn Kruithof <jk...@apache.org>.
>      public void waitUntilFinished(long timeout) throws InterruptedException {
>          synchronized(notify) {
> -            if(finished) {
> -                return;
> +            while (!finished) {
> +                notify.wait(timeout);
>              }
> -            notify.wait(timeout);
>          }
>      }
>   
I hope you noticed that this is not an equivalent change, not just 
starting to accepting "false" interrupts, but also to ignore the timeout.

to do it correctly probably it should be using something using 
currentTimeMillis as well.


Martijn

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org