Harmony
  1. Harmony
  2. HARMONY-1974

[classlib][lini] unit test ThreadTest.test_enumerate$Ljava_lang_Thread() fails intermittently on drlvm

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Classlib
    • Labels:
      None
    • Patch Info:
      Patch Available

      Description

      The test fails rather often when running the whole luni module with the following message:

      Incorrect value returned2
      junit.framework.AssertionFailedError: Incorrect value returned2 at junit.framework.AssertionFailedError.<init>(AssertionFailedError.java:11) at org.apache.harmony.luni.tests.java.lang.ThreadTest.test_enumerate$Ljava_lang_Thread(ThreadTest.java:357) at java.lang.reflect.VMReflection.invokeMethod(Native Method) 0.000
      test_getContextClassLoader Success

      or

      Incorrect value returned3
      junit.framework.AssertionFailedError: Incorrect value returned3 at junit.framework.AssertionFailedError.<init>(AssertionFailedError.java:11) at org.apache.harmony.luni.tests.java.lang.ThreadTest.test_enumerate$Ljava_lang_Thread(ThreadTest.java:357) at java.lang.reflect.VMReflection.invokeMethod(Native Method) 0.000
      test_getContextClassLoader Success

      It seems that the test should wait until threads are started and then call the enumerate() method.

      1. ThreadTest.patch
        2 kB
        Elena Semukhina
      2. H-1974ThreadTest_2.patch
        5 kB
        Elena Semukhina

        Issue Links

          Activity

          Hide
          Elena Semukhina added a comment -

          I attached the patch which fixes the test.

          Show
          Elena Semukhina added a comment - I attached the patch which fixes the test.
          Hide
          Alexei Fedotov added a comment -

          Without a patch I see a different behavior for ThreadTest on r472083: the test hangs.

          Show
          Alexei Fedotov added a comment - Without a patch I see a different behavior for ThreadTest on r472083: the test hangs.
          Hide
          Nikolay Kuznetsov added a comment -

          ThreadTest indeed hangs for now. First off all j.l.Thread.stop(Throwable) misses isAlive check on thread, but after that fix, this test still hangs because of the problem described in HARMONY-1789. I'm working on both problems hopefully will provide combined fix soon.

          Show
          Nikolay Kuznetsov added a comment - ThreadTest indeed hangs for now. First off all j.l.Thread.stop(Throwable) misses isAlive check on thread, but after that fix, this test still hangs because of the problem described in HARMONY-1789 . I'm working on both problems hopefully will provide combined fix soon.
          Hide
          Nikolay Kuznetsov added a comment -

          Combined fix attached to HARMONY-1789 fixes the problem of hanging ThreadTest.

          Show
          Nikolay Kuznetsov added a comment - Combined fix attached to HARMONY-1789 fixes the problem of hanging ThreadTest.
          Hide
          Alexey Varlamov added a comment -

          Elena,

          IMHO the check you suggest:
          firstOne.start();
          + while (!firstOne.isAlive() && waitTime > 0) {

          is a bogus: accordingly to API specification, after method start() returns the target thread is already running. I.e. not necessarily it's run() is already being executed, but it is "alive" by definition. If this is not so for DRLVM, it must be a bug of DRLVM.

          Show
          Alexey Varlamov added a comment - Elena, IMHO the check you suggest: firstOne.start(); + while (!firstOne.isAlive() && waitTime > 0) { is a bogus: accordingly to API specification, after method start() returns the target thread is already running. I.e. not necessarily it's run() is already being executed, but it is "alive" by definition. If this is not so for DRLVM, it must be a bug of DRLVM.
          Hide
          Elena Semukhina added a comment -

          Alexey, I agree with you that isAlive() check is redundant here.

          I ran the test a few times until it failed and discovered that it failed when the number of threads exceeded the length of tarray used for threads enumeration.

          I think the test is incorrect because
          1) it assumes that there are no more than 10 threads in main ThreadGroup;
          2) it believes that the number of threads in the main ThreadGroup, once counted, cannot change later.

          I rewrote the test so that its algorithm was executed in a thread created in a special ThreadGroup. I this case we can be sure that there are no extra threads in this ThreadGroup and enumeration is quite predictable.

          Show
          Elena Semukhina added a comment - Alexey, I agree with you that isAlive() check is redundant here. I ran the test a few times until it failed and discovered that it failed when the number of threads exceeded the length of tarray used for threads enumeration. I think the test is incorrect because 1) it assumes that there are no more than 10 threads in main ThreadGroup; 2) it believes that the number of threads in the main ThreadGroup, once counted, cannot change later. I rewrote the test so that its algorithm was executed in a thread created in a special ThreadGroup. I this case we can be sure that there are no extra threads in this ThreadGroup and enumeration is quite predictable.
          Hide
          Elena Semukhina added a comment -

          I attached H-1974ThreadTest_2.patch which fixes the test.

          Show
          Elena Semukhina added a comment - I attached H-1974ThreadTest_2.patch which fixes the test.
          Hide
          Alexey Varlamov added a comment -

          Elena, I could not reproduce the failure (maybe due to intermittent nature), but the change you suggest looks reasonable.
          Applied at r475706, thank you!

          Show
          Alexey Varlamov added a comment - Elena, I could not reproduce the failure (maybe due to intermittent nature), but the change you suggest looks reasonable. Applied at r475706, thank you!

            People

            • Assignee:
              Alexey Varlamov
              Reporter:
              Elena Semukhina
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development