Re: 8221303: sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java fails due to java.rmi.server.ExportException: Port already in use

2019-07-18 Thread Daniil Titov
Thank you, Chris, Severin, and Alex, for reviewing this change!

Best regards,
Daniil

On 7/18/19, 10:44 AM, "Alex Menkov"  wrote:

+1

--alex

On 07/18/2019 08:27, Chris Plummer wrote:
> Hi Daniil,
> 
> Looks good.
> 
> Chris
> 
> On 7/17/19 6:26 PM, Daniil Titov wrote:
>> Hi Chris,
>>
>> Yes, I added output.reportDiagnosticSummary() in webrev-01, but 
>> removed it
>> In webrev-02, and later restored it in webrev-03.
>>
>> When the test runs of retries output.getExitValue() is set to 1 
>> (COMMUNICATION_ERROR_EXIT_VAL)
>>   (the "exitValue =1 " in the previous email).
>>
>> Please review a new version of the fix that has an explicit error 
>> message printed
>> if the test runs out of retries (line 168).
>>
>> Webrev: http://cr.openjdk.java.net/~dtitov/8221303/webrev.04/
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8221303
>>
>> Thanks!
>> --Daniil
>>
>> On 7/17/19, 5:33 PM, "Chris Plummer"  wrote:
>>
>>  Hi Daniil,
>>  I'm confused now. I mentioned output.reportDiagnosticSummary() 
>> because I
>>  thought I saw it in your webrev-02. Now I don't. Maybe I had glanced
>>  back at webrev-01 and saw it there. One issue with exceptions in the
>>  output that are not considered errors is that if later there is an
>>  error, mdash wills show the exception as one of (possible multiple)
>>  reasons for the failure, so it's good to avoid if possible. Looks 
>> like
>>  what you have now is ok.
>>  I have a question about what happens if you run out of retries. 
>> What is
>>  output.getExitValue() set to? Also, I think you should print an an
>>  explicit error message indicating you've run out of retries.
>>  thanks,
>>  Chris
>>  On 7/17/19 4:36 PM, Daniil Titov wrote:
>>  > Hi Chris,
>>  >
>>  > output.reportDiagnosticSummary() prints the output from the 
>> process (both stdout and stderr) and the exit value to the test's stderr.
>>  > In case if the port is already in use it prints the following:
>>  >
>>  > stdout: [];
>>  >   stderr: [Error: JMX connector server communication error: 
>> service:jmx:rmi://127.0.0.1:9101
>>  > jdk.internal.agent.AgentConfigurationError: 
>> java.rmi.server.ExportException: Port already in use: 9101; nested 
>> exception is:
>>  > java.net.BindException: Address already in use
>>  > at 
>> 
jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.exportMBeanServer(ConnectorBootstrap.java:820)
 
>>
>>  > at 
>> 
jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.startRemoteConnectorServer(ConnectorBootstrap.java:479)
 
>>
>>  > at 
>> jdk.management.agent/jdk.internal.agent.Agent.startAgent(Agent.java:447)
>>  > at 
>> jdk.management.agent/jdk.internal.agent.Agent.startAgent(Agent.java:599)
>>  > Caused by: java.rmi.server.ExportException: Port already in 
>> use: 9101; nested exception is:
>>  > < I skipped the rest>
>>  > ]
>>  >   exitValue = 1
>>  >
>>  > It makes sense to have it called if the test fails, otherwise 
>> this information would be missed in the test output.
>>  > Please review the new version of the fix that has the call to 
>> output.reportDiagnosticSummary() restored.
>>  >
>>  > Thanks!
>>  >
>>  > Webrev: http://cr.openjdk.java.net/~dtitov/8221303/webrev.03/
>>  > Bug: https://bugs.openjdk.java.net/browse/JDK-8221303
>>  >
>>  > -Daniil
>>  >
>>  > On 7/17/19, 3:58 PM, "Chris Plummer" 
>>  wrote:
>>  >
>>  >  What does output.reportDiagnosticSummary() print out then 
>> the port is
>>  >  already in use, and have you made this happen with your 
>> fixes in place?
>>  >
>>  >  Chris
>>  >
>>  >  On 7/17/19 3:20 PM, Daniil Titov wrote:
>>  >  > Hi Chris and Alex,
>>  >  >
>>  >  > Please review a new version of the fix that moves the 
>> diagnostic output for the test failure to run()
>>  >  > method after the number of retry attempts is exceeded. 
>> It also includes other corrections that
>>  >  > you suggested.
>>  >  >
>>  >  > Thanks!
>>  >  >
>>  >  > Webrev: 
>> http://cr.openjdk.java.net/~dtitov/8221303/webrev.02/
>>  >  > Bug: https://bugs.openjdk.java.net/browse/JDK-8221303
>>  >  >
>>  >  > --Best regards,
>>  >  > Daniil
>>  >  >
>>  >  >
>>  >  > On 7/17/19, 1:47 PM, "Chris 

