Harmony
  1. Harmony
  2. HARMONY-1592

Thread interrupt and Thread.stop may not work properly if thread is waiting

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: DRLVM
    • Labels:
      None

      Description

      Thread.interrupt and Thread.stop may not wake up a thread if it's waiting. The problem here is that both hythread_stop and hythread_interrupt do
      hythread_notify instead of hythread_notify_all on current condition the thread is waiting on, while this notification may not affect target thread.

      1. stop_interrupt_waited.patch
        1 kB
        Nikolay Kuznetsov
      2. Test.java
        1 kB
        Nikolay Kuznetsov
      3. Test2.java
        1 kB
        Nikolay Kuznetsov

        Issue Links

          Activity

          Hide
          Nikolay Kuznetsov added a comment -

          Fix for this issue.

          Show
          Nikolay Kuznetsov added a comment - Fix for this issue.
          Hide
          Geir Magnusson Jr added a comment -

          is there any way to demonstrate this with a test-case?

          Show
          Geir Magnusson Jr added a comment - is there any way to demonstrate this with a test-case?
          Hide
          Nikolay Kuznetsov added a comment -

          Sorry for the delay, I missed your comment somehow.

          Attached test case demonstrates the problem. The idea of the test is simple:
          Start two threads wich will wait on the same monitor, ensure that both threads
          are waiting and try to stop the one which began to wait later.

          Currently the test will hang, while the valid output should be:
          ./java Test
          Will start two threads which will wait on the same monitor
          Both threads are waiting now, will stop second one
          Thread was stopped, will notify waiting and exit

          Thank you.
          Nik.

          Show
          Nikolay Kuznetsov added a comment - Sorry for the delay, I missed your comment somehow. Attached test case demonstrates the problem. The idea of the test is simple: Start two threads wich will wait on the same monitor, ensure that both threads are waiting and try to stop the one which began to wait later. Currently the test will hang, while the valid output should be: ./java Test Will start two threads which will wait on the same monitor Both threads are waiting now, will stop second one Thread was stopped, will notify waiting and exit Thank you. Nik.
          Hide
          Geir Magnusson Jr added a comment -

          This doesn't work consistently for me after patch applied.

          There is a 50% chance that the test will fail. This is after the invocationAPI patch - can you please review to see if that had any effect?

          Show
          Geir Magnusson Jr added a comment - This doesn't work consistently for me after patch applied. There is a 50% chance that the test will fail. This is after the invocationAPI patch - can you please review to see if that had any effect?
          Hide
          Nikolay Kuznetsov added a comment -

          Geir,
          how it works for you? If it prints that Thread was stopped and hangs then it anothe problem, if main thread finishes earlier than child thread vm just hangs on exit, try the following test whic prints that sleep is finished and hangs on Linux:

          public class Test {
          public static void main(String[] args) {
          new Thread() {
          public void run() {
          try

          { Thread.sleep(5000); }

          catch (InterruptedException e)

          { e.printStackTrace(); }

          System.out.println("Sleep finished");
          }
          }.start();
          }
          }

          Show
          Nikolay Kuznetsov added a comment - Geir, how it works for you? If it prints that Thread was stopped and hangs then it anothe problem, if main thread finishes earlier than child thread vm just hangs on exit, try the following test whic prints that sleep is finished and hangs on Linux: public class Test { public static void main(String[] args) { new Thread() { public void run() { try { Thread.sleep(5000); } catch (InterruptedException e) { e.printStackTrace(); } System.out.println("Sleep finished"); } }.start(); } }
          Hide
          Nikolay Kuznetsov added a comment -

          HARMONY-1816 hang on shutdown problem also leads to the hang at shutdown of the test attached to ths issue

          Show
          Nikolay Kuznetsov added a comment - HARMONY-1816 hang on shutdown problem also leads to the hang at shutdown of the test attached to ths issue
          Hide
          Elena Semukhina added a comment -

          I tried the patch together with HARMONY-1816 and HARMONY-1823.
          They eiminate recent classlib tests pass rate degradation.
          drlvm smoke, cunit, kernel pass.

          Show
          Elena Semukhina added a comment - I tried the patch together with HARMONY-1816 and HARMONY-1823 . They eiminate recent classlib tests pass rate degradation. drlvm smoke, cunit, kernel pass.
          Hide
          Geir Magnusson Jr added a comment -

          The new test above in the comments also runs to completion, and prints "sleep finished" as of r463973.

          Can you verifiy?

          Show
          Geir Magnusson Jr added a comment - The new test above in the comments also runs to completion, and prints "sleep finished" as of r463973. Can you verifiy?
          Hide
          Nikolay Kuznetsov added a comment -

          Attached tests examines interrupt behavior and hangs both on windows and linux. The old test for some reason hangs on windows only, the difference in behavior for windows and linux is that we use signals in linux stop implementation and probably that's why the first test passes on linux. Thus the patch is still valid, and I still whould like to apply it.

          In short, our interrupt scheme does not allow us notifying particular thread and thus we have to notify all of them.

          Show
          Nikolay Kuznetsov added a comment - Attached tests examines interrupt behavior and hangs both on windows and linux. The old test for some reason hangs on windows only, the difference in behavior for windows and linux is that we use signals in linux stop implementation and probably that's why the first test passes on linux. Thus the patch is still valid, and I still whould like to apply it . In short, our interrupt scheme does not allow us notifying particular thread and thus we have to notify all of them.
          Hide
          Geir Magnusson Jr added a comment -

          Test2.java doesn't even complile, does it?

          Show
          Geir Magnusson Jr added a comment - Test2.java doesn't even complile, does it?
          Hide
          Geir Magnusson Jr added a comment -

          r467322

          Ubuntu 6 - smoke, c-unit (usually), ~kernel

          Show
          Geir Magnusson Jr added a comment - r467322 Ubuntu 6 - smoke, c-unit (usually), ~kernel
          Hide
          Nikolay Kuznetsov added a comment -

          Geir Magnusson Jr [24/Oct/06 05:13 AM] Test2.java doesn't even complile, does it?

          I'm very sorry, I've modified original test to demonstrate problem with interrupts and missed several occurrences of Test remained while renaming new test to test2.

          Thanks a lot for your help.
          Nik.

          Show
          Nikolay Kuznetsov added a comment - Geir Magnusson Jr [24/Oct/06 05:13 AM] Test2.java doesn't even complile, does it? I'm very sorry, I've modified original test to demonstrate problem with interrupts and missed several occurrences of Test remained while renaming new test to test2. Thanks a lot for your help. Nik.

            People

            • Assignee:
              Geir Magnusson Jr
              Reporter:
              Nikolay Kuznetsov
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development