Hi Alex,

Yes,  I did test the change by modifying  the test to use the RMI port that is 
already in use
( the stack trace in the original email was exact from this changed test) and 
then ensured that with the fix 
the such issue is properly handled.

I will send a new version of the webrev that removes the old version of the 
code that tried to handle the "port in use" case.

Thanks!

Best regards,
Daniil




On 3/16/20, 4:47 PM, "Alex Menkov" <alexey.men...@oracle.com> wrote:

    I don't agree.
    The code handles exact the same "port in use" case for the same tool.
    So it either works or doesn't.
    And have 2 code blocks which suppose to do the same makes the code messy.
    BTW did you tested the change (I mean craft the test to get "port in 
    use" error)?
    
    --alex
    
    On 03/16/2020 16:17, Daniil Titov wrote:
    > Resending with the corrected subject ...
    > 
    > Hi Alex,
    > 
    > Yes, you are right, class JstatdTest has the code that is supposed to 
handle the "port in use"
    > case but at least for this specific test  
(sun/tools/jstatd/TestJstatdPort.java) it doesn't work.
    > 
    > Since there are multiple tests in sun/tools/jstatd/* folder that use this 
class and different ports
    > might be subject to the "port in use" error and taking into account that 
it's hard to reproduce such case
    > I found it safer to leave the original code and just augment it with what 
was missing for this specific
    > case rather than completely replacing it.
    > 
    > Best regards,
    > Daniil
    > 
    > On 3/16/20, 4:02 PM, "Alex Menkov" <alexey.men...@oracle.com> wrote:
    > 
    >      Hi Daniil,
    >      
    >      Looks like the test is supposed to handle "port in use" issue (see 
lines
    >      103-114).
    >      I suppose in case "port in use" jstatd exits, but
    >      ProcessTools.startProcess() continue to wait for "jstatd started" 
message.
    >      
    >      --alex
    >      
    >      On 03/16/2020 12:00, Daniil Titov wrote:
    >      > Please review the change [1] that fixes the intermittent failure 
of the test.
    >      >
    >      > The problem here is that if the RMI port is in use than the test 
keep waiting for "jstatd started (bound to " to appear in the process output 
and in this case
    >      > It doesn't happen.
    >      >
    >      >    at 
java.util.concurrent.CountDownLatch.await(java.base@15-internal/CountDownLatch.java:232)
    >      >    at 
jdk.test.lib.process.ProcessTools.startProcess(ProcessTools.java:205)
    >      >    at 
jdk.test.lib.process.ProcessTools.startProcess(ProcessTools.java:133)
    >      >    at 
jdk.test.lib.process.ProcessTools.startProcess(ProcessTools.java:254)
    >      >    at 
jdk.test.lib.thread.ProcessThread$ProcessRunnable.xrun(ProcessThread.java:153)
    >      >    at jdk.test.lib.thread.XRun.run(XRun.java:40)
    >      >    at java.lang.Thread.run(java.base@15-internal/Thread.java:832)
    >      >    at jdk.test.lib.thread.TestThread.run(TestThread.java:123)
    >      >
    >      > Testing: Mach5 tests for sun/tools/jstatd/ successfully passed.  
Tier1-tier3 tests are still in progress.
    >      >
    >      > [1] http://cr.openjdk.java.net/~dtitov/8240711/webrev.01/
    >      > [2] https://bugs.openjdk.java.net/browse/JDK-8240711
    >      >
    >      >
    >      > Thank you,
    >      > Daniil
    >      >
    >      >
    >      >
    >      
    > 
    > 
    


Reply via email to