Re: 8221303: sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java fails due to java.rmi.server.ExportException: Port already in use

2019-07-18 Thread Alex Menkov

+1

--alex

On 07/18/2019 08:27, Chris Plummer wrote:

Hi Daniil,

Looks good.

Chris

On 7/17/19 6:26 PM, Daniil Titov wrote:

Hi Chris,

Yes, I added output.reportDiagnosticSummary() in webrev-01, but 
removed it

In webrev-02, and later restored it in webrev-03.

When the test runs of retries output.getExitValue() is set to 1 
(COMMUNICATION_ERROR_EXIT_VAL)

  (the "exitValue =1 " in the previous email).

Please review a new version of the fix that has an explicit error 
message printed

if the test runs out of retries (line 168).

Webrev: http://cr.openjdk.java.net/~dtitov/8221303/webrev.04/
Bug: https://bugs.openjdk.java.net/browse/JDK-8221303

Thanks!
--Daniil

On 7/17/19, 5:33 PM, "Chris Plummer"  wrote:

 Hi Daniil,
 I'm confused now. I mentioned output.reportDiagnosticSummary() 
because I

 thought I saw it in your webrev-02. Now I don't. Maybe I had glanced
 back at webrev-01 and saw it there. One issue with exceptions in the
 output that are not considered errors is that if later there is an
 error, mdash wills show the exception as one of (possible multiple)
 reasons for the failure, so it's good to avoid if possible. Looks 
like

 what you have now is ok.
 I have a question about what happens if you run out of retries. 
What is

 output.getExitValue() set to? Also, I think you should print an an
 explicit error message indicating you've run out of retries.
 thanks,
 Chris
 On 7/17/19 4:36 PM, Daniil Titov wrote:
 > Hi Chris,
 >
 > output.reportDiagnosticSummary() prints the output from the 
process (both stdout and stderr) and the exit value to the test's stderr.

 > In case if the port is already in use it prints the following:
 >
 > stdout: [];
 >   stderr: [Error: JMX connector server communication error: 
service:jmx:rmi://127.0.0.1:9101
 > jdk.internal.agent.AgentConfigurationError: 
java.rmi.server.ExportException: Port already in use: 9101; nested 
exception is:

 > java.net.BindException: Address already in use
 > at 
jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.exportMBeanServer(ConnectorBootstrap.java:820) 

 > at 
jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.startRemoteConnectorServer(ConnectorBootstrap.java:479) 

 > at 
jdk.management.agent/jdk.internal.agent.Agent.startAgent(Agent.java:447)
 > at 
jdk.management.agent/jdk.internal.agent.Agent.startAgent(Agent.java:599)
 > Caused by: java.rmi.server.ExportException: Port already in 
use: 9101; nested exception is:

 > < I skipped the rest>
 > ]
 >   exitValue = 1
 >
 > It makes sense to have it called if the test fails, otherwise 
this information would be missed in the test output.
 > Please review the new version of the fix that has the call to 
output.reportDiagnosticSummary() restored.

 >
 > Thanks!
 >
 > Webrev: http://cr.openjdk.java.net/~dtitov/8221303/webrev.03/
 > Bug: https://bugs.openjdk.java.net/browse/JDK-8221303
 >
 > -Daniil
 >
 > On 7/17/19, 3:58 PM, "Chris Plummer" 
 wrote:

 >
 >  What does output.reportDiagnosticSummary() print out then 
the port is
 >  already in use, and have you made this happen with your 
fixes in place?

 >
 >  Chris
 >
 >  On 7/17/19 3:20 PM, Daniil Titov wrote:
 >  > Hi Chris and Alex,
 >  >
 >  > Please review a new version of the fix that moves the 
diagnostic output for the test failure to run()
 >  > method after the number of retry attempts is exceeded. 
It also includes other corrections that

 >  > you suggested.
 >  >
 >  > Thanks!
 >  >
 >  > Webrev: 
