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
}
