[PATCH] JDK-8155102: Process.toString could include pid, isAlive, exitStatus (original) (raw)

Roger Riggs Roger.Riggs at Oracle.com
Mon May 9 14:49:30 UTC 2016

Hi Andrey,

Since toString is a public method it needs to provide complete javadoc specification as providing values for debugging information. ToString methods typically directly reflect the state of the fields of the object.

Why should the exitValue have the value of the pid if the process is not alive?

Also, the if toString is providing the exitValue then it should be identified as "exitValue", not exitCode. the isAlive value should be identified as "isAlive". The implementation class should not be exposed. (remove this.getClass().getSimpleName()).

The performance of what seems like a simple toString method is not going to be great because of the native calls necessary to determine if the process is alive (called twice) and in the case of exitValue to handle the IllegalStateException.

Some of the overhead could be avoided, by implementing toString in each of the ProcessImpl.java files where the current state is known more conveniently with less overhead.

$.02, Roger

On 5/8/2016 2:47 PM, Andrey Dyachkov wrote:

Hello,

I have added toString() method in Process.java. diff --git a/src/java.base/share/classes/java/lang/Process.java b/src/java.base/share/classes/java/lang/Process.java --- a/src/java.base/share/classes/java/lang/Process.java +++ b/src/java.base/share/classes/java/lang/Process.java @@ -548,5 +548,16 @@ return toHandle().descendants(); } + @Override + public String toString() { + boolean isAlive = this.isAlive(); + return new StringBuilder(this.getClass().getSimpleName()).append("[") + .append("running=").append(isAlive).append(", ") + .append(isAlive ? "pid=" : "exitCode=") + .append(isAlive ? this.getPid() : this.exitValue()) + .append("]") + .toString(); + } + }

More information about the core-libs-dev mailing list