Thanks for your clarifications Roger. I'm very much in favor of your
suggestion for naming the method "supportsNormalTermination".
Kind regards,
Anthony
On 11/04/2015 20:35, Roger Riggs wrote:
Hi Anthony,
Thanks for the review and comments.
On 4/11/2015 5:00 AM, Anthony Vanelverdinghe wrote:
Hi Roger
In my opinion, the method "supportsDestroyForcibly" is unintuitive,
for the following 2 reasons:
- it's named "supportsXxx", where Xxx is the name of a method in the
same class. So as a user of this API, I would intuitively assume that
"supportsDestroyForcibly" is related to "destroyForcibly", and means
whether or not "destroyForcibly" actually forcibly terminates the
process.
- "supports" has a positive connotation. However, if
"supportsDestroyForcibly" returns true, this means that the
implementation of "destroy" is forcible. And "destroyForcibly" either
invokes "destroy" (default implementation) or is overridden with a
compliant implementation. So in other words: if
"supportsDestroyForcibly" returns true, it's impossible to allow the
process to shut down cleanly. This, in my opinion, is a bad thing, so
the method indicating this behavior should not start with "supports".
The naming of the method has been problematic; as is the ambiguous
semantics of Process.destroy
which are historical.
Therefore I would like to propose to replace
"supportsDestroyForcibly" with "supportsDestroy", which returns the
negation of what's currently returned by "supportsDestroyForcibly".
I'm not sure this is clearer. Both destroy() and destroyForcibly()
always support the destruction
of the process and without reading more closely, supportDestroy()
would always be true.
Perhaps supportNormalTermination() would reflect the more general
understanding of the behavior
and the spec could provide the details in relation to the destroy method.
Another question I have is: what happens if "destroy" or
"destroyForcibly" are called on processes other than the current
process & its children, i.e. a process that is in "allProcesses" but
isn't "current" & isn't in "allChildren" (e.g. the parent process of
the current process)?
The current spec does not guarantee that the process will be destroyed;
processes can protect themselves against the signals (SIGTERM or SIGKILL)
just as it does not make any assertion about the timing of the process
state change.
The behavior is to attempt to kill the process but it may not work for
a variety of
reasons, not all of which can be detected or reported to the caller.
For ProcessHandle.destroy, if the OS does not allow the signal delivered,
the destory/destroyForcibly method could return a boolean.
Alternatively, Martin's request to throw arbitrary signals would be a
new method
with more os specific behavior and exceptions.
The decoupling of ProcessHandle and Process opens the possibility
to have different semantics than for Process.destroy/destroyForcibly.
Roger
Kind regards,
Anthony
On 9/04/2015 22:00, Roger Riggs wrote:
Please review the API and implementation of the Process API Updates
described inJEP 102
<https://bugs.openjdk.java.net/browse/JDK-8046092>. Please review
and comment by April 23rd.
The recommendation to make ProcessHandle an interface is included
allowing the new functions to be extended by Process subclasses.
The implementation covers all functions on Unix, Windows, Solaris,
and Mac OS X.
The API doc: http://cr.openjdk.java.net/~rriggs/ph-apidraft/
The webrev: http://cr.openjdk.java.net/~rriggs/webrev-ph
Issue: JDK-8077350
<https://bugs.openjdk.java.net/browse/JDK-8077350> Process API
Updates Implementation
The code is in the jdk9 sandbox on branch JDK-8046092-branch.
Please review and comment, Roger
