harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Salikh Zakirov <Salikh.Zaki...@Intel.com>
Subject Re: [drlvm][threading] is harmony-1951 a bug or a feature?
Date Tue, 21 Nov 2006 18:41:17 GMT
Artem Aliev wrote:
> Guys,
> 
> I attach a kind of fix for the problem to the JIRA, but I'm not sure I
> want to commit such fix.
> Please review and comment

Thanks to Artem for clear explanation of the behaviour of the Test3.
(see HARMONY-1951 comments).

Formally speaking, the Java specification does not guarantee that Test3.java
from HARMONY-1951 will work correctly, as specification allows for
"spurious" wake ups from wait().

Thus, the behaviour of the test is still within compliant with the spec.

Still, I agree that it would be nice to fix interruption() to prevent spurious wakeups.

Regarding the fix proposed by Artem, I think it is overly complex,
and does many things that are not required by the specification.
I'm thinking about a simpler fix using a state variable in HyThreadMonitor.
(which is conveniently protected by monitor mutex)
It looks like it fixes the Test3, but I need to test it some more.

diff --git vm/thread/src/thread_native_fat_monitor.c vm/thread/src/thread_native_fat_monitor.c
index 7e812a2..9994c22 100644
--- vm/thread/src/thread_native_fat_monitor.c
+++ vm/thread/src/thread_native_fat_monitor.c
@@ -67,6 +67,7 @@ IDATA VMCALL hythread_monitor_init_with_
     mon->flags = flags;
     mon->name  = name;
     mon->owner = 0;
+    mon->notify_flag = 0;
 
         *mon_ptr = mon;
     return TM_ERROR_NONE;
@@ -205,8 +206,33 @@ IDATA monitor_wait_impl(hythread_monitor
     self->state |= TM_THREAD_STATE_IN_MONITOR_WAIT;
     hymutex_unlock(self->mutex);
 
-    status =  condvar_wait_impl(mon_ptr->condition, mon_ptr->mutex, ms, nano, interruptable);
-
+    do {
+        apr_time_t start;
+        mon_ptr->notify_flag = 0; // clear flag before waiting
+        start = apr_time_now();
+        status = condvar_wait_impl(mon_ptr->condition, mon_ptr->mutex, ms, nano, interruptable);
+        if (status != TM_ERROR_NONE
+                || mon_ptr->notify_flag || hythread_interrupted(self))
+            break;
+        // we should not change ms and nano if both are 0 (meaning "no timeout")
+        if (ms || nano) {
+            apr_interval_time_t elapsed;
+            elapsed = apr_time_now() - start; // microseconds
+            nano -= (IDATA)((elapsed % 1000) * 1000);
+            if (nano < 0) {
+                ms -= elapsed/1000 + 1;
+                nano += 1000000;
+            } else {
+                ms -= elapsed/1000;
+            }
+            if (ms < 0) {
+                assert(status == TM_ERROR_NONE);
+                status = TM_ERROR_TIMEOUT;
+                break;
+            }
+            assert(0 <= nano && nano < 1000000);
+        }
+    } while (1);
     hymutex_lock(self->mutex);
     self->state &= ~TM_THREAD_STATE_IN_MONITOR_WAIT;
     self->current_condition = NULL;
@@ -321,6 +347,7 @@ IDATA VMCALL hythread_monitor_notify_all
         mon_ptr->notify_flag=1;
         return TM_ERROR_NONE;
 #else
+    mon_ptr->notify_flag = 1;
         return hycond_notify_all(mon_ptr->condition);   
 #endif
 }
@@ -347,6 +374,7 @@ IDATA VMCALL hythread_monitor_notify(hyt
         mon_ptr->notify_flag=1;
         return TM_ERROR_NONE;
 #else
+    mon_ptr->notify_flag = 1;
         return hycond_notify(mon_ptr->condition);   
 #endif
 }


Mime
View raw message