http://cr.openjdk.java.net/~dtitov/8221303/webrev.02/

 >  > Bug: https://bugs.openjdk.java.net/browse/JDK-8221303
 >  >
 >  > --Best regards,
 >  > Daniil
 >  >
 >  >
 >  > On 7/17/19, 1:47 PM, "Chris Plummer" 
 wrote:

 >  >
 >  >  Hi Daniil,
 >  >
 >  >  I think you can remove "Ok" from the following 
message:

 >  >
 >  >    237 System.out.println("DEBUG: OK. Spawned java 
process terminated

 >  >  with expected exit code of "
 >  >    238 + 
STOP_PROCESS_EXIT_VAL);

 >  >
 >  >  It's somewhat misleading since the test can still 
fail. I think the

 >  >  following is also misleading:
 >  >
 >  >    249 if (testFailed) {
 >  >    250 
output.reportDiagnosticSummary();
 >  >    251 if 
(output.getStderr().contains("Port already in

 >  >  use")) {
 >  >    252 // Need to retry
 >  >    253 return true;
 >  >    254   

Re: 8221303: sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java fails due to java.rmi.server.ExportException: Port already in use

2019-07-18 Thread Chris Plummer

Hi Daniil,

Looks good.

Chris

On 7/17/19 6:26 PM, Daniil Titov wrote:

Hi Chris,

Yes, I added output.reportDiagnosticSummary() in webrev-01, but removed it
In webrev-02, and later restored it in webrev-03.

When the test runs of retries output.getExitValue() is set to 1 
(COMMUNICATION_ERROR_EXIT_VAL)
  (the "exitValue =1 " in the previous email).

Please review a new version of the fix that has an explicit error message 
printed
if the test runs out of retries (line 168).

Webrev: http://cr.openjdk.java.net/~dtitov/8221303/webrev.04/
Bug: https://bugs.openjdk.java.net/browse/JDK-8221303

Thanks!
--Daniil

On 7/17/19, 5:33 PM, "Chris Plummer"  wrote:

 Hi Daniil,
 
 I'm confused now. I mentioned output.reportDiagnosticSummary() because I

 thought I saw it in your webrev-02. Now I don't. Maybe I had glanced
 back at webrev-01 and saw it there. One issue with exceptions in the
 output that are not considered errors is that if later there is an
 error, mdash wills show the exception as one of (possible multiple)
 reasons for the failure, so it's good to avoid if possible. Looks like
 what you have now is ok.
 
 I have a question about what happens if you run out of retries. What is

 output.getExitValue() set to? Also, I think you should print an an
 explicit error message indicating you've run out of retries.
 
 thanks,
 
 Chris
 
 On 7/17/19 4:36 PM, Daniil Titov wrote:

 > Hi Chris,
 >
 > output.reportDiagnosticSummary() prints the output from the process 
(both stdout and stderr) and the exit value to the test's stderr.
 > In case if the port is already in use it prints the following:
 >
 > stdout: [];
 >   stderr: [Error: JMX connector server communication error: 
service:jmx:rmi://127.0.0.1:9101
 > jdk.internal.agent.AgentConfigurationError: 
java.rmi.server.ExportException: Port already in use: 9101; nested exception is:
 >   java.net.BindException: Address already in use
 >   at 
jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.exportMBeanServer(ConnectorBootstrap.java:820)
 >   at 
jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.startRemoteConnectorServer(ConnectorBootstrap.java:479)
 >   at 
jdk.management.agent/jdk.internal.agent.Agent.startAgent(Agent.java:447)
 >   at 
jdk.management.agent/jdk.internal.agent.Agent.startAgent(Agent.java:599)
 > Caused by: java.rmi.server.ExportException: Port already in use: 9101; 
nested exception is:
 > < I skipped the rest>
 > ]
 >   exitValue = 1
 >
 > It makes sense to have it called if the test fails, otherwise this 
information would be missed in the test output.
 > Please review the new version of the fix that has the call to 
output.reportDiagnosticSummary() restored.
 >
 > Thanks!
 >
 > Webrev: http://cr.openjdk.java.net/~dtitov/8221303/webrev.03/
 > Bug: https://bugs.openjdk.java.net/browse/JDK-8221303
 >
 > -Daniil
 >
 > On 7/17/19, 3:58 PM, "Chris Plummer"  wrote:
 >
 >  What does output.reportDiagnosticSummary() print out then the port 
is
 >  already in use, and have you made this happen with your fixes in 
place?
 >
 >  Chris
 >
 >  On 7/17/19 3:20 PM, Daniil Titov wrote:
 >  > Hi Chris and Alex,
 >  >
 >  > Please review a new version of the fix that moves the diagnostic 
output for the test failure to run()
 >  > method after the number of retry attempts is exceeded. It also 
includes other corrections that
 >  > you suggested.
 >  >
 >  > Thanks!
 >  >
 >  > Webrev: http://cr.openjdk.java.net/~dtitov/8221303/webrev.02/
 >  > Bug: https://bugs.openjdk.java.net/browse/JDK-8221303
 >  >
 >  > --Best regards,
 >  > Daniil
 >  >
 >  >
 >  > On 7/17/19, 1:47 PM, "Chris Plummer"  
wrote:
 >  >
 >  >  Hi Daniil,
 >  >
 >  >  I think you can remove "Ok" from the following message:
 >  >
 >  >237 System.out.println("DEBUG: OK. Spawned java process 
terminated
 >  >  with expected exit code of "
 >  >238 + STOP_PROCESS_EXIT_VAL);
 >  >
 >  >  It's somewhat misleading since the test can still fail. I 
think the
 >  >  following is also misleading:
 >  >
 >  >249 if (testFailed) {
 >  >250 output.reportDiagnosticSummary();
 >  >251 if (output.getStderr().contains("Port 
already in
 >  >  use")) {
 >  >252 // Need to retry
 >  >253 return true;
 >  >254 }
 >  >255  

Re: 8221303: sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java fails due to java.rmi.server.ExportException: Port already in use

2019-07-18 Thread Severin Gehwolf
Hi Daniil,

On Wed, 2019-07-17 at 18:26 -0700, Daniil Titov wrote:
> Hi Chris,
> 
> Yes, I added output.reportDiagnosticSummary() in webrev-01, but removed it 
> In webrev-02, and later restored it in webrev-03.
> 
> When the test runs of retries output.getExitValue() is set to 1 
> (COMMUNICATION_ERROR_EXIT_VAL) 
>  (the "exitValue =1 " in the previous email).
> 
> Please review a new version of the fix that has an explicit error message 
> printed
> if the test runs out of retries (line 168).
> 
> Webrev: http://cr.openjdk.java.net/~dtitov/8221303/webrev.04/ 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8221303

This looks reasonable to me. Thanks for fixing it!

Thanks,
Severin

> Thanks!
> --Daniil
> 
> On 7/17/19, 5:33 PM, "Chris Plummer" 
> wrote:
> 
> Hi Daniil,
> 
> I'm confused now. I mentioned output.reportDiagnosticSummary()
> because I 
> thought I saw it in your webrev-02. Now I don't. Maybe I had
> glanced 
> back at webrev-01 and saw it there. One issue with exceptions in
> the 
> output that are not considered errors is that if later there is
> an 
> error, mdash wills show the exception as one of (possible
> multiple) 
> reasons for the failure, so it's good to avoid if possible. Looks
> like 
> what you have now is ok.
> 
> I have a question about what happens if you run out of retries.
> What is 
> output.getExitValue() set to? Also, I think you should print an
> an 
> explicit error message indicating you've run out of retries.
> 
> thanks,
> 
> Chris
> 
> On 7/17/19 4:36 PM, Daniil Titov wrote:
> > Hi Chris,
> >
> > output.reportDiagnosticSummary() prints the output from the
> process (both stdout and stderr) and the exit value to the test's
> stderr.
> > In case if the port is already in use it prints the following:
> >
> > stdout: [];
> >   stderr: [Error: JMX connector server communication error:
> service:jmx:rmi://127.0.0.1:9101
> > jdk.internal.agent.AgentConfigurationError:
> java.rmi.server.ExportException: Port already in use: 9101; nested
> exception is:
> > java.net.BindException: Address already in use
> > at
> jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.expo
> rtMBeanServer(ConnectorBootstrap.java:820)
> > at
> jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.star
> tRemoteConnectorServer(ConnectorBootstrap.java:479)
> > at
> jdk.management.agent/jdk.internal.agent.Agent.startAgent(Agent.java:4
> 47)
> > at
> jdk.management.agent/jdk.internal.agent.Agent.startAgent(Agent.java:5
> 99)
> > Caused by: java.rmi.server.ExportException: Port already in
> use: 9101; nested exception is:
> > < I skipped the rest>
> > ]
> >   exitValue = 1
> >
> > It makes sense to have it called if the test fails, otherwise
> this information would be missed in the test output.
> > Please review the new version of the fix that has the call to
> output.reportDiagnosticSummary() restored.
> >
> > Thanks!
> >
> > Webrev: http://cr.openjdk.java.net/~dtitov/8221303/webrev.03/
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8221303
> >
> > -Daniil
> >
> > On 7/17/19, 3:58 PM, "Chris Plummer"  > wrote:
> >
> >  What does output.reportDiagnosticSummary() print out then
> the port is
> >  already in use, and have you made this happen with your
> fixes in place?
> >  
> >  Chris
> >  
> >  On 7/17/19 3:20 PM, Daniil Titov wrote:
> >  > Hi Chris and Alex,
> >  >
> >  > Please review a new version of the fix that moves the
> diagnostic output for the test failure to run()
> >  > method after the number of retry attempts is exceeded.
> It also includes other corrections that
> >  > you suggested.
> >  >
> >  > Thanks!
> >  >
> >  > Webrev: 
> http://cr.openjdk.java.net/~dtitov/8221303/webrev.02/
> >  > Bug: https://bugs.openjdk.java.net/browse/JDK-8221303
> >  >
> >  > --Best regards,
> >  > Daniil
> >  >
> >  >
> >  > On 7/17/19, 1:47 PM, "Chris Plummer" <
> chris.plum...@oracle.com> wrote:
> >  >
> >  >  Hi Daniil,
> >  >
> >  >  I think you can remove "Ok" from the following
> message:
> >  >
> >  >237 System.out.println("DEBUG: OK. Spawned java
> process terminated
> >  >  with expected exit code of "
> >  >238 +
> STOP_PROCESS_EXIT_VAL);
> >  >
> >  >  It's somewhat misleading since the test can still
> fail. I think the
> >  >  following is also misleading:
> >  >
> >  >249 if (testFailed) {
> >  >250 output.reportDiagnosticSummar
> y();
> 

Re: 8221303: sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java fails due to java.rmi.server.ExportException: Port already in use

2019-07-17 Thread Daniil Titov
Hi Chris,

Yes, I added output.reportDiagnosticSummary() in webrev-01, but removed it 
In webrev-02, and later restored it in webrev-03.

When the test runs of retries output.getExitValue() is set to 1 
(COMMUNICATION_ERROR_EXIT_VAL) 
 (the "exitValue =1 " in the previous email).

Please review a new version of the fix that has an explicit error message 
printed
if the test runs out of retries (line 168).

Webrev: http://cr.openjdk.java.net/~dtitov/8221303/webrev.04/ 
Bug: https://bugs.openjdk.java.net/browse/JDK-8221303

Thanks!
--Daniil

On 7/17/19, 5:33 PM, "Chris Plummer"  wrote:

Hi Daniil,

I'm confused now. I mentioned output.reportDiagnosticSummary() because I 
thought I saw it in your webrev-02. Now I don't. Maybe I had glanced 
back at webrev-01 and saw it there. One issue with exceptions in the 
output that are not considered errors is that if later there is an 
error, mdash wills show the exception as one of (possible multiple) 
reasons for the failure, so it's good to avoid if possible. Looks like 
what you have now is ok.

I have a question about what happens if you run out of retries. What is 
output.getExitValue() set to? Also, I think you should print an an 
explicit error message indicating you've run out of retries.

thanks,

Chris

On 7/17/19 4:36 PM, Daniil Titov wrote:
> Hi Chris,
>
> output.reportDiagnosticSummary() prints the output from the process (both 
stdout and stderr) and the exit value to the test's stderr.
> In case if the port is already in use it prints the following:
>
> stdout: [];
>   stderr: [Error: JMX connector server communication error: 
service:jmx:rmi://127.0.0.1:9101
> jdk.internal.agent.AgentConfigurationError: 
java.rmi.server.ExportException: Port already in use: 9101; nested exception is:
>   java.net.BindException: Address already in use
>   at 
jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.exportMBeanServer(ConnectorBootstrap.java:820)
>   at 
jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.startRemoteConnectorServer(ConnectorBootstrap.java:479)
>   at 
jdk.management.agent/jdk.internal.agent.Agent.startAgent(Agent.java:447)
>   at 
jdk.management.agent/jdk.internal.agent.Agent.startAgent(Agent.java:599)
> Caused by: java.rmi.server.ExportException: Port already in use: 9101; 
nested exception is:
> < I skipped the rest>
> ]
>   exitValue = 1
>
> It makes sense to have it called if the test fails, otherwise this 
information would be missed in the test output.
> Please review the new version of the fix that has the call to 
output.reportDiagnosticSummary() restored.
>
> Thanks!
>
> Webrev: http://cr.openjdk.java.net/~dtitov/8221303/webrev.03/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8221303
>
> -Daniil
>
> On 7/17/19, 3:58 PM, "Chris Plummer"  wrote:
>
>  What does output.reportDiagnosticSummary() print out then the port is
>  already in use, and have you made this happen with your fixes in 
place?
>  
>  Chris
>  
>  On 7/17/19 3:20 PM, Daniil Titov wrote:
>  > Hi Chris and Alex,
>  >
>  > Please review a new version of the fix that moves the diagnostic 
output for the test failure to run()
>  > method after the number of retry attempts is exceeded. It also 
includes other corrections that
>  > you suggested.
>  >
>  > Thanks!
>  >
>  > Webrev: http://cr.openjdk.java.net/~dtitov/8221303/webrev.02/
>  > Bug: https://bugs.openjdk.java.net/browse/JDK-8221303
>  >
>  > --Best regards,
>  > Daniil
>  >
>  >
>  > On 7/17/19, 1:47 PM, "Chris Plummer"  
wrote:
>  >
>  >  Hi Daniil,
>  >
>  >  I think you can remove "Ok" from the following message:
>  >
>  >237 System.out.println("DEBUG: OK. Spawned java process 
terminated
>  >  with expected exit code of "
>  >238 + STOP_PROCESS_EXIT_VAL);
>  >
>  >  It's somewhat misleading since the test can still fail. I 
think the
>  >  following is also misleading:
>  >
>  >249 if (testFailed) {
>  >250 output.reportDiagnosticSummary();
>  >251 if (output.getStderr().contains("Port 
already in
>  >  use")) {
>  >252 // Need to retry
>  >253 return true;
>  >254 }
>  >255 }
>  >
>  >  The test can still pass after this happens, right? And I 
think there are
>  >  other "Test FAILURE" error message 

Re: 8221303: sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java fails due to java.rmi.server.ExportException: Port already in use

2019-07-17 Thread Chris Plummer

Hi Daniil,

I'm confused now. I mentioned output.reportDiagnosticSummary() because I 
thought I saw it in your webrev-02. Now I don't. Maybe I had glanced 
back at webrev-01 and saw it there. One issue with exceptions in the 
output that are not considered errors is that if later there is an 
error, mdash wills show the exception as one of (possible multiple) 
reasons for the failure, so it's good to avoid if possible. Looks like 
what you have now is ok.


I have a question about what happens if you run out of retries. What is 
output.getExitValue() set to? Also, I think you should print an an 
explicit error message indicating you've run out of retries.


thanks,

Chris

On 7/17/19 4:36 PM, Daniil Titov wrote:

Hi Chris,

output.reportDiagnosticSummary() prints the output from the process (both 
stdout and stderr) and the exit value to the test's stderr.
In case if the port is already in use it prints the following:

stdout: [];
  stderr: [Error: JMX connector server communication error: 
service:jmx:rmi://127.0.0.1:9101
jdk.internal.agent.AgentConfigurationError: java.rmi.server.ExportException: 
Port already in use: 9101; nested exception is:
java.net.BindException: Address already in use
at 
jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.exportMBeanServer(ConnectorBootstrap.java:820)
at 
jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.startRemoteConnectorServer(ConnectorBootstrap.java:479)
at 
jdk.management.agent/jdk.internal.agent.Agent.startAgent(Agent.java:447)
at 
jdk.management.agent/jdk.internal.agent.Agent.startAgent(Agent.java:599)
Caused by: java.rmi.server.ExportException: Port already in use: 9101; nested 
exception is:
< I skipped the rest>
]
  exitValue = 1

It makes sense to have it called if the test fails, otherwise this information 
would be missed in the test output.
Please review the new version of the fix that has the call to 
output.reportDiagnosticSummary() restored.

Thanks!

Webrev: http://cr.openjdk.java.net/~dtitov/8221303/webrev.03/
Bug: https://bugs.openjdk.java.net/browse/JDK-8221303

-Daniil

On 7/17/19, 3:58 PM, "Chris Plummer"  wrote:

 What does output.reportDiagnosticSummary() print out then the port is
 already in use, and have you made this happen with your fixes in place?
 
 Chris
 
 On 7/17/19 3:20 PM, Daniil Titov wrote:

 > Hi Chris and Alex,
 >
 > Please review a new version of the fix that moves the diagnostic output 
for the test failure to run()
 > method after the number of retry attempts is exceeded. It also includes 
other corrections that
 > you suggested.
 >
 > Thanks!
 >
 > Webrev: http://cr.openjdk.java.net/~dtitov/8221303/webrev.02/
 > Bug: https://bugs.openjdk.java.net/browse/JDK-8221303
 >
 > --Best regards,
 > Daniil
 >
 >
 > On 7/17/19, 1:47 PM, "Chris Plummer"  wrote:
 >
 >  Hi Daniil,
 >
 >  I think you can remove "Ok" from the following message:
 >
 >237 System.out.println("DEBUG: OK. Spawned java process terminated
 >  with expected exit code of "
 >238 + STOP_PROCESS_EXIT_VAL);
 >
 >  It's somewhat misleading since the test can still fail. I think the
 >  following is also misleading:
 >
 >249 if (testFailed) {
 >250 output.reportDiagnosticSummary();
 >251 if (output.getStderr().contains("Port already 
in
 >  use")) {
 >252 // Need to retry
 >253 return true;
 >254 }
 >255 }
 >
 >  The test can still pass after this happens, right? And I think 
there are
 >  other "Test FAILURE" error message that can appear just before this.
 >  Perhaps the code above should add a println that indicates that 
there
 >  will be a retry attempt since the failure was due to the port being 
in use.
 >
 >  Otherwise looks good.
 >
 >  thanks,
 >
 >  Chris
 >
 >  On 7/17/19 1:30 PM, Alex Menkov wrote:
 >  > Hi Daniil,
 >  >
 >  > The fix looks good in general.
 >  > Couple cosmetic notes (no new webrev required):
 >  >
 >  >  162 needRetry = runTest();
 >  >  163 }
 >  >  164 while (needRetry && (attempts++ < 
MAX_RETRY_ATTEMTS));
 >  > Please move "while" to the prev line:
 >  >  163 } while (needRetry && (attempts++ < 
MAX_RETRY_ATTEMTS));
 >  >
 >  >
 >  >  242 System.out.println("Test FAILURE on" + 
name +
 >  > " reason: The expected line \"" + READY_MSG
 >  >  243 + "\" is not present in 

Re: 8221303: sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java fails due to java.rmi.server.ExportException: Port already in use

2019-07-17 Thread Daniil Titov
Hi Chris,

output.reportDiagnosticSummary() prints the output from the process (both 
stdout and stderr) and the exit value to the test's stderr.
In case if the port is already in use it prints the following:

stdout: [];
 stderr: [Error: JMX connector server communication error: 
service:jmx:rmi://127.0.0.1:9101
jdk.internal.agent.AgentConfigurationError: java.rmi.server.ExportException: 
Port already in use: 9101; nested exception is: 
java.net.BindException: Address already in use
at 
jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.exportMBeanServer(ConnectorBootstrap.java:820)
at 
jdk.management.agent/sun.management.jmxremote.ConnectorBootstrap.startRemoteConnectorServer(ConnectorBootstrap.java:479)
at 
jdk.management.agent/jdk.internal.agent.Agent.startAgent(Agent.java:447)
at 
jdk.management.agent/jdk.internal.agent.Agent.startAgent(Agent.java:599)
Caused by: java.rmi.server.ExportException: Port already in use: 9101; nested 
exception is:
< I skipped the rest>
]
 exitValue = 1

It makes sense to have it called if the test fails, otherwise this information 
would be missed in the test output.
Please review the new version of the fix that has the call to 
output.reportDiagnosticSummary() restored.

Thanks!

Webrev: http://cr.openjdk.java.net/~dtitov/8221303/webrev.03/ 
Bug: https://bugs.openjdk.java.net/browse/JDK-8221303

-Daniil

On 7/17/19, 3:58 PM, "Chris Plummer"  wrote:

What does output.reportDiagnosticSummary() print out then the port is 
already in use, and have you made this happen with your fixes in place?

Chris

On 7/17/19 3:20 PM, Daniil Titov wrote:
> Hi Chris and Alex,
>
> Please review a new version of the fix that moves the diagnostic output 
for the test failure to run()
> method after the number of retry attempts is exceeded. It also includes 
other corrections that
> you suggested.
>
> Thanks!
>
> Webrev: http://cr.openjdk.java.net/~dtitov/8221303/webrev.02/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8221303
>
> --Best regards,
> Daniil
>
>
> On 7/17/19, 1:47 PM, "Chris Plummer"  wrote:
>
>  Hi Daniil,
>  
>  I think you can remove "Ok" from the following message:
>  
>237 System.out.println("DEBUG: OK. Spawned java process terminated
>  with expected exit code of "
>238 + STOP_PROCESS_EXIT_VAL);
>  
>  It's somewhat misleading since the test can still fail. I think the
>  following is also misleading:
>  
>249 if (testFailed) {
>250 output.reportDiagnosticSummary();
>251 if (output.getStderr().contains("Port already 
in
>  use")) {
>252 // Need to retry
>253 return true;
>254 }
>255 }
>  
>  The test can still pass after this happens, right? And I think there 
are
>  other "Test FAILURE" error message that can appear just before this.
>  Perhaps the code above should add a println that indicates that there
>  will be a retry attempt since the failure was due to the port being 
in use.
>  
>  Otherwise looks good.
>  
>  thanks,
>  
>  Chris
>  
>  On 7/17/19 1:30 PM, Alex Menkov wrote:
>  > Hi Daniil,
>  >
>  > The fix looks good in general.
>  > Couple cosmetic notes (no new webrev required):
>  >
>  >  162 needRetry = runTest();
>  >  163 }
>  >  164 while (needRetry && (attempts++ < 
MAX_RETRY_ATTEMTS));
>  > Please move "while" to the prev line:
>  >  163 } while (needRetry && (attempts++ < 
MAX_RETRY_ATTEMTS));
>  >
>  >
>  >  242 System.out.println("Test FAILURE on" + 
name +
>  > " reason: The expected line \"" + READY_MSG
>  >  243 + "\" is not present in the 
process
>  > output");
>  > Please add space: "Test FAILURE on " + name
>  >
>  > --alex
>  >
>  > On 07/17/2019 12:46, Daniil Titov wrote:
>  >> Hi Chris,
>  >>
>  >>> It's a little unclear to me why you moved from ProcessThread 
to
>  >>>TestProcessThread + Process. An explanation of that would 
make it
>  >>> easier
>  >>>to understand many of the changes.
>  >>
>  >> There are two reasons for that:
>  >> 1)  For every network interface the test starts a separate thread
>  >> that runs the test for this interface. We want that if the test 
fails
>  >>  due to the bind error 

Re: 8221303: sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java fails due to java.rmi.server.ExportException: Port already in use

2019-07-17 Thread Daniil Titov
Hi Chris,

>It's a little unclear to me why you moved from ProcessThread to 
>   TestProcessThread + Process. An explanation of that would make it easier 
>   to understand many of the changes.

There are two reasons for that:
1)  For every network interface the test starts a separate thread that runs the 
test for this interface. We want that if the test fails
due to the bind error the thread not to exit but try to repeat the test 
several times. jdk.test.lib.thread.ProcessThread doesn't allow this.
2) To filter out the cases when the test fails due to the bind error we need to 
parse the process output.  jdk.test.lib.thread.ProcessThread 
  registers its own pumps for stdin and sdtout  (by calling 
ProcessTools.startProcess()) that consume all output of the process and prevent 
  the output analyzer from collecting this output.
 
Thanks!
--Daniil

On 7/17/19, 12:11 PM, "Chris Plummer"  wrote:

Hi Daniil,

It's a little unclear to me why you moved from ProcessThread to 
TestProcessThread + Process. An explanation of that would make it easier 
to understand many of the changes.

thanks,

Chris

On 7/11/19 10:16 AM, Daniil Titov wrote:
> Please review the change that fixes an intermittent failure of  
sun/management/jmxremote/bootstrap/JMXInterfaceBindingTest.java
> test due to ports collision. The tests finds all network interfaces and 
for every interface starts a separate process that tests
> the connection to JMX agent server for a specific ports/interface 
combination.
>
> The test was changed to retry in case of the failure. If the subprocess 
fails to bind and the number
> of retry attempts doesn't exceed the limit a new pair of random ports is 
selected and the test is run again.
>
> Webrev: http://cr.openjdk.java.net/~dtitov/8221303/webrev.01/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8221303
>
> Thanks!
> --Daniil
>
>