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

[Andrey Dyachkov]

Hi Roger,

Thank you for reviewing it! I've prepared another version.

diff --git a/src/java.base/unix/classes/java/lang/ProcessImpl.java
b/src/java.base/unix/classes/java/lang/ProcessImpl.java
--- a/src/java.base/unix/classes/java/lang/ProcessImpl.java
+++ b/src/java.base/unix/classes/java/lang/ProcessImpl.java
@@ -630,6 +630,21 @@
         return !hasExited;
     }

+    /**
+     * The {@code toString} method for class {@code ProcessImpl}
+     * returns a string consisting of the native process ID of the process,
+     * exit code of the process and the status of the process.
+     *
+     * @return  a string representation of the object.
+     */
+    @Override
+    public String toString() {
+        return new StringBuilder("ProcessImpl{pid=").append(pid)
+        .append(", exitcode=").append(exitcode)
+        .append(", hasExited=").append(hasExited)
+        .append("}").toString();
+    }
+
     private static native void init();

     static {

On Mon, 9 May 2016 at 16:50 Roger Riggs <Roger.Riggs at oracle.com> wrote:

> 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();
> > +    }
> > +
> >
> >   }
>
> --

With great enthusiasm,
Andrey