RFR 9: 8077350 Process API Updates Implementation Review
anthony.vanelverdinghe at gmail.com
Sat Apr 11 19:24:21 UTC 2015
Thanks for your clarifications Roger. I'm very much in favor of your
suggestion for naming the method "supportsNormalTermination".
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
>> - "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.
>> Kind regards,
>> 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
More information about the core-libs-dev