Integer overflow in _Jv_CondWait

[Boehm, Hans]

Note that as of Java 5, it is always acceptable for Object.wait() to
return early.  Hence the original behavior was a severe performance bug,
but technically correct, unless we use _Jv_CondWait with different
assumptions internally.  (Apparently a lot of pre-1.5 implementations
also exhibit this behavior.  But the bugs were fixed by a spec change in
1.5.  And I think the 1.5 spec is probably the correct one.)

By the same token, it would probably be slightly preferable to turn an
overflow into the longest possible finite wait.  But as you point out,
it's extremely unlikely this will ever matter.

Hans

> -----Original Message-----
> From: java-patches-owner@gcc.gnu.org 
> [mailto:java-patches-owner@gcc.gnu.org] On Behalf Of Andrew Haley
> Sent: Wednesday, November 23, 2005 7:33 AM
> To: java-patches@gcc.gnu.org
> Cc: overholt@redhat.com
> Subject: Integer overflow in _Jv_CondWait
> 
> 
> _Jv_CondWait makes no allowances for the possibility of an 
> integer overflow, and this means we can return too early.
> 
> Consider this program:
> 
> 
> public class TimedWait
> {
>   public static void main (String[] argv)
>     throws InterruptedException
>   {
>     Object o = new Object();
> 
>     synchronized (o)
>       {
>         o.wait(Long.MAX_VALUE);
>       }
>   }
> }
> 
> 
> It's obvious that we never expect this program to terminate, 
> because the delay is some 292 million years.  However, try 
> this on gcj and it returns immediately -- because 
> _Jv_CondWait is broken.
> 
> We must be much more careful how we convert time from one 
> representation to another.
> 
> Also, there is the question of what we should do if the 
> required timeout won't fit into a struct timespec.  In this 
> patch I assume that when this happens the user actually 
> doesn't want a timeout at all.
> 
> Andrew.
> 
> 
> 
> 2005-11-23  Andrew Haley  <aph@redhat.com>
> 
> 	* posix-threads.cc (_Jv_CondWait): Rewrite calculation of the
> 	struct timespec we pass to pthread_cond_timedwait.
> 
> *** posix-threads.cc~	2005-11-09 09:48:52.000000000 +0000
> --- posix-threads.cc	2005-11-23 15:28:07.000000000 +0000
> ***************
> *** 22,25 ****
> --- 22,26 ----
>   #include <stdlib.h>
>   #include <time.h>
> + #include <stdint.h>
>   #include <signal.h>
>   #include <errno.h>
> ***************
> *** 93,104 ****
>   
>     struct timespec ts;
> -   jlong m, startTime;
>   
>     if (millis > 0 || nanos > 0)
>       {
> !       startTime = java::lang::System::currentTimeMillis();
> !       m = millis + startTime;
> !       ts.tv_sec = m / 1000; 
> !       ts.tv_nsec = ((m % 1000) * 1000000) + nanos; 
>       }
>   
> --- 94,118 ----
>   
>     struct timespec ts;
>   
>     if (millis > 0 || nanos > 0)
>       {
> !       // Calculate the abstime corresponding to the timeout.
> !       // Everything is in milliseconds.
> !       uintmax_t startTime = 
> (uintmax_t)java::lang::System::currentTimeMillis();
> !       uintmax_t m = (uintmax_t)millis + startTime;
> !       uintmax_t seconds = m / 1000; 
> ! 
> !       ts.tv_sec = seconds;
> !       if (ts.tv_sec < 0 || ts.tv_sec != seconds)
> !         {
> !           // We treat a timeout that won't fit into a 
> struct timespec
> !           // as a wait forever.
> !           millis = nanos = 0;
> !         }
> !       else
> !         {
> !           m = m - seconds * 1000;
> !           ts.tv_nsec = m * 1000000 + (uintmax_t)nanos;
> !         }
>       }
>   
>