This looks great to me. Thanks for taking a look at it. -Rob
On 12/09/19 12:36, Pavel Rappo wrote: > > On 12 Sep 2019, at 03:54, Martin Buchholz <marti...@google.com> wrote: > > > > We're mostly in agreement. > > Good to hear and I agree! > > > On Wed, Sep 11, 2019 at 11:02 AM Pavel Rappo <pavel.ra...@oracle.com > > <mailto:pavel.ra...@oracle.com>> wrote: > > > > > > a. is not using any synchronization aid to make sure both threads wait for > > each other (probably, the timeout value accommodates that) > > > > Thread.join is a synchronization aid! But there's no shortage of others. > > We typically use a CountDownLatch to synchronize with another thread. > > > 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. > > I'm seeing differences in our timeout-measuring routines as just a difference > in tastes. Since a particular behavior here is not guaranteed anyway (as it's > not a hard real-time system), all we can hope for in both cases is adequate > timeout values and useful diagnostic output. > > Thanks for patiently staying with this thread for so long, Martin. > > > *** > > > Here's the updated version of the RFR based on the discussion so far: > > http://cr.openjdk.java.net/~prappo/8151678/webrev.02/ > > For the reviewers. I totally forgot to explain why there's a multiplier of 2 > in some of the timeout calculations. The reason is the current behavior of > InitialDirContext. The supplied connect timeout seems to be used twice. Once > for making the actual TCP connection [1] and the second time while waiting > for the server to respond to the BIND message [2]. Thus, the total time spent > in that InitialDirContext ctor may be twice the expected. I believe it's a > bug, but the bug that is not related to the issue in question. The current > issue (8151678) is about intermittent failures of LdapTimeoutTest. > > -Pavel > > --------------------------------------------------- > [1] > http://hg.openjdk.java.net/jdk/jdk/file/79186d82463e/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java#l296, > > http://hg.openjdk.java.net/jdk/jdk/file/79186d82463e/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java#l320 > [2] > http://hg.openjdk.java.net/jdk/jdk/file/79186d82463e/src/java.naming/share/classes/com/sun/jndi/ldap/LdapClient.java#l154, > > http://hg.openjdk.java.net/jdk/jdk/file/79186d82463e/src/java.naming/share/classes/com/sun/jndi/ldap/LdapClient.java#l365 >