You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by "Salikh Zakirov (JIRA)" <ji...@apache.org> on 2007/02/28 18:53:57 UTC
[jira] Created: (HARMONY-3267) [drlvm][thread] Thread.stop()
implementation is broken
[drlvm][thread] Thread.stop() implementation is broken
------------------------------------------------------
Key: HARMONY-3267
URL: https://issues.apache.org/jira/browse/HARMONY-3267
Project: Harmony
Issue Type: Bug
Components: DRLVM
Reporter: Salikh Zakirov
Priority: Minor
Attachments: StopBlocked.java, StopWaiting.java
The current implementation of Thread.stop() in DRLVM is completely broken.
The attached tests StopWaiting.java and StopBlocked.java demonstrate current incorrect behaviour:
DRLVM can't stop the thread which is blocked either waiting or entering monitor.
The waiting case can be fixed by checking the thread state, setting some checked status and waking thread up, like the patch below.
However, the blocking case cannot fixed in this way, as we have no easy way to wake up a thread which is blocked on a pthread_mutex_lock or EnterCriticalSection OS call.
It looks like a correct implementation of stopping blocked thread will require either pthread_kill() or SuspendThread() and modifying thread context (like throwing an exception or longjmp).
Both tests pass on competition VMs (Sun, Bea, Ibm).
----8<-----
A limited fix for a waiting case only.
--- vm/thread/src/thread_java_basic.c
+++ vm/thread/src/thread_java_basic.c
@@ -416,7 +416,23 @@ void stop_callback(void) {
tm_native_thread->suspend_request = 0;
hysem_post(tm_native_thread->resume_event);
+ // Does not return if the exception could be thrown straight away
jthread_throw_exception_object(excn);
+
+ // getting here means top stack frame is non-unwindable.
+
+ if (tm_native_thread->state &
+ (TM_THREAD_STATE_SLEEPING | TM_THREAD_STATE_WAITING_WITH_TIMEOUT
+ | TM_THREAD_STATE_WAITING | TM_THREAD_STATE_IN_MONITOR_WAIT
+ | TM_THREAD_STATE_WAITING_INDEFINITELY | TM_THREAD_STATE_PARKED)) {
+ // This is needed for correct stopping of a thread blocked on monitor_wait.
+ // The thread needs some flag to exit its waiting loop.
+ // We piggy-back on interrupted status. A correct exception from TLS
+ // will be thrown because the check of exception status on leaving
+ // JNI frame comes before checking return status in Object.wait().
+ // Interrupted status will be cleared by
+ hythread_interrupt(tm_native_thread);
+ }
}
/**
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (HARMONY-3267) [drlvm][thread] Thread.stop()
implementation is broken
Posted by "weldon washburn (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3267?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
weldon washburn closed HARMONY-3267.
------------------------------------
Resolution: Fixed
The latest patch was commited at revision 596372
> [drlvm][thread] Thread.stop() implementation is broken
> ------------------------------------------------------
>
> Key: HARMONY-3267
> URL: https://issues.apache.org/jira/browse/HARMONY-3267
> Project: Harmony
> Issue Type: Bug
> Components: DRLVM
> Reporter: Salikh Zakirov
> Assignee: weldon washburn
> Priority: Minor
> Attachments: H3267.patch, H3267.patch, Limited-Thread.stop-fix-for-stopping-waiting-threads.patch, StopBlocked.java, StopBlockedOnMonitor.java, StopWaiting.java
>
>
> The current implementation of Thread.stop() in DRLVM is completely broken.
> The attached tests StopWaiting.java and StopBlocked.java demonstrate current incorrect behaviour:
> DRLVM can't stop the thread which is blocked either waiting or entering monitor.
> The waiting case can be fixed by checking the thread state, setting some checked status and waking thread up, like the patch below.
> However, the blocking case cannot fixed in this way, as we have no easy way to wake up a thread which is blocked on a pthread_mutex_lock or EnterCriticalSection OS call.
> It looks like a correct implementation of stopping blocked thread will require either pthread_kill() or SuspendThread() and modifying thread context (like throwing an exception or longjmp).
> Both tests pass on competition VMs (Sun, Bea, Ibm).
> ----8<-----
> A limited fix for a waiting case only.
> --- vm/thread/src/thread_java_basic.c
> +++ vm/thread/src/thread_java_basic.c
> @@ -416,7 +416,23 @@ void stop_callback(void) {
> tm_native_thread->suspend_request = 0;
> hysem_post(tm_native_thread->resume_event);
>
> + // Does not return if the exception could be thrown straight away
> jthread_throw_exception_object(excn);
> +
> + // getting here means top stack frame is non-unwindable.
> +
> + if (tm_native_thread->state &
> + (TM_THREAD_STATE_SLEEPING | TM_THREAD_STATE_WAITING_WITH_TIMEOUT
> + | TM_THREAD_STATE_WAITING | TM_THREAD_STATE_IN_MONITOR_WAIT
> + | TM_THREAD_STATE_WAITING_INDEFINITELY | TM_THREAD_STATE_PARKED)) {
> + // This is needed for correct stopping of a thread blocked on monitor_wait.
> + // The thread needs some flag to exit its waiting loop.
> + // We piggy-back on interrupted status. A correct exception from TLS
> + // will be thrown because the check of exception status on leaving
> + // JNI frame comes before checking return status in Object.wait().
> + // Interrupted status will be cleared by
> + hythread_interrupt(tm_native_thread);
> + }
> }
>
> /**
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HARMONY-3267) [drlvm][thread] Thread.stop()
implementation is broken
Posted by "Pavel Rebriy (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3267?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Pavel Rebriy updated HARMONY-3267:
----------------------------------
Attachment: H3267.patch
License is added.
> [drlvm][thread] Thread.stop() implementation is broken
> ------------------------------------------------------
>
> Key: HARMONY-3267
> URL: https://issues.apache.org/jira/browse/HARMONY-3267
> Project: Harmony
> Issue Type: Bug
> Components: DRLVM
> Reporter: Salikh Zakirov
> Assignee: weldon washburn
> Priority: Minor
> Attachments: H3267.patch, H3267.patch, Limited-Thread.stop-fix-for-stopping-waiting-threads.patch, StopBlocked.java, StopBlockedOnMonitor.java, StopWaiting.java
>
>
> The current implementation of Thread.stop() in DRLVM is completely broken.
> The attached tests StopWaiting.java and StopBlocked.java demonstrate current incorrect behaviour:
> DRLVM can't stop the thread which is blocked either waiting or entering monitor.
> The waiting case can be fixed by checking the thread state, setting some checked status and waking thread up, like the patch below.
> However, the blocking case cannot fixed in this way, as we have no easy way to wake up a thread which is blocked on a pthread_mutex_lock or EnterCriticalSection OS call.
> It looks like a correct implementation of stopping blocked thread will require either pthread_kill() or SuspendThread() and modifying thread context (like throwing an exception or longjmp).
> Both tests pass on competition VMs (Sun, Bea, Ibm).
> ----8<-----
> A limited fix for a waiting case only.
> --- vm/thread/src/thread_java_basic.c
> +++ vm/thread/src/thread_java_basic.c
> @@ -416,7 +416,23 @@ void stop_callback(void) {
> tm_native_thread->suspend_request = 0;
> hysem_post(tm_native_thread->resume_event);
>
> + // Does not return if the exception could be thrown straight away
> jthread_throw_exception_object(excn);
> +
> + // getting here means top stack frame is non-unwindable.
> +
> + if (tm_native_thread->state &
> + (TM_THREAD_STATE_SLEEPING | TM_THREAD_STATE_WAITING_WITH_TIMEOUT
> + | TM_THREAD_STATE_WAITING | TM_THREAD_STATE_IN_MONITOR_WAIT
> + | TM_THREAD_STATE_WAITING_INDEFINITELY | TM_THREAD_STATE_PARKED)) {
> + // This is needed for correct stopping of a thread blocked on monitor_wait.
> + // The thread needs some flag to exit its waiting loop.
> + // We piggy-back on interrupted status. A correct exception from TLS
> + // will be thrown because the check of exception status on leaving
> + // JNI frame comes before checking return status in Object.wait().
> + // Interrupted status will be cleared by
> + hythread_interrupt(tm_native_thread);
> + }
> }
>
> /**
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HARMONY-3267) [drlvm][thread] Thread.stop()
implementation is broken
Posted by "Pavel Rebriy (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3267?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Pavel Rebriy updated HARMONY-3267:
----------------------------------
Attachment: H3267.patch
The patch to fix the issue.
With the patch DRLVM has got results the same as RI.
> [drlvm][thread] Thread.stop() implementation is broken
> ------------------------------------------------------
>
> Key: HARMONY-3267
> URL: https://issues.apache.org/jira/browse/HARMONY-3267
> Project: Harmony
> Issue Type: Bug
> Components: DRLVM
> Reporter: Salikh Zakirov
> Assignee: weldon washburn
> Priority: Minor
> Attachments: H3267.patch, H3267.patch, Limited-Thread.stop-fix-for-stopping-waiting-threads.patch, StopBlocked.java, StopBlockedOnMonitor.java, StopWaiting.java
>
>
> The current implementation of Thread.stop() in DRLVM is completely broken.
> The attached tests StopWaiting.java and StopBlocked.java demonstrate current incorrect behaviour:
> DRLVM can't stop the thread which is blocked either waiting or entering monitor.
> The waiting case can be fixed by checking the thread state, setting some checked status and waking thread up, like the patch below.
> However, the blocking case cannot fixed in this way, as we have no easy way to wake up a thread which is blocked on a pthread_mutex_lock or EnterCriticalSection OS call.
> It looks like a correct implementation of stopping blocked thread will require either pthread_kill() or SuspendThread() and modifying thread context (like throwing an exception or longjmp).
> Both tests pass on competition VMs (Sun, Bea, Ibm).
> ----8<-----
> A limited fix for a waiting case only.
> --- vm/thread/src/thread_java_basic.c
> +++ vm/thread/src/thread_java_basic.c
> @@ -416,7 +416,23 @@ void stop_callback(void) {
> tm_native_thread->suspend_request = 0;
> hysem_post(tm_native_thread->resume_event);
>
> + // Does not return if the exception could be thrown straight away
> jthread_throw_exception_object(excn);
> +
> + // getting here means top stack frame is non-unwindable.
> +
> + if (tm_native_thread->state &
> + (TM_THREAD_STATE_SLEEPING | TM_THREAD_STATE_WAITING_WITH_TIMEOUT
> + | TM_THREAD_STATE_WAITING | TM_THREAD_STATE_IN_MONITOR_WAIT
> + | TM_THREAD_STATE_WAITING_INDEFINITELY | TM_THREAD_STATE_PARKED)) {
> + // This is needed for correct stopping of a thread blocked on monitor_wait.
> + // The thread needs some flag to exit its waiting loop.
> + // We piggy-back on interrupted status. A correct exception from TLS
> + // will be thrown because the check of exception status on leaving
> + // JNI frame comes before checking return status in Object.wait().
> + // Interrupted status will be cleared by
> + hythread_interrupt(tm_native_thread);
> + }
> }
>
> /**
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (HARMONY-3267) [drlvm][thread] Thread.stop()
implementation is broken
Posted by "weldon washburn (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3267?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
weldon washburn reassigned HARMONY-3267:
----------------------------------------
Assignee: weldon washburn
> [drlvm][thread] Thread.stop() implementation is broken
> ------------------------------------------------------
>
> Key: HARMONY-3267
> URL: https://issues.apache.org/jira/browse/HARMONY-3267
> Project: Harmony
> Issue Type: Bug
> Components: DRLVM
> Reporter: Salikh Zakirov
> Assigned To: weldon washburn
> Priority: Minor
> Attachments: Limited-Thread.stop-fix-for-stopping-waiting-threads.patch, StopBlocked.java, StopBlockedOnMonitor.java, StopWaiting.java
>
>
> The current implementation of Thread.stop() in DRLVM is completely broken.
> The attached tests StopWaiting.java and StopBlocked.java demonstrate current incorrect behaviour:
> DRLVM can't stop the thread which is blocked either waiting or entering monitor.
> The waiting case can be fixed by checking the thread state, setting some checked status and waking thread up, like the patch below.
> However, the blocking case cannot fixed in this way, as we have no easy way to wake up a thread which is blocked on a pthread_mutex_lock or EnterCriticalSection OS call.
> It looks like a correct implementation of stopping blocked thread will require either pthread_kill() or SuspendThread() and modifying thread context (like throwing an exception or longjmp).
> Both tests pass on competition VMs (Sun, Bea, Ibm).
> ----8<-----
> A limited fix for a waiting case only.
> --- vm/thread/src/thread_java_basic.c
> +++ vm/thread/src/thread_java_basic.c
> @@ -416,7 +416,23 @@ void stop_callback(void) {
> tm_native_thread->suspend_request = 0;
> hysem_post(tm_native_thread->resume_event);
>
> + // Does not return if the exception could be thrown straight away
> jthread_throw_exception_object(excn);
> +
> + // getting here means top stack frame is non-unwindable.
> +
> + if (tm_native_thread->state &
> + (TM_THREAD_STATE_SLEEPING | TM_THREAD_STATE_WAITING_WITH_TIMEOUT
> + | TM_THREAD_STATE_WAITING | TM_THREAD_STATE_IN_MONITOR_WAIT
> + | TM_THREAD_STATE_WAITING_INDEFINITELY | TM_THREAD_STATE_PARKED)) {
> + // This is needed for correct stopping of a thread blocked on monitor_wait.
> + // The thread needs some flag to exit its waiting loop.
> + // We piggy-back on interrupted status. A correct exception from TLS
> + // will be thrown because the check of exception status on leaving
> + // JNI frame comes before checking return status in Object.wait().
> + // Interrupted status will be cleared by
> + hythread_interrupt(tm_native_thread);
> + }
> }
>
> /**
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HARMONY-3267) [drlvm][thread] Thread.stop()
implementation is broken
Posted by "Salikh Zakirov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3267?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477786 ]
Salikh Zakirov commented on HARMONY-3267:
-----------------------------------------
The fixed case is exercized by test StopWaiting.java.
So, I agree that it makes sense to reformat StopWaiting.java to the regression test format and add to regression test suite.
> [drlvm][thread] Thread.stop() implementation is broken
> ------------------------------------------------------
>
> Key: HARMONY-3267
> URL: https://issues.apache.org/jira/browse/HARMONY-3267
> Project: Harmony
> Issue Type: Bug
> Components: DRLVM
> Reporter: Salikh Zakirov
> Assigned To: weldon washburn
> Priority: Minor
> Attachments: Limited-Thread.stop-fix-for-stopping-waiting-threads.patch, StopBlocked.java, StopBlockedOnMonitor.java, StopWaiting.java
>
>
> The current implementation of Thread.stop() in DRLVM is completely broken.
> The attached tests StopWaiting.java and StopBlocked.java demonstrate current incorrect behaviour:
> DRLVM can't stop the thread which is blocked either waiting or entering monitor.
> The waiting case can be fixed by checking the thread state, setting some checked status and waking thread up, like the patch below.
> However, the blocking case cannot fixed in this way, as we have no easy way to wake up a thread which is blocked on a pthread_mutex_lock or EnterCriticalSection OS call.
> It looks like a correct implementation of stopping blocked thread will require either pthread_kill() or SuspendThread() and modifying thread context (like throwing an exception or longjmp).
> Both tests pass on competition VMs (Sun, Bea, Ibm).
> ----8<-----
> A limited fix for a waiting case only.
> --- vm/thread/src/thread_java_basic.c
> +++ vm/thread/src/thread_java_basic.c
> @@ -416,7 +416,23 @@ void stop_callback(void) {
> tm_native_thread->suspend_request = 0;
> hysem_post(tm_native_thread->resume_event);
>
> + // Does not return if the exception could be thrown straight away
> jthread_throw_exception_object(excn);
> +
> + // getting here means top stack frame is non-unwindable.
> +
> + if (tm_native_thread->state &
> + (TM_THREAD_STATE_SLEEPING | TM_THREAD_STATE_WAITING_WITH_TIMEOUT
> + | TM_THREAD_STATE_WAITING | TM_THREAD_STATE_IN_MONITOR_WAIT
> + | TM_THREAD_STATE_WAITING_INDEFINITELY | TM_THREAD_STATE_PARKED)) {
> + // This is needed for correct stopping of a thread blocked on monitor_wait.
> + // The thread needs some flag to exit its waiting loop.
> + // We piggy-back on interrupted status. A correct exception from TLS
> + // will be thrown because the check of exception status on leaving
> + // JNI frame comes before checking return status in Object.wait().
> + // Interrupted status will be cleared by
> + hythread_interrupt(tm_native_thread);
> + }
> }
>
> /**
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HARMONY-3267) [drlvm][thread] Thread.stop()
implementation is broken
Posted by "Salikh Zakirov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3267?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Salikh Zakirov updated HARMONY-3267:
------------------------------------
Attachment: StopBlocked.java
> [drlvm][thread] Thread.stop() implementation is broken
> ------------------------------------------------------
>
> Key: HARMONY-3267
> URL: https://issues.apache.org/jira/browse/HARMONY-3267
> Project: Harmony
> Issue Type: Bug
> Components: DRLVM
> Reporter: Salikh Zakirov
> Priority: Minor
> Attachments: StopBlocked.java, StopWaiting.java
>
>
> The current implementation of Thread.stop() in DRLVM is completely broken.
> The attached tests StopWaiting.java and StopBlocked.java demonstrate current incorrect behaviour:
> DRLVM can't stop the thread which is blocked either waiting or entering monitor.
> The waiting case can be fixed by checking the thread state, setting some checked status and waking thread up, like the patch below.
> However, the blocking case cannot fixed in this way, as we have no easy way to wake up a thread which is blocked on a pthread_mutex_lock or EnterCriticalSection OS call.
> It looks like a correct implementation of stopping blocked thread will require either pthread_kill() or SuspendThread() and modifying thread context (like throwing an exception or longjmp).
> Both tests pass on competition VMs (Sun, Bea, Ibm).
> ----8<-----
> A limited fix for a waiting case only.
> --- vm/thread/src/thread_java_basic.c
> +++ vm/thread/src/thread_java_basic.c
> @@ -416,7 +416,23 @@ void stop_callback(void) {
> tm_native_thread->suspend_request = 0;
> hysem_post(tm_native_thread->resume_event);
>
> + // Does not return if the exception could be thrown straight away
> jthread_throw_exception_object(excn);
> +
> + // getting here means top stack frame is non-unwindable.
> +
> + if (tm_native_thread->state &
> + (TM_THREAD_STATE_SLEEPING | TM_THREAD_STATE_WAITING_WITH_TIMEOUT
> + | TM_THREAD_STATE_WAITING | TM_THREAD_STATE_IN_MONITOR_WAIT
> + | TM_THREAD_STATE_WAITING_INDEFINITELY | TM_THREAD_STATE_PARKED)) {
> + // This is needed for correct stopping of a thread blocked on monitor_wait.
> + // The thread needs some flag to exit its waiting loop.
> + // We piggy-back on interrupted status. A correct exception from TLS
> + // will be thrown because the check of exception status on leaving
> + // JNI frame comes before checking return status in Object.wait().
> + // Interrupted status will be cleared by
> + hythread_interrupt(tm_native_thread);
> + }
> }
>
> /**
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HARMONY-3267) [drlvm][thread] Thread.stop()
implementation is broken
Posted by "Salikh Zakirov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3267?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Salikh Zakirov updated HARMONY-3267:
------------------------------------
Attachment: Limited-Thread.stop-fix-for-stopping-waiting-threads.patch
The Limited-Thread.stop-fix-for-stopping-waiting-threads.patch provides a limited fix for the waiting case.
Since even the RI does not handle blocked case well, we may live with a deficient implementation as well,
and the waiting case is worth fixing.
> [drlvm][thread] Thread.stop() implementation is broken
> ------------------------------------------------------
>
> Key: HARMONY-3267
> URL: https://issues.apache.org/jira/browse/HARMONY-3267
> Project: Harmony
> Issue Type: Bug
> Components: DRLVM
> Reporter: Salikh Zakirov
> Priority: Minor
> Attachments: Limited-Thread.stop-fix-for-stopping-waiting-threads.patch, StopBlocked.java, StopBlockedOnMonitor.java, StopWaiting.java
>
>
> The current implementation of Thread.stop() in DRLVM is completely broken.
> The attached tests StopWaiting.java and StopBlocked.java demonstrate current incorrect behaviour:
> DRLVM can't stop the thread which is blocked either waiting or entering monitor.
> The waiting case can be fixed by checking the thread state, setting some checked status and waking thread up, like the patch below.
> However, the blocking case cannot fixed in this way, as we have no easy way to wake up a thread which is blocked on a pthread_mutex_lock or EnterCriticalSection OS call.
> It looks like a correct implementation of stopping blocked thread will require either pthread_kill() or SuspendThread() and modifying thread context (like throwing an exception or longjmp).
> Both tests pass on competition VMs (Sun, Bea, Ibm).
> ----8<-----
> A limited fix for a waiting case only.
> --- vm/thread/src/thread_java_basic.c
> +++ vm/thread/src/thread_java_basic.c
> @@ -416,7 +416,23 @@ void stop_callback(void) {
> tm_native_thread->suspend_request = 0;
> hysem_post(tm_native_thread->resume_event);
>
> + // Does not return if the exception could be thrown straight away
> jthread_throw_exception_object(excn);
> +
> + // getting here means top stack frame is non-unwindable.
> +
> + if (tm_native_thread->state &
> + (TM_THREAD_STATE_SLEEPING | TM_THREAD_STATE_WAITING_WITH_TIMEOUT
> + | TM_THREAD_STATE_WAITING | TM_THREAD_STATE_IN_MONITOR_WAIT
> + | TM_THREAD_STATE_WAITING_INDEFINITELY | TM_THREAD_STATE_PARKED)) {
> + // This is needed for correct stopping of a thread blocked on monitor_wait.
> + // The thread needs some flag to exit its waiting loop.
> + // We piggy-back on interrupted status. A correct exception from TLS
> + // will be thrown because the check of exception status on leaving
> + // JNI frame comes before checking return status in Object.wait().
> + // Interrupted status will be cleared by
> + hythread_interrupt(tm_native_thread);
> + }
> }
>
> /**
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HARMONY-3267) [drlvm][thread] Thread.stop()
implementation is broken
Posted by "weldon washburn (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3267?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12477573 ]
weldon washburn commented on HARMONY-3267:
------------------------------------------
I assume the *.java regression tests go in .../src/test/regression/H3267. Please let me know if this is wrong.
> [drlvm][thread] Thread.stop() implementation is broken
> ------------------------------------------------------
>
> Key: HARMONY-3267
> URL: https://issues.apache.org/jira/browse/HARMONY-3267
> Project: Harmony
> Issue Type: Bug
> Components: DRLVM
> Reporter: Salikh Zakirov
> Assigned To: weldon washburn
> Priority: Minor
> Attachments: Limited-Thread.stop-fix-for-stopping-waiting-threads.patch, StopBlocked.java, StopBlockedOnMonitor.java, StopWaiting.java
>
>
> The current implementation of Thread.stop() in DRLVM is completely broken.
> The attached tests StopWaiting.java and StopBlocked.java demonstrate current incorrect behaviour:
> DRLVM can't stop the thread which is blocked either waiting or entering monitor.
> The waiting case can be fixed by checking the thread state, setting some checked status and waking thread up, like the patch below.
> However, the blocking case cannot fixed in this way, as we have no easy way to wake up a thread which is blocked on a pthread_mutex_lock or EnterCriticalSection OS call.
> It looks like a correct implementation of stopping blocked thread will require either pthread_kill() or SuspendThread() and modifying thread context (like throwing an exception or longjmp).
> Both tests pass on competition VMs (Sun, Bea, Ibm).
> ----8<-----
> A limited fix for a waiting case only.
> --- vm/thread/src/thread_java_basic.c
> +++ vm/thread/src/thread_java_basic.c
> @@ -416,7 +416,23 @@ void stop_callback(void) {
> tm_native_thread->suspend_request = 0;
> hysem_post(tm_native_thread->resume_event);
>
> + // Does not return if the exception could be thrown straight away
> jthread_throw_exception_object(excn);
> +
> + // getting here means top stack frame is non-unwindable.
> +
> + if (tm_native_thread->state &
> + (TM_THREAD_STATE_SLEEPING | TM_THREAD_STATE_WAITING_WITH_TIMEOUT
> + | TM_THREAD_STATE_WAITING | TM_THREAD_STATE_IN_MONITOR_WAIT
> + | TM_THREAD_STATE_WAITING_INDEFINITELY | TM_THREAD_STATE_PARKED)) {
> + // This is needed for correct stopping of a thread blocked on monitor_wait.
> + // The thread needs some flag to exit its waiting loop.
> + // We piggy-back on interrupted status. A correct exception from TLS
> + // will be thrown because the check of exception status on leaving
> + // JNI frame comes before checking return status in Object.wait().
> + // Interrupted status will be cleared by
> + hythread_interrupt(tm_native_thread);
> + }
> }
>
> /**
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HARMONY-3267) [drlvm][thread] Thread.stop()
implementation is broken
Posted by "Salikh Zakirov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3267?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Salikh Zakirov updated HARMONY-3267:
------------------------------------
Estimated Complexity: Moderate
Patch Info: [Patch Available]
> [drlvm][thread] Thread.stop() implementation is broken
> ------------------------------------------------------
>
> Key: HARMONY-3267
> URL: https://issues.apache.org/jira/browse/HARMONY-3267
> Project: Harmony
> Issue Type: Bug
> Components: DRLVM
> Reporter: Salikh Zakirov
> Priority: Minor
> Attachments: Limited-Thread.stop-fix-for-stopping-waiting-threads.patch, StopBlocked.java, StopBlockedOnMonitor.java, StopWaiting.java
>
>
> The current implementation of Thread.stop() in DRLVM is completely broken.
> The attached tests StopWaiting.java and StopBlocked.java demonstrate current incorrect behaviour:
> DRLVM can't stop the thread which is blocked either waiting or entering monitor.
> The waiting case can be fixed by checking the thread state, setting some checked status and waking thread up, like the patch below.
> However, the blocking case cannot fixed in this way, as we have no easy way to wake up a thread which is blocked on a pthread_mutex_lock or EnterCriticalSection OS call.
> It looks like a correct implementation of stopping blocked thread will require either pthread_kill() or SuspendThread() and modifying thread context (like throwing an exception or longjmp).
> Both tests pass on competition VMs (Sun, Bea, Ibm).
> ----8<-----
> A limited fix for a waiting case only.
> --- vm/thread/src/thread_java_basic.c
> +++ vm/thread/src/thread_java_basic.c
> @@ -416,7 +416,23 @@ void stop_callback(void) {
> tm_native_thread->suspend_request = 0;
> hysem_post(tm_native_thread->resume_event);
>
> + // Does not return if the exception could be thrown straight away
> jthread_throw_exception_object(excn);
> +
> + // getting here means top stack frame is non-unwindable.
> +
> + if (tm_native_thread->state &
> + (TM_THREAD_STATE_SLEEPING | TM_THREAD_STATE_WAITING_WITH_TIMEOUT
> + | TM_THREAD_STATE_WAITING | TM_THREAD_STATE_IN_MONITOR_WAIT
> + | TM_THREAD_STATE_WAITING_INDEFINITELY | TM_THREAD_STATE_PARKED)) {
> + // This is needed for correct stopping of a thread blocked on monitor_wait.
> + // The thread needs some flag to exit its waiting loop.
> + // We piggy-back on interrupted status. A correct exception from TLS
> + // will be thrown because the check of exception status on leaving
> + // JNI frame comes before checking return status in Object.wait().
> + // Interrupted status will be cleared by
> + hythread_interrupt(tm_native_thread);
> + }
> }
>
> /**
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HARMONY-3267) [drlvm][thread] Thread.stop()
implementation is broken
Posted by "weldon washburn (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3267?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12478136 ]
weldon washburn commented on HARMONY-3267:
------------------------------------------
build, build test pass on Linux32. But on windows32, I get the following rather strange error message. Please see if the problem can be reproduced:
Class java.lang.RuntimeTest2
Name Tests Errors Failures Time(s)
RuntimeTest2 11 0 1 13.400
Tests
Name Status Type Time(s)
test_runFinalization Success 3.875
test_runFinalizersOnExit Success 3.936
test_addShutdownHook Success 0.030
test_removeShutdownHook Success 0.030
test_exec_Str Failure exec(String[], String[], File).check004: where is the date's reaction on the incorrect enterring?
junit.framework.AssertionFailedError: exec(String[], String[], File).check004: where is the date's reaction on the incorrect enterring? at junit.framework.Assert.fail(Assert.java:47) at java.lang.RuntimeTest2.test_exec_Str(RuntimeTest2.java:449) at java.lang.reflect.VMReflection.invokeMethod(VMReflection.java) at java.lang.reflect.Method.invoke(Method.java:381) at junit.framework.TestCase.runTest(TestCase.java:164) at junit.framework.TestCase.runBare(TestCase.java:128) at junit.framework.TestResult$1.protect(TestResult.java) at junit.framework.TestResult.runProtected(TestResult.java) at junit.framework.TestResult.run(TestResult.java:104) at junit.framework.TestCase.run(TestCase.java) at junit.framework.TestSuite.runTest(TestSuite.java) at junit.framework.TestSuite.run(TestSuite.java:224) at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.run(JUnitTestRunner.java:294) at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.launch(JUnitTestRunner.java:670) at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.main(JUnitTestRunner.java:545)
> [drlvm][thread] Thread.stop() implementation is broken
> ------------------------------------------------------
>
> Key: HARMONY-3267
> URL: https://issues.apache.org/jira/browse/HARMONY-3267
> Project: Harmony
> Issue Type: Bug
> Components: DRLVM
> Reporter: Salikh Zakirov
> Assigned To: weldon washburn
> Priority: Minor
> Attachments: Limited-Thread.stop-fix-for-stopping-waiting-threads.patch, StopBlocked.java, StopBlockedOnMonitor.java, StopWaiting.java
>
>
> The current implementation of Thread.stop() in DRLVM is completely broken.
> The attached tests StopWaiting.java and StopBlocked.java demonstrate current incorrect behaviour:
> DRLVM can't stop the thread which is blocked either waiting or entering monitor.
> The waiting case can be fixed by checking the thread state, setting some checked status and waking thread up, like the patch below.
> However, the blocking case cannot fixed in this way, as we have no easy way to wake up a thread which is blocked on a pthread_mutex_lock or EnterCriticalSection OS call.
> It looks like a correct implementation of stopping blocked thread will require either pthread_kill() or SuspendThread() and modifying thread context (like throwing an exception or longjmp).
> Both tests pass on competition VMs (Sun, Bea, Ibm).
> ----8<-----
> A limited fix for a waiting case only.
> --- vm/thread/src/thread_java_basic.c
> +++ vm/thread/src/thread_java_basic.c
> @@ -416,7 +416,23 @@ void stop_callback(void) {
> tm_native_thread->suspend_request = 0;
> hysem_post(tm_native_thread->resume_event);
>
> + // Does not return if the exception could be thrown straight away
> jthread_throw_exception_object(excn);
> +
> + // getting here means top stack frame is non-unwindable.
> +
> + if (tm_native_thread->state &
> + (TM_THREAD_STATE_SLEEPING | TM_THREAD_STATE_WAITING_WITH_TIMEOUT
> + | TM_THREAD_STATE_WAITING | TM_THREAD_STATE_IN_MONITOR_WAIT
> + | TM_THREAD_STATE_WAITING_INDEFINITELY | TM_THREAD_STATE_PARKED)) {
> + // This is needed for correct stopping of a thread blocked on monitor_wait.
> + // The thread needs some flag to exit its waiting loop.
> + // We piggy-back on interrupted status. A correct exception from TLS
> + // will be thrown because the check of exception status on leaving
> + // JNI frame comes before checking return status in Object.wait().
> + // Interrupted status will be cleared by
> + hythread_interrupt(tm_native_thread);
> + }
> }
>
> /**
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HARMONY-3267) [drlvm][thread] Thread.stop()
implementation is broken
Posted by "Salikh Zakirov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3267?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Salikh Zakirov updated HARMONY-3267:
------------------------------------
Attachment: StopBlockedOnMonitor.java
The test StopBlockedOnMonitor tests how VM can stop a thread blocked on a genuine MONITORENTER instruction (rather than a re-acquire of monitor on wait() exit of StopBlocked test).
This test shows surprising results:
Bea has some race with 2 possible outcomes: (1) can't stop thread, (2) the stopped thread falls through MONITORENTER instruction as if it managed to enter a monitor currently hold by other thread. Looks like a bug in VM.
Sun, J9: fail
DRLVM: passes.
DRLVM works okay on this test because of "thin lock" optimization used in DRLVM. The first thread to contend for a monitor first spins in a loop, and inflates a lock only after it manages to grab a lock. Since busy loop has a safepoint in it, the thread notices stop_callback requested, and successfully throws an exception.
A modifcation of test to make sure the lock is inflated before blocking on it makes all VMs fail in a stable manner:
--- /home/sszakiro/Desktop/StopBlockedOnMonitor.java 2007-02-28 21:19:44.809276900 +0300
+++ StopBlockedOnMonitor.java 2007-02-28 21:53:38.807620500 +0300
@@ -5,6 +5,11 @@
public static void main(String[] args) {
try {
+ synchronized (lock) {
+ // use wait() to inflate a lock
+ lock.wait(1);
+ }
+
// grab the monitor and keep it forever
synchronized (lock) {
StopBlockedOnMonitor t = new StopBlockedOnMonitor();
> [drlvm][thread] Thread.stop() implementation is broken
> ------------------------------------------------------
>
> Key: HARMONY-3267
> URL: https://issues.apache.org/jira/browse/HARMONY-3267
> Project: Harmony
> Issue Type: Bug
> Components: DRLVM
> Reporter: Salikh Zakirov
> Priority: Minor
> Attachments: StopBlocked.java, StopBlockedOnMonitor.java, StopWaiting.java
>
>
> The current implementation of Thread.stop() in DRLVM is completely broken.
> The attached tests StopWaiting.java and StopBlocked.java demonstrate current incorrect behaviour:
> DRLVM can't stop the thread which is blocked either waiting or entering monitor.
> The waiting case can be fixed by checking the thread state, setting some checked status and waking thread up, like the patch below.
> However, the blocking case cannot fixed in this way, as we have no easy way to wake up a thread which is blocked on a pthread_mutex_lock or EnterCriticalSection OS call.
> It looks like a correct implementation of stopping blocked thread will require either pthread_kill() or SuspendThread() and modifying thread context (like throwing an exception or longjmp).
> Both tests pass on competition VMs (Sun, Bea, Ibm).
> ----8<-----
> A limited fix for a waiting case only.
> --- vm/thread/src/thread_java_basic.c
> +++ vm/thread/src/thread_java_basic.c
> @@ -416,7 +416,23 @@ void stop_callback(void) {
> tm_native_thread->suspend_request = 0;
> hysem_post(tm_native_thread->resume_event);
>
> + // Does not return if the exception could be thrown straight away
> jthread_throw_exception_object(excn);
> +
> + // getting here means top stack frame is non-unwindable.
> +
> + if (tm_native_thread->state &
> + (TM_THREAD_STATE_SLEEPING | TM_THREAD_STATE_WAITING_WITH_TIMEOUT
> + | TM_THREAD_STATE_WAITING | TM_THREAD_STATE_IN_MONITOR_WAIT
> + | TM_THREAD_STATE_WAITING_INDEFINITELY | TM_THREAD_STATE_PARKED)) {
> + // This is needed for correct stopping of a thread blocked on monitor_wait.
> + // The thread needs some flag to exit its waiting loop.
> + // We piggy-back on interrupted status. A correct exception from TLS
> + // will be thrown because the check of exception status on leaving
> + // JNI frame comes before checking return status in Object.wait().
> + // Interrupted status will be cleared by
> + hythread_interrupt(tm_native_thread);
> + }
> }
>
> /**
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HARMONY-3267) [drlvm][thread] Thread.stop()
implementation is broken
Posted by "Salikh Zakirov (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HARMONY-3267?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Salikh Zakirov updated HARMONY-3267:
------------------------------------
Attachment: StopWaiting.java
> [drlvm][thread] Thread.stop() implementation is broken
> ------------------------------------------------------
>
> Key: HARMONY-3267
> URL: https://issues.apache.org/jira/browse/HARMONY-3267
> Project: Harmony
> Issue Type: Bug
> Components: DRLVM
> Reporter: Salikh Zakirov
> Priority: Minor
> Attachments: StopBlocked.java, StopWaiting.java
>
>
> The current implementation of Thread.stop() in DRLVM is completely broken.
> The attached tests StopWaiting.java and StopBlocked.java demonstrate current incorrect behaviour:
> DRLVM can't stop the thread which is blocked either waiting or entering monitor.
> The waiting case can be fixed by checking the thread state, setting some checked status and waking thread up, like the patch below.
> However, the blocking case cannot fixed in this way, as we have no easy way to wake up a thread which is blocked on a pthread_mutex_lock or EnterCriticalSection OS call.
> It looks like a correct implementation of stopping blocked thread will require either pthread_kill() or SuspendThread() and modifying thread context (like throwing an exception or longjmp).
> Both tests pass on competition VMs (Sun, Bea, Ibm).
> ----8<-----
> A limited fix for a waiting case only.
> --- vm/thread/src/thread_java_basic.c
> +++ vm/thread/src/thread_java_basic.c
> @@ -416,7 +416,23 @@ void stop_callback(void) {
> tm_native_thread->suspend_request = 0;
> hysem_post(tm_native_thread->resume_event);
>
> + // Does not return if the exception could be thrown straight away
> jthread_throw_exception_object(excn);
> +
> + // getting here means top stack frame is non-unwindable.
> +
> + if (tm_native_thread->state &
> + (TM_THREAD_STATE_SLEEPING | TM_THREAD_STATE_WAITING_WITH_TIMEOUT
> + | TM_THREAD_STATE_WAITING | TM_THREAD_STATE_IN_MONITOR_WAIT
> + | TM_THREAD_STATE_WAITING_INDEFINITELY | TM_THREAD_STATE_PARKED)) {
> + // This is needed for correct stopping of a thread blocked on monitor_wait.
> + // The thread needs some flag to exit its waiting loop.
> + // We piggy-back on interrupted status. A correct exception from TLS
> + // will be thrown because the check of exception status on leaving
> + // JNI frame comes before checking return status in Object.wait().
> + // Interrupted status will be cleared by
> + hythread_interrupt(tm_native_thread);
> + }
> }
>
> /**
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.