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

[Alex Menkov]

LGTM

--alex

On 03/17/2020 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
>          >      >
>          >      >
>          >      >
>          >
>          >
>          >
>          
>      
> 
>