RFR [14] 8151678: com/sun/jndi/ldap/LdapTimeoutTest.java failed due to timeout on DeadServerNoTimeoutTest is incorrect

[Pavel Rappo]

Martin, 

May I add you to the list of people who reviewed this change? I mean the concurrency-related portion of the change, specifically in the test. Not the LDAP part.

> On 12 Sep 2019, at 16:21, Martin Buchholz <martinrb at google.com> wrote:
> 
> 
> 
> On Thu, Sep 12, 2019 at 4:36 AM Pavel Rappo <pavel.rappo at oracle.com> wrote:
> 
> I should have expressed myself more clear. I meant that the main thread and the thread created (and started) by `newStartedThread` (the test tread) do not wait for *each other* before they begin. This might be needed to make sure that they start at the same time (loosely speaking, of course). One might argue that the importance of this diminishes with the increase of the timeout that the main thread uses to wait for the test thread to die and the accuracy of measurements one agrees to tolerate.
> 
> 
> You want coordination?  We got coordination.
> 
> Here's some infrastructure that uses a threadsStarted latch:
> 
>     void assertThrowInterruptedExceptionWhenInterrupted(Action[] actions) {
>         int n = actions.length;
>         Future<?>[] futures = new Future<?>[n];
>         CountDownLatch threadsStarted = new CountDownLatch(n);
>         CountDownLatch done = new CountDownLatch(n);
> 
>         for (int i = 0; i < n; i++) {
>             Action action = actions[i];
>             futures[i] = cachedThreadPool.submit(new CheckedRunnable() {
>                 public void realRun() throws Throwable {
>                     threadsStarted.countDown();
>                     try {
>                         action.run();
>                         shouldThrow();
>                     }
>                     catch (InterruptedException success) {}
>                     catch (Throwable fail) { threadUnexpectedException(fail); }
>                     assertFalse(Thread.interrupted());
>                     done.countDown();
>                 }});
>         }
> 
>         await(threadsStarted);
>         assertEquals(n, done.getCount());
>         for (Future<?> future : futures) // Interrupt all the tasks
>             future.cancel(true);
>         await(done);
>     }
> 
> When communicating a request, just like in real life, you can't force others to do anything, so be politely Canadian and use "please" in the name of your latch
> 
>     /**
>      * await throws InterruptedException if interrupted before counted down
>      */
>     public void testAwait_Interruptible() {
>         final CountDownLatch l = new CountDownLatch(1);
>         final CountDownLatch pleaseInterrupt = new CountDownLatch(1);
>         Thread t = newStartedThread(new CheckedRunnable() {
>             public void realRun() throws InterruptedException {
>                 Thread.currentThread().interrupt();
>                 try {
>                     l.await();
>                     shouldThrow();
>                 } catch (InterruptedException success) {}
>                 assertFalse(Thread.interrupted());
> 
>                 pleaseInterrupt.countDown();
>                 try {
>                     l.await();
>                     shouldThrow();
>                 } catch (InterruptedException success) {}
>                 assertFalse(Thread.interrupted());
> 
>                 assertEquals(1, l.getCount());
>             }});
> 
>         await(pleaseInterrupt);
>         if (randomBoolean()) assertThreadBlocks(t, Thread.State.WAITING);
>         t.interrupt();
>         awaitTermination(t);
>     }
>