Hi Hamlin,
One more suggested improvement. Instead of two copy/paste copies of
the launch with options code,
it would cleaner to create a separate RMID.launch(String[] options)
method that would be passed the extra arguments.
Use it in forceLogSnapshot.java and ShutdownGracefully.java.
The (current) no-arg version could call the version with args with an
empty array(or null).
There would be only 1 copy of the launch algorithm.
Roger
On 9/27/2016 11:22 PM, Hamlin Li wrote:
Hi Roger,
Thank you for reviewing.
Please check the new webrev:
http://cr.openjdk.java.net/~mli/8085192/webrev.01/, and comments
inline below.
On 2016/9/27 23:14, Roger Riggs wrote:
Hi Hamlin,
Marking each test that uses RMID.launch with the bugid does not seem
to be meaningful
since the bug is in the support infrastructure of the test and not
specific to the test itself.
It would be overkill to try to confirm the bug was fixed by running
all those tests.
Putting the bugid on 1 of the tests would be sufficient.
Remove all bug ids except of the one in CheckImplClassLoader.java.
JavaVM.java:
- 134-138: Why define these private methods if they are not going
to be used in outputContains?
- 185: Its inefficient to convert the byte array to a string for
when looking for each string.
It would be cleaner for JavaVM to have public outputString and
errorString methods
and check them separately in RMID.
(remove the JavaVM.outputContains method which hides which stream
the string appeared in).
Fixed.
(It would be a bigger change to change this to use the test library
ProcessTools and OutputAnalyzer).
Thank you suggestion. Yes, you're right, it will be a little bit
complicated to use ProcessTools and OutputAnalyzer in this situation,
need to modify these classes, because they will block until process
terminates. So I prefer to use current implementation, as it's simple
and clear.
Thank you
-Hamlin
Roger
On 9/27/2016 5:22 AM, Hamlin Li wrote:
Please review the fix for JDK-8085192. The fix checks whether it
fails to launch rmid due to "Port already in use" error, it will
launch rmid again and again(20 times at most) until no such issue.
bug:
https://bugs.openjdk.java.net/browse/JDK-8085192
webrev:
http://cr.openjdk.java.net/~mli/8085192/webrev.00/
Thank you
-Hamlin
