Re: RFR: 8240711: TestJstatdPort.java failed due to "ExportException: Port already in use:"
Hi Daniil,
Please, skip my concern.
As we privately concluded the chance to endlessly loop on getting
unused free port is very small.
I'm okay with the fix as it is.
Thanks,
Serguei
On 3/23/20 12:48, serguei.spit...@oracle.com wrote:
Hi Daniil,
My concern is that we have too many failures (and different
cases) with timeouts.
A big part of them happen when a connection is being
established.
Can these situation happen because servers are configured
incorrectly?
If so, is it better to detect it if possible instead of looping
in search for unused port endlessly?
I'm not sure if such scenario really happens in out testing.
What is your opinion?
It is interesting how many other tests behave the same as this
one.
If there are more such tests then it is better to file a
separate bug or enhancement.
Thanks,
Serguei
On 3/23/20 12:32, Daniil Titov wrote:
Hi Serguei,
I don’t think that in any real
environment the loop could not be able to find the pair of
free ports before it is killed by JTREG due to timeout. But
if you think that we need to limit the number of attempts
here I could create a new issue for that.
Thanks!
--Daniil
From: "serguei.spit...@oracle.com"
Date: Monday, March 23, 2020 at 12:13 PM
To: Daniil Titov ,
Alex Menkov ,
serviceability-dev
Subject: Re: 8240711: TestJstatdPort.java failed
due to "ExportException: Port already in use:"
On 3/23/20 12:05, Daniil Titov wrote:
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.
I understand this.
My question if this loop can be endless.
What happens if there is no new pair of ports that we did
not check yet?
Do we fail with a timeout in such a case?
If so, would it better to report that unused free port was
not found?
Is it possible to detect this situation?
Thanks,
Serguei
Best regards,
Daniil
From: "serguei.spit...@oracle.com"
Date: Monday, March 23, 2020 at 11:45 AM
To: Daniil Titov ,
Alex Menkov ,
serviceability-dev
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:
Re: RFR: 8240711: TestJstatdPort.java failed due to "ExportException: Port already in use:"
Hi Daniil,
My concern is that we have too many failures (and different cases)
with timeouts.
A big part of them happen when a connection is being established.
Can these situation happen because servers are configured
incorrectly?
If so, is it better to detect it if possible instead of looping in
search for unused port endlessly?
I'm not sure if such scenario really happens in out testing.
What is your opinion?
It is interesting how many other tests behave the same as this
one.
If there are more such tests then it is better to file a separate
bug or enhancement.
Thanks,
Serguei
On 3/23/20 12:32, Daniil Titov wrote:
Hi Serguei,
I don’t think that in any real environment
the loop could not be able to find the pair of free ports
before it is killed by JTREG due to timeout. But if you think
that we need to limit the number of attempts here I could
create a new issue for that.
Thanks!
--Daniil
From: "serguei.spit...@oracle.com"
Date: Monday, March 23, 2020 at 12:13 PM
To: Daniil Titov
, Alex Menkov
, serviceability-dev
Subject: Re: 8240711: TestJstatdPort.java failed
due to "ExportException: Port already in use:"
On 3/23/20 12:05, Daniil Titov wrote:
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.
I understand this.
My question if this loop can be endless.
What happens if there is no new pair of ports that we did not
check yet?
Do we fail with a timeout in such a case?
If so, would it better to report that unused free port was not
found?
Is it possible to detect this situation?
Thanks,
Serguei
Best regards,
Daniil
From: "serguei.spit...@oracle.com"
Date: Monday, March 23, 2020 at 11:45 AM
To: Daniil Titov ,
Alex Menkov ,
serviceability-dev
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
Re: RFR: 8240711: TestJstatdPort.java failed due to "ExportException: Port already in use:"
Hi Serguei,
I don’t think that in any real environment the loop could not be able to find
the pair of free ports before it is killed by JTREG due to timeout. But if you
think that we need to limit the number of attempts here I could create a new
issue for that.
Thanks!
--Daniil
From: "serguei.spit...@oracle.com"
Date: Monday, March 23, 2020 at 12:13 PM
To: Daniil Titov , Alex Menkov
, serviceability-dev
Subject: Re: 8240711: TestJstatdPort.java failed due to "ExportException: Port
already in use:"
On 3/23/20 12:05, Daniil Titov wrote:
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.
I understand this.
My question if this loop can be endless.
What happens if there is no new pair of ports that we did not check yet?
Do we fail with a timeout in such a case?
If so, would it better to report that unused free port was not found?
Is it possible to detect this situation?
Thanks,
Serguei
Best regards,
Daniil
From: "serguei.spit...@oracle.com"
Date: Monday, March 23, 2020 at 11:45 AM
To: Daniil Titov , Alex Menkov
, serviceability-dev
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" 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" 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" 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.startProce
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" 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" 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" 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]
Re: RFR: 8240711: TestJstatdPort.java failed due to "ExportException: Port already in use:"
Hi Alex, Thank you for reviewing this change. Best regards, Daniil On 3/17/20, 11:58 AM, "Alex Menkov" wrote: 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" 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" 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" 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 > > > > > > > >
Re: RFR: 8240711: TestJstatdPort.java failed due to "ExportException: Port already in use:"
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" 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" 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" 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 > > > > > > > > >
Re: RFR: 8240711: TestJstatdPort.java failed due to "ExportException: Port already in use:"
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" 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" 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" 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 > > > > > > > > >
Re: RFR: 8240711: TestJstatdPort.java failed due to "ExportException: Port already in use:"
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" 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" 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 > > > > > > > > >
Re: RFR: 8240711: TestJstatdPort.java failed due to "ExportException: Port already in use:"
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" 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 > > >
Re: RFR: 8240711: TestJstatdPort.java failed due to "ExportException: Port already in use:"
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" 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 > > >
Re: RFR: 8240711: TestJstatdPort.java failed due to "ExportException: Port already in use:"
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
