Fix Javadoc for exitValue method exception tag#348
Conversation
There was a problem hiding this comment.
Pull request overview
Updates Javadoc on ManagedProcess#exitValue() to use the standard @throws tag for documenting ManagedProcessException.
Changes:
- Replaced
@exceptionwith@throwsin theexitValue()method Javadoc.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Javadoc for the exitValue method in ManagedProcess.java, changing the @exception tag to @throws. Feedback indicates that the Javadoc description remains misleading because the implementation blocks until the process terminates, which deviates from the standard java.lang.Process.exitValue() behavior, and it lacks necessary checks to prevent indefinite blocking if the process was never started.
| * @return the exit value of the subprocess represented by this <code>Process</code> object. by | ||
| * convention, the value <code>0</code> indicates normal termination. | ||
| * @exception ManagedProcessException if the subprocess represented by this <code>ManagedProcess | ||
| * @throws ManagedProcessException if the subprocess represented by this <code>ManagedProcess |
There was a problem hiding this comment.
The Javadoc description for this method (which continues on line 445) is misleading. It states that a ManagedProcessException is thrown if the subprocess has not yet terminated, but the implementation actually calls asyncResult.get(), which blocks until the process terminates. This behavior differs significantly from java.lang.Process.exitValue(), which is non-blocking and throws an exception if the process is still running. Furthermore, the current implementation will block indefinitely if the process was never started, as it lacks the assertWaitForIsValid() check found in the waitForExit methods.
No description provided.