8240711: TestJstatdPort.java failed due to "ExportException: Port already in use:"

[Daniil Titov]

Hi Serguei,

 

In this case tryToSetupJstatdProcess() on line 346 return null and the test  will try to find a new pair of ports and start jstatd process.

 

Best regards,

Daniil

 

 

From: "serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com>
Date: Monday, March 23, 2020 at 11:45 AM
To: Daniil Titov <daniil.x.titov at oracle.com>, Alex Menkov <alexey.menkov at oracle.com>, serviceability-dev <serviceability-dev at openjdk.java.net>
Subject: Re: RFR: 8240711: TestJstatdPort.java failed due to "ExportException: Port already in use:"

 

Hi Daniil,

It looks Okay in general.
But I've got a question.
 329             while (jstatdThread == null) {
 330                 if (!useDefaultPort) {
 331                     port = String.valueOf(Utils.getFreePort());
 332                 }
 333 
 334                 if (!useDefaultRmiPort) {
 335                     rmiPort = String.valueOf(Utils.getFreePort());
 336                 }
 337 
 338                 if (withExternalRegistry) {
 339                     Registry registry = startRegistry();
 340                     if (registry == null) {
 341                         // The port is already in use. Cancel and try with a new one.
 342                         continue;
 343                     }
 344                 }
 345 
 346                 jstatdThread = tryToSetupJstatdProcess();
 347             }

What is going to happen if all ports that we try are already in use?
Does the test report this situation?

Thanks,
Serguei


On 3/17/20 11:40, Daniil Titov wrote:
Hi Alex,
 
Please review a new version of the fix that removes the old version of the code that tried to handle the "port in use" case.
 
Testing: Mach5 tests for sun/tools/jstatd/  successfully passed 100 times.  Tier1-tier3 tests successfully passed. 
 
[1] http://cr.openjdk.java.net/~dtitov/8240711/webrev.02  
[2] https://bugs.openjdk.java.net/browse/JDK-8240711
 
Thanks,
Daniil
 
 
 
On 3/16/20, 5:38 PM, "Daniil Titov" <daniil.x.titov at oracle.com> wrote:
 
    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.menkov at 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.menkov at 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 at 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 at 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
        >      >
        >      >
        >      >
        >      
        > 
        > 
        
    
 
 



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20200323/07fa4ca4/attachment-0001.htm>