Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-20 Thread Roger Riggs
Hi Peter, yes, your patch does reduce the code quite a bit except where the complexity is needed. The commonPool does have a soft limit (256/system property) on the number of compensating processes it will start. That limit would only come into play for subclasses of Process that did not pro

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-20 Thread Peter Levart
Hi Roger, I looked at Martin's idea and I think that we don't need the AsyncExecutor at all (it already sounds like I hate it ;-). Using ManagedBlocker, a ForkJoinPoll can compensate and grow it's pool as needed when Process.waitFor() blocks. So we could leverage this feature and simplify thi

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-19 Thread Ivan Gerasimov
On 19.05.2015 23:15, Roger Riggs wrote: The webrev, javadoc, and specdiffs have been updated to address recent recommendations: Please review and comment: Webrev: http://cr.openjdk.java.net/~rriggs/webrev-ph/ (May 19) src/java.base/windows/classes/java/lang/ProcessImpl.java : 37 import ja

RFR 9: 8077350 Process API Updates Implementation Review

2015-05-19 Thread Roger Riggs
The webrev, javadoc, and specdiffs have been updated to address recent recommendations: Please review and comment: Webrev: http://cr.openjdk.java.net/~rriggs/webrev-ph/ (May 19) javadoc: http://cr.openjdk.java.net/~rriggs/ph-apidraft/ (May 19) Diffs of the spec/javadoc from previous draft: ht

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-19 Thread Roger Riggs
Hi Peter, On 5/19/2015 4:34 AM, Peter Levart wrote: Hi Roger, On 05/18/2015 11:49 PM, Roger Riggs wrote: The earlier spec specifically used the common pool but from a specification point of view it allows more flexibility in the implementation not to bind onExit to the commonPool. Its not cle

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-19 Thread Roger Riggs
Hi Paul, a couple of followup responses... On 5/18/2015 5:16 PM, Paul Sandoz wrote: 251 /** 252 * Kills the subprocess forcibly. The subprocess represented by this 253 * {@code Process} object is forcibly terminated. 254 * Forcible process destruction is defined as

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-19 Thread Peter Levart
Hi Roger, On 05/18/2015 11:49 PM, Roger Riggs wrote: The earlier spec specifically used the common pool but from a specification point of view it allows more flexibility in the implementation not to bind onExit to the commonPool. Its not clear that the attributes of threads used to handle outpu

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-18 Thread Roger Riggs
Hi Peter, On 5/15/2015 6:35 AM, Peter Levart wrote: Hi Roger, On 05/14/2015 03:44 PM, Roger Riggs wrote: Hi Peter, On 5/14/15 8:19 AM, Peter Levart wrote: Hi Roger, The new API using Optional(s) looks fine. In particular for the ProcessHandle returning methods. They now either return Stre

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-18 Thread Paul Sandoz
On May 18, 2015, at 10:49 PM, Roger Riggs wrote: > Hi Paul, > > Thanks for the comments. > > On 5/18/2015 7:58 AM, Paul Sandoz wrote: >> Ho Roger, >> >> I mostly focused on the specification. >> >> Paul. >> >> >> Process >> -- >> >> 35 * {@code Process} provides control of native proces

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-18 Thread Roger Riggs
Hi Paul, Thanks for the comments. On 5/18/2015 7:58 AM, Paul Sandoz wrote: Ho Roger, I mostly focused on the specification. Paul. Process -- 35 * {@code Process} provides control of native processes started by 36 * ProcessBuilder.start and Runtime.exec. Link to those methods? Links

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-18 Thread Paul Sandoz
Ho Roger, I mostly focused on the specification. Paul. Process -- 35 * {@code Process} provides control of native processes started by 36 * ProcessBuilder.start and Runtime.exec. Link to those methods? 92 /** 93 * Default constructor for Process. 94 */ 95 publi

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-15 Thread Martin Buchholz
On Thu, May 14, 2015 at 6:44 AM, Roger Riggs wrote: > >> At some point in the development of this API, the implementation >> introduced the AsyncExecutor to execute synchronous continuations of the >> onExit() returned CompletableFuture(s). What was the main motivation for >> this given that: >>

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-15 Thread Peter Levart
Hi Roger, On 05/15/2015 12:35 PM, Peter Levart wrote: public class AsyncCompletableFuture ... { /** * Returns a new CompletionStage that, when this stage completes * normally, completes with the given replacementResult. * * @param replacementResult the successful complet

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-15 Thread Peter Levart
Hi Roger, On 05/14/2015 03:44 PM, Roger Riggs wrote: Hi Peter, On 5/14/15 8:19 AM, Peter Levart wrote: Hi Roger, The new API using Optional(s) looks fine. In particular for the ProcessHandle returning methods. They now either return Stream or Optional. At some point in the development of

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-14 Thread Roger Riggs
Hi Alan, For something to log from an inoperative destroy call, are you thinking it should throw an exception? In that case the return value could be void and the exception message could expose the OS specific reason. None of the currently available exceptions seem appropriate; A bare Runtim

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-14 Thread Roger Riggs
Hi Peter, On 5/14/15 8:19 AM, Peter Levart wrote: Hi Roger, The new API using Optional(s) looks fine. In particular for the ProcessHandle returning methods. They now either return Stream or Optional. At some point in the development of this API, the implementation introduced the AsyncExecu

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-14 Thread Alan Bateman
On 13/05/2015 16:10, Roger Riggs wrote: Hi Alan, Yes, the destroy use looks a bit odd. In Process, destroyForcibly returns this so that the application can fluently wait for the termination to complete. Returning Optional enables the case to fluently perform an action on the process when it

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-14 Thread Peter Levart
Hi Roger, The new API using Optional(s) looks fine. In particular for the ProcessHandle returning methods. They now either return Stream or Optional. At some point in the development of this API, the implementation introduced the AsyncExecutor to execute synchronous continuations of the onE

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-13 Thread Roger Riggs
Hi Alan, Yes, the destroy use looks a bit odd. In Process, destroyForcibly returns this so that the application can fluently wait for the termination to complete. Returning Optional enables the case to fluently perform an action on the process when it does exit. ProcessHandle ph = ...;

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-13 Thread Alan Bateman
On 13/05/2015 15:16, Roger Riggs wrote: Hi, Are there any comments about the use of java.util.Optional in the ProcessHandle API? Or a review of the changes? Having parent return Optional looks good. Having all methods in ProcessHandle.Info return Optional is probably right, it gives us a bi

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-13 Thread Roger Riggs
Hi, Are there any comments about the use of java.util.Optional in the ProcessHandle API? Or a review of the changes? Thanks, Roger On 5/11/2015 11:49 AM, Roger Riggs wrote: Please review clarifications and updates to the proposed Precess API. A few loose ends in the ProcessHandle API were

RFR 9: 8077350 Process API Updates Implementation Review

2015-05-11 Thread Roger Riggs
Please review clarifications and updates to the proposed Precess API. A few loose ends in the ProcessHandle API were identified. 1) The ProcessHandle.parent() method currently returns null if the parent cannot be determined and the ProcessHandle.of(PID) method returns null if the PID does not

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-05-07 Thread Roger Riggs
fyi, with respect to using the start time to uniquely identify a process. It turns out that on Linux[1] looking at st_mtime from stat(/proc/pid/status) doesn't always produce the same value (and the inode number can change). When spawning large number of processes (< 500) it appears the pseudo

Re: RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

2015-04-24 Thread Roger Riggs
Hi Alan, On 4/24/2015 12:21 PM, Alan Bateman wrote: On 24/04/2015 16:49, Roger Riggs wrote: : I'm not sure about the @implSpec in Process::supportsNormalTermination. Shouldn't that just specify that the default implementation throws UOE. An @implNote could comment on how ProcessBuilder wor

Re: RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

2015-04-24 Thread Alan Bateman
On 24/04/2015 16:49, Roger Riggs wrote: : I'm not sure about the @implSpec in Process::supportsNormalTermination. Shouldn't that just specify that the default implementation throws UOE. An @implNote could comment on how ProcessBuilder works. Same comment on Process::toHandle. There needs t

Re: RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

2015-04-24 Thread Roger Riggs
Hi Alan, On 4/24/2015 8:06 AM, Alan Bateman wrote: On 17/04/2015 20:12, Roger Riggs wrote: The webrev for ProcessAPI updates has been updated to reflect recent comments. Please review and comment by April 23rd. The updates include: - Renaming Process/ProcessHandle supportsDestroyForcibly to

Re: RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

2015-04-24 Thread Alan Bateman
On 17/04/2015 20:12, Roger Riggs wrote: The webrev for ProcessAPI updates has been updated to reflect recent comments. Please review and comment by April 23rd. The updates include: - Renaming Process/ProcessHandle supportsDestroyForcibly to supportsNormalTermination and updating related

Re: RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

2015-04-21 Thread Roger Riggs
Hi Thomas, Good point and more robust. Thanks, Roger On 4/21/2015 11:49 AM, Thomas Stüfe wrote: Hi Roger, small remark, I see you at a number of places using the same pattern when reading information from /proc: 342 ret = fread(&psinfo, 1, (sizeof psinfo), fp); 343 fclose(fp);

Re: RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

2015-04-21 Thread Paul Sandoz
On Apr 21, 2015, at 5:20 PM, Roger Riggs wrote: > Hi Paul, > > The onExit() @implNote recommends the subclass delegate to the underlying > process. I know, but is that recommendation just for our (Oracle's) JDK? Is it subject to change? What if another JDK vendor decides to have a different

Re: RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

2015-04-21 Thread Thomas Stüfe
Hi Roger, small remark, I see you at a number of places using the same pattern when reading information from /proc: 342 ret = fread(&psinfo, 1, (sizeof psinfo), fp); 343 fclose(fp); 344 if (ret < 0) { 345 return; 346 } A better way would be to check for the whole siz

Re: RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

2015-04-21 Thread Roger Riggs
Hi Paul, The onExit() @implNote recommends the subclass delegate to the underlying process. For a simple subclass acting as a proxy or decorator that will provide the best implementation. I'll add the onExit() method to the class level recommendation for Subclasses of Process. Thanks, Rog

Re: RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

2015-04-21 Thread Paul Sandoz
On Apr 21, 2015, at 2:57 PM, Roger Riggs wrote: > Hi Paul, > > On 4/21/2015 8:29 AM, Paul Sandoz wrote: >> There are statements in Process about the specified behavior of Processes >> created by ProcessBuilder. That's why I included them in the @implSpec >> clause. If @implSpec is only for th

Re: RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

2015-04-21 Thread Roger Riggs
Hi Paul, On 4/21/2015 8:29 AM, Paul Sandoz wrote: There are statements in Process about the specified behavior of Processes created by ProcessBuilder. That's why I included them in the @implSpec clause. If @implSpec is only for the specifics of the method itself then where should the behavior

Re: RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

2015-04-21 Thread Paul Sandoz
On Apr 20, 2015, at 8:42 PM, Roger Riggs wrote: > Hi Paul, > > On 4/20/2015 2:26 PM, Paul Sandoz wrote: >> On Apr 20, 2015, at 7:33 PM, Roger Riggs wrote: >> >>> Hi Paul, >>> >>> There are statements in Process about the specified behavior of Processes >>> created by ProcessBuilder. That's

Re: RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

2015-04-20 Thread Roger Riggs
Hi Paul, On 4/20/2015 2:26 PM, Paul Sandoz wrote: On Apr 20, 2015, at 7:33 PM, Roger Riggs wrote: Hi Paul, There are statements in Process about the specified behavior of Processes created by ProcessBuilder. That's why I included them in the @implSpec clause. If @implSpec is only for the sp

Re: RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

2015-04-20 Thread Paul Sandoz
On Apr 20, 2015, at 7:33 PM, Roger Riggs wrote: > Hi Paul, > > There are statements in Process about the specified behavior of Processes > created by ProcessBuilder. That's why I included them in the @implSpec > clause. > If @implSpec is only for the specifics of the method itself then where

Re: RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

2015-04-20 Thread Roger Riggs
Hi Paul, There are statements in Process about the specified behavior of Processes created by ProcessBuilder. That's why I included them in the @implSpec clause. If @implSpec is only for the specifics of the method itself then where should the behavior of ProcessBuilder created instances be sp

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-20 Thread Roger Riggs
ok, thanks, roger On 4/20/2015 11:57 AM, Thomas Stüfe wrote: Hi Roger, thanks! Maybe better: "When using ProcessHandles avoid assumptions about the state or liveness of the underlying process." -> "When using ProcessHandles avoid assumptions about liveness or identity of the underlying pro

Re: RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

2015-04-20 Thread Paul Sandoz
On Apr 20, 2015, at 5:49 PM, Roger Riggs wrote: > Hi Paul, > > On 4/20/2015 9:01 AM, Paul Sandoz wrote: >> Hi Roger, >> >> I am not sure you have the @implSpec/@implNote quite correct on the new >> methods of Process. >> >> For example, for Process.toHandle i would expect something like: >>

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-20 Thread Roger Riggs
Hi Thomas, I expanded the ProcessHandle class javadoc [1] paragraph to include the caution about process id reuse. Thanks, Roger [1] http://cr.openjdk.java.net/~rriggs/ph-apidraft/java/lang/ProcessHandle.html On 4/18/2015 2:44 PM, Thomas Stüfe wrote: Hi Roger, On Fri, Apr 17, 2015 at 8:57

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-20 Thread Thomas Stüfe
Hi Roger, thanks! Maybe better: "When using ProcessHandles avoid assumptions about the state or liveness of the underlying process." -> "When using ProcessHandles avoid assumptions about liveness or identity of the underlying process." Regards, Thomas On Mon, Apr 20, 2015 at 5:53 PM, Roger Ri

Re: RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

2015-04-20 Thread Roger Riggs
Hi Paul, On 4/20/2015 9:01 AM, Paul Sandoz wrote: Hi Roger, I am not sure you have the @implSpec/@implNote quite correct on the new methods of Process. For example, for Process.toHandle i would expect something like: ... @implSpec This implementation throws an instance of Unsupport

Re: RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

2015-04-20 Thread Paul Sandoz
Hi Roger, I am not sure you have the @implSpec/@implNote quite correct on the new methods of Process. For example, for Process.toHandle i would expect something like: ... @implSpec This implementation throws an instance of UnsupportedOperationException and performs no other action. S

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-18 Thread Thomas Stüfe
Hi Roger, On Fri, Apr 17, 2015 at 8:57 PM, Roger Riggs wrote: > Hi David, > > On 4/17/2015 2:44 PM, David M. Lloyd wrote: > >> On 04/17/2015 11:53 AM, Roger Riggs wrote: >> >>> Hi Peter, >>> >>> On 4/17/2015 4:05 AM, Peter Levart wrote: >>> Hi Roger, Retrieving and caching the pro

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-18 Thread Thomas Stüfe
Hi Roger, On Fri, Apr 17, 2015 at 7:05 PM, Roger Riggs wrote: > Hi Thomas, > > On 4/16/2015 3:01 PM, Thomas Stüfe wrote: > > Hi Roger, > > thank you for your answer! > > The reason I take an interest is not just theoretical. We (SAP) use our > JVM for our test infrastructure and we had exactl

Re: RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

2015-04-17 Thread Roger Riggs
The webrev for ProcessAPI updates has been updated to reflect recent comments. Please review and comment by April 23rd. The updates include: - Renaming Process/ProcessHandle supportsDestroyForcibly to supportsNormalTermination and updating related descriptions - ProcessHandle.destroy/dest

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-17 Thread Roger Riggs
Hi David, On 4/17/2015 2:44 PM, David M. Lloyd wrote: On 04/17/2015 11:53 AM, Roger Riggs wrote: Hi Peter, On 4/17/2015 4:05 AM, Peter Levart wrote: Hi Roger, Retrieving and caching the process start time as soon as ProcessHandle is instantiated might be a good idea. "destroy" native code wo

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-17 Thread David M. Lloyd
On 04/17/2015 11:53 AM, Roger Riggs wrote: Hi Peter, On 4/17/2015 4:05 AM, Peter Levart wrote: Hi Roger, Retrieving and caching the process start time as soon as ProcessHandle is instantiated might be a good idea. "destroy" native code would then use pid *and* start time to check the identity

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-17 Thread Roger Riggs
Hi Thomas, On 4/16/2015 3:01 PM, Thomas Stüfe wrote: Hi Roger, thank you for your answer! The reason I take an interest is not just theoretical. We (SAP) use our JVM for our test infrastructure and we had exactly the problem allChildren() is designed to solve: killing a process tree related

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-17 Thread Roger Riggs
Hi Thomas, On 4/17/2015 4:22 AM, Thomas Stüfe wrote: Hi Roger, aside from the recycle-pid-question, one additional remark: in ProcessHandleImpl_unix.c, Java_java_lang_ProcessHandleImpl_isAlive0, you call kill(pid, 0) for the liveness check. If you have not the necessary permissions to do t

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-17 Thread Roger Riggs
Hi Peter, On 4/17/2015 4:05 AM, Peter Levart wrote: Hi Roger, Retrieving and caching the process start time as soon as ProcessHandle is instantiated might be a good idea. "destroy" native code would then use pid *and* start time to check the identity of the process before killing it. Yes, th

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-17 Thread Thomas Stüfe
Hi Roger, aside from the recycle-pid-question, one additional remark: in ProcessHandleImpl_unix.c, Java_java_lang_ProcessHandleImpl_isAlive0, you call kill(pid, 0) for the liveness check. If you have not the necessary permissions to do this call, this may fail with EPERM. In this case, isAlive()

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-17 Thread Thomas Stüfe
On Fri, Apr 17, 2015 at 8:40 AM, Staffan Larsen wrote: > > On 16 apr 2015, at 21:01, Thomas Stüfe wrote: > > Hi Roger, > > thank you for your answer! > > The reason I take an interest is not just theoretical. We (SAP) use our JVM > for our test infrastructure and we had exactly the problem allCh

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-17 Thread Peter Levart
Hi Roger, Retrieving and caching the process start time as soon as ProcessHandle is instantiated might be a good idea. "destroy" native code would then use pid *and* start time to check the identity of the process before killing it. At least on Linux (/proc//stat) and Solaris (/proc//status)

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-16 Thread Staffan Larsen
> On 16 apr 2015, at 21:01, Thomas Stüfe wrote: > > Hi Roger, > > thank you for your answer! > > The reason I take an interest is not just theoretical. We (SAP) use our JVM > for our test infrastructure and we had exactly the problem allChildren() is > designed to solve: killing a process tree

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-16 Thread Thomas Stüfe
Hi Roger, thank you for your answer! The reason I take an interest is not just theoretical. We (SAP) use our JVM for our test infrastructure and we had exactly the problem allChildren() is designed to solve: killing a process tree related to a specific tests (similar to jtreg tests) in case of er

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-15 Thread Peter Levart
Hi Roger, On 04/14/2015 11:33 PM, Roger Riggs wrote: Hi Peter, Thanks for the review. On 4/14/2015 11:47 AM, Peter Levart wrote: On 04/09/2015 10:00 PM, Roger Riggs wrote: Please review the API and implementation of the Process API Updates described inJEP 102

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-14 Thread Roger Riggs
Hi Peter, Thanks for the review. On 4/14/2015 11:47 AM, Peter Levart wrote: On 04/09/2015 10:00 PM, Roger Riggs wrote: Please review the API and implementation of the Process API Updates described inJEP 102 . Please review and comment by Apr

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-14 Thread Alan Bateman
On 14/04/2015 16:47, Peter Levart wrote: The behavior of pipes on Windows is a little different (perhaps because the Pipe NIO API uses sockets under the hood on Windows - why is that? Windows does have a pipe equivalent). Multiplexing is a bit limited on Windows, I don't think you can select

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-14 Thread Peter Levart
On 04/09/2015 10:00 PM, Roger Riggs wrote: Please review the API and implementation of the Process API Updates described inJEP 102 . Please review and comment by April 23rd. The recommendation to make ProcessHandle an interface is included al

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-13 Thread Martin Buchholz
On Sat, Apr 11, 2015 at 11:13 AM, Roger Riggs wrote: > Hi Martin, > > Thanks for the review. > > On 4/11/2015 1:37 AM, Martin Buchholz wrote: > > Thanks for the huge effort. I did a superficial review and it seems > pretty good. Of course, changing the Process good is high risk and some > thin

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-11 Thread Anthony Vanelverdinghe
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

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-11 Thread Roger Riggs
Hi Thomas, Thanks for the comments. On 4/11/2015 8:31 AM, Thomas Stüfe wrote: Hi Roger, I have a question about getChildren() and getAllChildren(). I assume the point of those functions is to implement point 4 of JEP 102 ("The ability to deal with process trees, in particular some means to

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-11 Thread Roger Riggs
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

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-11 Thread Roger Riggs
Hi Martin, Thanks for the review. On 4/11/2015 1:37 AM, Martin Buchholz wrote: Thanks for the huge effort. I did a superficial review and it seems pretty good. Of course, changing the Process good is high risk and some things will probably need fixup later. On Unix, you seem to be identify

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-11 Thread Thomas Stüfe
p.s. Note that using allChildren() to kill process trees has a second problem, even without PID recycling: the PID list it returns may not be complete once you come around to use it. Imagine a process tree with some runaway process forking below you constantly. You want to kill the complete proce

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-11 Thread Thomas Stüfe
Hi Roger, I have a question about getChildren() and getAllChildren(). I assume the point of those functions is to implement point 4 of JEP 102 ("The ability to deal with process trees, in particular some means to destroy a process tree."), by returning a collection of PIDs which are the children

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-11 Thread Anthony Vanelverdinghe
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 "destro

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-10 Thread Martin Buchholz
On Fri, Apr 10, 2015 at 10:37 PM, Martin Buchholz wrote: > > +} else if (siginfo.si_code == CLD_KILLED || siginfo.si_code == > CLD_DUMPED) { > > Using siginfo probably won't work under load because multiple signals > arriving at the same time are coalesced into one, at least on Linux. >

Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-10 Thread Martin Buchholz
Thanks for the huge effort. I did a superficial review and it seems pretty good. Of course, changing the Process good is high risk and some things will probably need fixup later. On Unix, you seem to be identifying ProcessHandles by their pid. What are you going to do about the problem of misid

RFR 9: 8077350 Process API Updates Implementation Review

2015-04-09 Thread Roger Riggs
Please review the API and implementation of the Process API Updates described inJEP 102 . Please review and comment by April 23rd. The recommendation to make ProcessHandle an interface is included allowing the new functions to be extended by Pr