Re: RFR 9: 8077350 Process API Updates Implementation Review
On 16 apr 2015, at 21:01, Thomas Stüfe thomas.stu...@gmail.com 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 to a specific tests (similar to jtreg tests) in case of errors or hangs. We have test machines running large workloads of tests in parallel and we reach pid wraparound - depending on the OS - quite fast. We solved this by adding process groups to Process.java and we are very happy with this solution. We are able to quickly kill a whole process tree, cleanly and completely, without ambiguity or risk to other tests. Of course we had to add this support as a sideways hack in order to not change the official Process.java interface. Therefore I was hoping that with JEP 102, we would get official support for process groups. Unfortunately, seems the decision is already done and we are too late in the discussion :( Interestingly we are hoping to use allChildren() to kill process trees in jtreg - exactly the use case you are describing. I haven’t been testing the current approach in allChildren(), but it seems your experience indicates that it will not be a perfect fit for the use case. In a previous test framework I was involved in we also used process groups for this with good results. This does beg the question: if the current approach isn’t useful for our own testing purposes, when is it useful? Thanks, /Staffan see my other comments inline. On Sat, Apr 11, 2015 at 8:55 PM, Roger Riggs roger.ri...@oracle.com mailto:roger.ri...@oracle.com wrote: 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 destroy a process tree.), by returning a collection of PIDs which are the children of the process and then killing them? Earlier versions included a killProcess tree method but it was recommended to leave the exact algorithm to kill processes to the caller. However, I am not sure that this can be implemented in a safe way, at least on UNIX, because - as Martin already pointed out - of PID recycling. I do not see how you can prevent allChildren() from returning PIDs which may be already reaped and recyled when you use them later. How do you prevent that? Unless there is an extended time between getting the children and destroying them the pids will still be valid. Why? Child process may be getting reaped the instant you are done reading it from /proc, and pid may have been recycled by the OS right away and already pointing to another process when allChildren() returns. If a process lives about as long as it takes the system to reach a pid wraparound to the same pid value, its pid could be recycled right after it is reaped, or? Sure, the longer you wait, the higher the chance of this to happen, but it may happen right away. As Martin said, we had those races in the kill() code since a long time, but children()/allChildren() could make those error more probable, because now more processes are involved. Especially if you use allChildren to kill a deep process tree. And there is nothing in the javadoc warning the user about this scenario. You would just happen from time to time to kill an unrelated process. Those problems are hard to debug. The technique of caching the start time can prevent that case; though it has AFAIK not been a problem. How would that work? User should, before issuing the kill, compare start time of process to kill with cached start time? Note even if your coding is bulletproof, that allChildren() will also return PIDs of sub processes which are completely unrelated to you and Process.java - they could have been forked by some third party native code which just happens to run in parallel in the same process. There, you have no control about when it gets reaped. It might already have been reaped by the time allChildren() returns, and now the same PID got recycled as another, unrelated process. Of course, the best case is for an application to spawn and manage its own processes and handle there proper termination. The use cases for children/allChildren are focused on supervisory/executive functions that monitor a running system and can cleanup even in the case of unexpected failures. All management of processes is subject to OS limitations, if the PID were from a completely different process tree, the ordinary destroy/info functions would not be available unless the process was running as a privileged os user (same as any other native application). Could you explain this please? If both trees run under the same user, why should
Re: RFR 9: 8077350 Process API Updates Implementation Review
On Fri, Apr 17, 2015 at 8:40 AM, Staffan Larsen staffan.lar...@oracle.com wrote: On 16 apr 2015, at 21:01, Thomas Stüfe thomas.stu...@gmail.com 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 to a specific tests (similar to jtreg tests) in case of errors or hangs. We have test machines running large workloads of tests in parallel and we reach pid wraparound - depending on the OS - quite fast. We solved this by adding process groups to Process.java and we are very happy with this solution. We are able to quickly kill a whole process tree, cleanly and completely, without ambiguity or risk to other tests. Of course we had to add this support as a sideways hack in order to not change the official Process.java interface. Therefore I was hoping that with JEP 102, we would get official support for process groups. Unfortunately, seems the decision is already done and we are too late in the discussion :( Interestingly we are hoping to use allChildren() to kill process trees in jtreg - exactly the use case you are describing. I haven’t been testing the current approach in allChildren(), but it seems your experience indicates that it will not be a perfect fit for the use case. In a previous test framework I was involved in we also used process groups for this with good results. This does beg the question: if the current approach isn’t useful for our own testing purposes, when is it useful? Monitoring, I guess. Like writing your own pstree. But not for anything requiring you to send signals to those pids. ..Thomas Thanks, /Staffan see my other comments inline. On Sat, Apr 11, 2015 at 8:55 PM, Roger Riggs roger.ri...@oracle.com wrote: 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 destroy a process tree.), by returning a collection of PIDs which are the children of the process and then killing them? Earlier versions included a killProcess tree method but it was recommended to leave the exact algorithm to kill processes to the caller. However, I am not sure that this can be implemented in a safe way, at least on UNIX, because - as Martin already pointed out - of PID recycling. I do not see how you can prevent allChildren() from returning PIDs which may be already reaped and recyled when you use them later. How do you prevent that? Unless there is an extended time between getting the children and destroying them the pids will still be valid. Why? Child process may be getting reaped the instant you are done reading it from /proc, and pid may have been recycled by the OS right away and already pointing to another process when allChildren() returns. If a process lives about as long as it takes the system to reach a pid wraparound to the same pid value, its pid could be recycled right after it is reaped, or? Sure, the longer you wait, the higher the chance of this to happen, but it may happen right away. As Martin said, we had those races in the kill() code since a long time, but children()/allChildren() could make those error more probable, because now more processes are involved. Especially if you use allChildren to kill a deep process tree. And there is nothing in the javadoc warning the user about this scenario. You would just happen from time to time to kill an unrelated process. Those problems are hard to debug. The technique of caching the start time can prevent that case; though it has AFAIK not been a problem. How would that work? User should, before issuing the kill, compare start time of process to kill with cached start time? Note even if your coding is bulletproof, that allChildren() will also return PIDs of sub processes which are completely unrelated to you and Process.java - they could have been forked by some third party native code which just happens to run in parallel in the same process. There, you have no control about when it gets reaped. It might already have been reaped by the time allChildren() returns, and now the same PID got recycled as another, unrelated process. Of course, the best case is for an application to spawn and manage its own processes and handle there proper termination. The use cases for children/allChildren are focused on supervisory/executive functions that monitor a running system and can cleanup even in the case of unexpected failures. All management of processes is subject to OS limitations, if the PID were from a completely different process tree, the ordinary destroy/info functions would not be available unless the process
Re: RFR 9: 8077350 Process API Updates Implementation Review
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() will return false, but the process exists, which strictly spoken is a lie. Caller may base actions on this, e.g. try to clean up resources for a process he thinks is dead. We actually had this problem, and our version of isAlive() grew to be more elaborate over time. Basically, we do: 1) first try kill(0). If EPERM: 2) try read /proc on OSes which have /proc. 3) do a system(ps). Alternativly, one may report EPERM to the caller - with an exception or an extension of the return value - and leave it up to him what to do. Kind Regards, Thomas On Thu, Apr 9, 2015 at 10:00 PM, Roger Riggs roger.ri...@oracle.com 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
Re: RFR 9: 8077350 Process API Updates Implementation Review
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/pid/stat) and Solaris (/proc/pid/status) it is not necessary to open those special files and read them. Just doing stat() on them and using the st_mtime will get you the process start time. I see AIX shares native code with Linux (unix), so perhaps AIX does the same. Mac OSX and Windows have special calls... In case OS does not allow retrieving the start time (not owner or similar), it should be cached as some undefined value and treated the same when destroying. If while obtaining the ProcessHandle *and* while destorying the process, the start time of the process with a particular pid is undefined then there are two options: 1 - don't kill the process 2 - kill the process They might actually be no different as the reason of not being able to retrieve the start time (not owner) might prevent the process from being killed too, so option 2 could be used to allow killing on any hypothetical platforms that don't support retrieving start time and it is no worse than current implementation anyway. What do you think? Regards, Peter On 04/11/2015 08:55 PM, Roger Riggs wrote: 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 destroy a process tree.), by returning a collection of PIDs which are the children of the process and then killing them? Earlier versions included a killProcess tree method but it was recommended to leave the exact algorithm to kill processes to the caller. However, I am not sure that this can be implemented in a safe way, at least on UNIX, because - as Martin already pointed out - of PID recycling. I do not see how you can prevent allChildren() from returning PIDs which may be already reaped and recyled when you use them later. How do you prevent that? Unless there is an extended time between getting the children and destroying them the pids will still be valid. The technique of caching the start time can prevent that case; though it has AFAIK not been a problem. Note even if your coding is bulletproof, that allChildren() will also return PIDs of sub processes which are completely unrelated to you and Process.java - they could have been forked by some third party native code which just happens to run in parallel in the same process. There, you have no control about when it gets reaped. It might already have been reaped by the time allChildren() returns, and now the same PID got recycled as another, unrelated process. Of course, the best case is for an application to spawn and manage its own processes and handle there proper termination. The use cases for children/allChildren are focused on supervisory/executive functions that monitor a running system and can cleanup even in the case of unexpected failures. All management of processes is subject to OS limitations, if the PID were from a completely different process tree, the ordinary destroy/info functions would not be available unless the process was running as a privileged os user (same as any other native application). If I am right, it would not be sufficient to state There is no guarantee that a process is alive. - it may be alive but by now be a different process altogether. This makes allChildren() useless for many cases, because the returned information may already be obsolete the moment the function returns. The caching of startTime can remove the ambiguity. Of course I may something missing here? But if I got all that right and the sole purpose of allChildren() is to be able to kill them (or otherwise signal them), why not use process groups? Process groups would be the traditional way on POSIX platforms to handle process trees, and they are also available on Windows in the form of Job Objects. Using process groups to signal sub process trees would be safe, would not rely on PID identity, and would be more efficient. Also way less coding. Also, it would be an old, established pattern - process groups have been around for a long time. Also, using process groups it is possible to break away from a group, so a program below you which wants to run as a demon can do so by removing itself from the process group and thus escaping your kill. On Windows we have Job objects, and I think there are enough similarities to POSIX process groups to abstract them into something platform independent. Earlier discussions of process termination and exit value reaping considered using process groups but it became evident that the Java runtime needed to be very careful to not
RFR: JDK-8074859 Turn on warnings as error
With JDK-8074096, the number of warnings in the product was reduced to a minimum. This enables the next step, which is turning on the respective compiler flags that turns warnings into errors. In the long run, this is the only way to keep the warnings from creeping back. Even with JDK-8074096, the product does not build 100% warning free. This is due to some warnings that cannot be disabled, or (in one case) where C and C++ code is mixed, and warnings for both languages cannot be used. A system similar to the one introduced in JDK-8074096 is therefore needed, in which individual libraries can be exempted from this flag, until such warnings are fixed. A library can thus disable warnings as errors with WARNINGS_AS_ERRORS := false, or (better) use a toolchain-specific version, e.g WARNINGS_AS_ERRORS_gcc := false. This is intended as a temporary measure, though. The long-term solution is reasonably to fix the warnings and remove that argument. Also, different versions of compilers can generate a different set of warnings. It is therefore necessary to be able to turn off this globally. Therefor a new flag for configure is introduced: --disable-warnings-as-errors. While the code compiles without errors on the build systems used internally at Oracle, this might not be the case on other combinations of operating system versions and toolchain versions. To facilitate for unexpecting developers, a help message is added if the build fails, that suggests using --disable-warnings-as-errors. This solution was chosen as a compromise between the hard core solution of turning on warnings as errors by default for anyone, and the cowar... erm, conservative solution of checking if the compiler versions exactly match what's used inside Oracle (and therefore regularly tested), and only turn it on in that case. Similarly to JDK-8074096, I intend to file follow-up bugs for each individual library that got a WARNINGS_AS_ERRORS_* := false. Bug: https://bugs.openjdk.java.net/browse/JDK-8074859 WebRev for top: http://cr.openjdk.java.net/~ihse/JDK-8074859-warnings-as-errors-top/webrev.01 WebRev for jdk: http://cr.openjdk.java.net/~ihse/JDK-8074859-warnings-as-errors-jdk/webrev.01 Some comments: * I needed to add a few more DISABLED_WARNINGS. For windows, this is most likely due to the recent compiler change. For other libraries, I'm not sure, but it might well be the result of recent changes that has introduced new warnings. If so, it highlights the need of this patch to keep the build warning free. * For a few libraries and toolchains, there is *both* WARNINGS_AS_ERRORS := false and a DISABLED_WARNINGS list. This is the case if not all warnings are possible to disable. * I have removed some incorrect uses of SHARED_LIBRARY_FLAGS. This is included in our JDK LDFLAGS, so it should not be set separately, and definitely not as CFLAGS. (This caused compiler warnings, which now turned into errors.) However, a more suitable long-term solution is probably to move the knowledge of how to create shared libraries more specifically into SetupNativeCompilation, and not set it as part of the JDK flags. /Magnus
RFR: 8078057: Reapply fixes for 8073361, 8073374, 8073696
Hello, The latest JAXWS [1] integration to JDK9 reverted back three bug fixes in JAXWS repository: https://bugs.openjdk.java.net/browse/JDK-8073374 https://bugs.openjdk.java.net/browse/JDK-8073696 https://bugs.openjdk.java.net/browse/JDK-8073361 It was caused by skipped integration to upstream JAXB/WS projects. Now the fixes is applied to upstream projects and the proposed fix [2] reapplies jaxws parts of these fixes in jdk9. JPRT testing shows no regression tests failures. Please, review the proposed changes. With Best Regards, Aleksej [1] Latest JAXWS integration: https://bugs.openjdk.java.net/browse/JDK-8076549 [2] Webrev: http://cr.openjdk.java.net/~aefimov/8078057/00 [3] JBS issue: https://bugs.openjdk.java.net/browse/JDK-8078057
Re: RFR: 8078057: Reapply fixes for 8073361, 8073374, 8073696
The change looks good to me. thanks Mandy On 4/17/2015 6:31 AM, Aleksej Efimov wrote: Hello, The latest JAXWS [1] integration to JDK9 reverted back three bug fixes in JAXWS repository: https://bugs.openjdk.java.net/browse/JDK-8073374 https://bugs.openjdk.java.net/browse/JDK-8073696 https://bugs.openjdk.java.net/browse/JDK-8073361 It was caused by skipped integration to upstream JAXB/WS projects. Now the fixes is applied to upstream projects and the proposed fix [2] reapplies jaxws parts of these fixes in jdk9. JPRT testing shows no regression tests failures. Please, review the proposed changes. With Best Regards, Aleksej [1] Latest JAXWS integration: https://bugs.openjdk.java.net/browse/JDK-8076549 [2] Webrev: http://cr.openjdk.java.net/~aefimov/8078057/00 [3] JBS issue: https://bugs.openjdk.java.net/browse/JDK-8078057
Re: RFR: 8078057: Reapply fixes for 8073361, 8073374, 8073696
On 17/04/2015 14:31, Aleksej Efimov wrote: Hello, The latest JAXWS [1] integration to JDK9 reverted back three bug fixes in JAXWS repository: https://bugs.openjdk.java.net/browse/JDK-8073374 https://bugs.openjdk.java.net/browse/JDK-8073696 https://bugs.openjdk.java.net/browse/JDK-8073361 It was caused by skipped integration to upstream JAXB/WS projects. Now the fixes is applied to upstream projects and the proposed fix [2] reapplies jaxws parts of these fixes in jdk9. The re-apply looks good to me too. -Alan.
Re: RFR [9] 8077332: tidy warnings from javax/xml
Hello Joe, [jw] as I mentioned, pre/pre is needed for the code snippet. Fixed, please see http://cr.openjdk.java.net/~avstepan/8077332/webrev.01/jaxp/src/java.xml/share/classes/javax/xml/stream/XMLStreamReader.java.udiff.html [jw] I saw in a few cases where two @code tags are next to each other Fixed in a couple of places. Regards, Alexander On 16.04.2015 19:57, huizhe wang wrote: Hi Alexander, Looks very good. Thanks for making all the changes! Please note that for the JAXWS, you may need to check with JAXWS/Miran (miroslav@oracle.com). Changes to JAXWS generally goes into the standalone first. They do periodic integration. For the jaxp portion: --- old/src/java.xml/share/classes/javax/xml/datatype/Duration.java 2015-04-16 13:50:25.249473095 +0400 +++ new/src/java.xml/share/classes/javax/xml/datatype/Duration.java 2015-04-16 13:50:25.161473099 +0400 @@ -725,37 +725,37 @@ * - * @return the relationship between codethis/code codeDuration/codeand codeduration/code parameter as + * @return the relationship between {@code this} {@code Duration}and {@code duration} parameter as [jw] I saw in a few cases where two @code tags are next to each other, you may do a s/} {@code//g to combine them. A space is also missing before 'and': e.g. {@code Duration} and. --- old/src/java.xml/share/classes/javax/xml/stream/XMLStreamReader.java 2015-04-16 13:50:28.197472963 +0400 +++ new/src/java.xml/share/classes/javax/xml/stream/XMLStreamReader.java 2015-04-16 13:50:28.105472967 +0400 @@ -542,7 +543,7 @@ * If the number of characters actually copied is less than the length, then there is no more text. * Otherwise, subsequent calls need to be made until all text has been retrieved. For example: * - *code + * {@code * int length = 1024; * char[] myBuffer = new char[ length ]; * @@ -553,7 +554,7 @@ * if (nCopied length) * break; * } - * /code + * } [jw] as I mentioned, pre/pre is needed for the code snippet. BTW, have you compiled and verified the Javadoc after the changes? Thanks, Joe On 4/16/2015 7:07 AM, alexander stepanov wrote: I'm sorry, two extra files touched - http://cr.openjdk.java.net/~avstepan/8077332/webrev.01/jaxws/src/java.activation/share/classes/javax/activation/MailcapCommandMap.java.udiff.html http://cr.openjdk.java.net/~avstepan/8077332/webrev.01/jaxws/src/java.activation/share/classes/javax/activation/MimetypesFileTypeMap.java.udiff.html Hopefully that's all for this bug... Thanks, Alexander On 16.04.2015 15:48, alexander stepanov wrote: Please note also that a couple of new files were touched: http://cr.openjdk.java.net/~avstepan/8077332/webrev.01/jaxws/src/java.annotations.common/share/classes/javax/annotation/PostConstruct.java.udiff.html http://cr.openjdk.java.net/%7Eavstepan/8077332/webrev.01/jaxws/src/java.annotations.common/share/classes/javax/annotation/PostConstruct.java.udiff.html http://cr.openjdk.java.net/~avstepan/8077332/webrev.01/jaxws/src/java.annotations.common/share/classes/javax/annotation/PreDestroy.java.udiff.html http://cr.openjdk.java.net/%7Eavstepan/8077332/webrev.01/jaxws/src/java.annotations.common/share/classes/javax/annotation/PreDestroy.java.udiff.html On 15.04.2015 19:12, alexander stepanov wrote: Hello Joe, The copyright changes were reverted. Please review the updated fix: http://cr.openjdk.java.net/~avstepan/8077332/webrev.01/ (code/code replaced with {@code}, removed unnecessary /p, used @literal tag). Thanks, Alexander On 13.04.2015 21:19, huizhe wang wrote: On 4/13/2015 4:42 AM, Alan Bateman wrote: On 13/04/2015 12:22, alexander stepanov wrote: Hello Joe, Thank you for the notes; Copyright year shall not be changed. That seems to be a bit controversial point; sometimes (while cleaning docs) I was asked to do that, other times - not to do that. Our internal policy seemingly assigns to change the 2nd date every time the sources were touched (but that may be a question of ambiguous interpretation). But of course I can easily revert these changes if you're totally sure it should be done. This has always been confusing. Some areas insist on updating the copyright dates, others don't. AFAIK, it has always been optional. I think the original assumption was that the update_copyright_year script (in the top-level repo) be run periodically to do bulk updates. Unfortunately that script doesn't seem to be run very often now and this strengthens the case to update the dates on a continuous basis. I have not come across the argument that html tidy tasks that don't change the javadoc should not update the copyright date. The general topic probably should move to jdk9-dev and get this decided once and documented in the developer guide. I think the key question to ask is: is this the code I can claim Copyright with? To me, format, code style, html tags and other minor changes, these are not code changes one can claim
Re: RFR: 8078057: Reapply fixes for 8073361, 8073374, 8073696
Mandy, Alan, Thanks for reviews. With Best Regards, Aleksej On 04/17/2015 04:58 PM, Alan Bateman wrote: On 17/04/2015 14:31, Aleksej Efimov wrote: Hello, The latest JAXWS [1] integration to JDK9 reverted back three bug fixes in JAXWS repository: https://bugs.openjdk.java.net/browse/JDK-8073374 https://bugs.openjdk.java.net/browse/JDK-8073696 https://bugs.openjdk.java.net/browse/JDK-8073361 It was caused by skipped integration to upstream JAXB/WS projects. Now the fixes is applied to upstream projects and the proposed fix [2] reapplies jaxws parts of these fixes in jdk9. The re-apply looks good to me too. -Alan.
Re: RFR 9: 8077350 Process API Updates Implementation Review
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, though it does raise a question about how to specify PH.equals(). Either the start time is explicitly mentioned (and is repeatable and testable) or it is vague and refers to some implementation specific value that uniquely identifies the process; and is less testable. At least on Linux (/proc/pid/stat) and Solaris (/proc/pid/status) it is not necessary to open those special files and read them. Just doing stat() on them and using the st_mtime will get you the process start time. I see AIX shares native code with Linux (unix), so perhaps AIX does the same. Mac OSX and Windows have special calls... That is less expensive. But opening /proc/pid/stat is necessary for allChildren to get and filter on the parent and can provide the starttime as well. stat would be useful for allProcesses which does not need the parent. In case OS does not allow retrieving the start time (not owner or similar), it should be cached as some undefined value and treated the same when destroying. If while obtaining the ProcessHandle *and* while destroying the process, the start time of the process with a particular pid is undefined then there are two options: 1 - don't kill the process 2 - kill the process They might actually be no different as the reason of not being able to retrieve the start time (not owner) might prevent the process from being killed too, so option 2 could be used to allow killing on any hypothetical platforms that don't support retrieving start time and it is no worse than current implementation anyway. Destroy is a best-effort method; it is not guaranteed to result in termination, though the docs are a bit weak on this point. I'd go for option 2, anyone using the API is looking for results on processes they already know something about (mostly), so there's no reason to be particularly conservative about. If the caller is trying to be more conservative, they can maintain their own extra information about the processes they are managing. What do you think? I'd like to pick this up as a separate change, and get the current one in first. Roger
Re: RFR 9: 8077350 Process API Updates Implementation Review
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 of the process before killing it. Yes, though it does raise a question about how to specify PH.equals(). Either the start time is explicitly mentioned (and is repeatable and testable) or it is vague and refers to some implementation specific value that uniquely identifies the process; and is less testable. Even with timestamps though, you cannot prevent the PID from being reused in the time window after you verify its timestamp but before you kill it unless it is a direct child process (on UNIX anyway; who knows what Windows does...). I think that this inevitable race should be mentioned in the docs regardless of whether the timestamp thing is implemented (or doc'd) to prevent unrealistic expectations (the api draft link seems to be dead so I didn't see if it already includes this kind of language). At least on Linux (/proc/pid/stat) and Solaris (/proc/pid/status) it is not necessary to open those special files and read them. Just doing stat() on them and using the st_mtime will get you the process start time. I see AIX shares native code with Linux (unix), so perhaps AIX does the same. Mac OSX and Windows have special calls... That is less expensive. But opening /proc/pid/stat is necessary for allChildren to get and filter on the parent and can provide the starttime as well. stat would be useful for allProcesses which does not need the parent. In case OS does not allow retrieving the start time (not owner or similar), it should be cached as some undefined value and treated the same when destroying. If while obtaining the ProcessHandle *and* while destroying the process, the start time of the process with a particular pid is undefined then there are two options: 1 - don't kill the process 2 - kill the process They might actually be no different as the reason of not being able to retrieve the start time (not owner) might prevent the process from being killed too, so option 2 could be used to allow killing on any hypothetical platforms that don't support retrieving start time and it is no worse than current implementation anyway. Destroy is a best-effort method; it is not guaranteed to result in termination, though the docs are a bit weak on this point. I'd go for option 2, anyone using the API is looking for results on processes they already know something about (mostly), so there's no reason to be particularly conservative about. If the caller is trying to be more conservative, they can maintain their own extra information about the processes they are managing. What do you think? I'd like to pick this up as a separate change, and get the current one in first. Roger -- - DML
Re: RFR [9] 8077332: tidy warnings from javax/xml
Hi Alexander, That fixed the issue in the existing Javadoc. The JAXP changes look good now. Thanks for doing this! Best, Joe On 4/17/2015 4:36 AM, alexander stepanov wrote: Hello Joe, [jw] as I mentioned, pre/pre is needed for the code snippet. Fixed, please see http://cr.openjdk.java.net/~avstepan/8077332/webrev.01/jaxp/src/java.xml/share/classes/javax/xml/stream/XMLStreamReader.java.udiff.html [jw] I saw in a few cases where two @code tags are next to each other Fixed in a couple of places. Regards, Alexander On 16.04.2015 19:57, huizhe wang wrote: Hi Alexander, Looks very good. Thanks for making all the changes! Please note that for the JAXWS, you may need to check with JAXWS/Miran (miroslav@oracle.com). Changes to JAXWS generally goes into the standalone first. They do periodic integration. For the jaxp portion: --- old/src/java.xml/share/classes/javax/xml/datatype/Duration.java 2015-04-16 13:50:25.249473095 +0400 +++ new/src/java.xml/share/classes/javax/xml/datatype/Duration.java 2015-04-16 13:50:25.161473099 +0400 @@ -725,37 +725,37 @@ * - * @return the relationship between codethis/code codeDuration/codeand codeduration/code parameter as + * @return the relationship between {@code this} {@code Duration}and {@code duration} parameter as [jw] I saw in a few cases where two @code tags are next to each other, you may do a s/} {@code//g to combine them. A space is also missing before 'and': e.g. {@code Duration} and. --- old/src/java.xml/share/classes/javax/xml/stream/XMLStreamReader.java 2015-04-16 13:50:28.197472963 +0400 +++ new/src/java.xml/share/classes/javax/xml/stream/XMLStreamReader.java 2015-04-16 13:50:28.105472967 +0400 @@ -542,7 +543,7 @@ * If the number of characters actually copied is less than the length, then there is no more text. * Otherwise, subsequent calls need to be made until all text has been retrieved. For example: * - *code + * {@code * int length = 1024; * char[] myBuffer = new char[ length ]; * @@ -553,7 +554,7 @@ * if (nCopied length) * break; * } - * /code + * } [jw] as I mentioned, pre/pre is needed for the code snippet. BTW, have you compiled and verified the Javadoc after the changes? Thanks, Joe On 4/16/2015 7:07 AM, alexander stepanov wrote: I'm sorry, two extra files touched - http://cr.openjdk.java.net/~avstepan/8077332/webrev.01/jaxws/src/java.activation/share/classes/javax/activation/MailcapCommandMap.java.udiff.html http://cr.openjdk.java.net/~avstepan/8077332/webrev.01/jaxws/src/java.activation/share/classes/javax/activation/MimetypesFileTypeMap.java.udiff.html Hopefully that's all for this bug... Thanks, Alexander On 16.04.2015 15:48, alexander stepanov wrote: Please note also that a couple of new files were touched: http://cr.openjdk.java.net/~avstepan/8077332/webrev.01/jaxws/src/java.annotations.common/share/classes/javax/annotation/PostConstruct.java.udiff.html http://cr.openjdk.java.net/%7Eavstepan/8077332/webrev.01/jaxws/src/java.annotations.common/share/classes/javax/annotation/PostConstruct.java.udiff.html http://cr.openjdk.java.net/~avstepan/8077332/webrev.01/jaxws/src/java.annotations.common/share/classes/javax/annotation/PreDestroy.java.udiff.html http://cr.openjdk.java.net/%7Eavstepan/8077332/webrev.01/jaxws/src/java.annotations.common/share/classes/javax/annotation/PreDestroy.java.udiff.html On 15.04.2015 19:12, alexander stepanov wrote: Hello Joe, The copyright changes were reverted. Please review the updated fix: http://cr.openjdk.java.net/~avstepan/8077332/webrev.01/ (code/code replaced with {@code}, removed unnecessary /p, used @literal tag). Thanks, Alexander On 13.04.2015 21:19, huizhe wang wrote: On 4/13/2015 4:42 AM, Alan Bateman wrote: On 13/04/2015 12:22, alexander stepanov wrote: Hello Joe, Thank you for the notes; Copyright year shall not be changed. That seems to be a bit controversial point; sometimes (while cleaning docs) I was asked to do that, other times - not to do that. Our internal policy seemingly assigns to change the 2nd date every time the sources were touched (but that may be a question of ambiguous interpretation). But of course I can easily revert these changes if you're totally sure it should be done. This has always been confusing. Some areas insist on updating the copyright dates, others don't. AFAIK, it has always been optional. I think the original assumption was that the update_copyright_year script (in the top-level repo) be run periodically to do bulk updates. Unfortunately that script doesn't seem to be run very often now and this strengthens the case to update the dates on a continuous basis. I have not come across the argument that html tidy tasks that don't change the javadoc should not update the copyright date. The general topic probably should move to jdk9-dev and get this decided once and documented in the developer guide. I
RFR: 8071968: javax/xml/ws/8046817/GenerateEnumSchema.java failed on Windows platform
Hello, Please, review an exclusion of GenerateEnumSchema.java from ProblemList.txt [1]. The problem was fixed in upstream JAXWS project and was bringed over to JDK as part of sync-up process [2]. The test passes on all platforms after exclusion on JDK9 JPRT builds. With Best Regards, Aleksej [1] Webrev: http://cr.openjdk.java.net/~aefimov/8071968/9/00 [2] JAXWS integration: https://bugs.openjdk.java.net/browse/JDK-8076549 [3] JBS: https://bugs.openjdk.java.net/browse/JDK-8071968
Re: RFR: 8071968: javax/xml/ws/8046817/GenerateEnumSchema.java failed on Windows platform
Hi Lance, Correct - bug is fixed by [2], just removing the test from problem list. Thank you for the review. Best Regards, Aleksej On 04/17/2015 07:44 PM, Lance Andersen wrote: Hi Aleksej, To be clear, you are just removing the test from the ProblemList.txt as the bug has since been fixed. The change looks OK. Best Lance On Apr 17, 2015, at 12:29 PM, Aleksej Efimov aleksej.efi...@oracle.com mailto:aleksej.efi...@oracle.com wrote: Hello, Please, review an exclusion of GenerateEnumSchema.java from ProblemList.txt [1]. The problem was fixed in upstream JAXWS project and was bringed over to JDK as part of sync-up process [2]. The test passes on all platforms after exclusion on JDK9 JPRT builds. With Best Regards, Aleksej [1] Webrev: http://cr.openjdk.java.net/~aefimov/8071968/9/00 http://cr.openjdk.java.net/%7Eaefimov/8071968/9/00 [2] JAXWS integration: https://bugs.openjdk.java.net/browse/JDK-8076549 [3] JBS: https://bugs.openjdk.java.net/browse/JDK-8071968 http://oracle.com/us/design/oracle-email-sig-198324.gif http://oracle.com/us/design/oracle-email-sig-198324.gifhttp://oracle.com/us/design/oracle-email-sig-198324.gif http://oracle.com/us/design/oracle-email-sig-198324.gifLance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com mailto:lance.ander...@oracle.com
Re: RFR [9] 8077332: tidy warnings from javax/xml
Thanks! Regards, Alexander On 17.04.2015 19:33, huizhe wang wrote: Hi Alexander, That fixed the issue in the existing Javadoc. The JAXP changes look good now. Thanks for doing this! Best, Joe On 4/17/2015 4:36 AM, alexander stepanov wrote: Hello Joe, [jw] as I mentioned, pre/pre is needed for the code snippet. Fixed, please see http://cr.openjdk.java.net/~avstepan/8077332/webrev.01/jaxp/src/java.xml/share/classes/javax/xml/stream/XMLStreamReader.java.udiff.html [jw] I saw in a few cases where two @code tags are next to each other Fixed in a couple of places. Regards, Alexander On 16.04.2015 19:57, huizhe wang wrote: Hi Alexander, Looks very good. Thanks for making all the changes! Please note that for the JAXWS, you may need to check with JAXWS/Miran (miroslav@oracle.com). Changes to JAXWS generally goes into the standalone first. They do periodic integration. For the jaxp portion: --- old/src/java.xml/share/classes/javax/xml/datatype/Duration.java 2015-04-16 13:50:25.249473095 +0400 +++ new/src/java.xml/share/classes/javax/xml/datatype/Duration.java 2015-04-16 13:50:25.161473099 +0400 @@ -725,37 +725,37 @@ * - * @return the relationship between codethis/code codeDuration/codeand codeduration/code parameter as + * @return the relationship between {@code this} {@code Duration}and {@code duration} parameter as [jw] I saw in a few cases where two @code tags are next to each other, you may do a s/} {@code//g to combine them. A space is also missing before 'and': e.g. {@code Duration} and. --- old/src/java.xml/share/classes/javax/xml/stream/XMLStreamReader.java 2015-04-16 13:50:28.197472963 +0400 +++ new/src/java.xml/share/classes/javax/xml/stream/XMLStreamReader.java 2015-04-16 13:50:28.105472967 +0400 @@ -542,7 +543,7 @@ * If the number of characters actually copied is less than the length, then there is no more text. * Otherwise, subsequent calls need to be made until all text has been retrieved. For example: * - *code + * {@code * int length = 1024; * char[] myBuffer = new char[ length ]; * @@ -553,7 +554,7 @@ * if (nCopied length) * break; * } - * /code + * } [jw] as I mentioned, pre/pre is needed for the code snippet. BTW, have you compiled and verified the Javadoc after the changes? Thanks, Joe On 4/16/2015 7:07 AM, alexander stepanov wrote: I'm sorry, two extra files touched - http://cr.openjdk.java.net/~avstepan/8077332/webrev.01/jaxws/src/java.activation/share/classes/javax/activation/MailcapCommandMap.java.udiff.html http://cr.openjdk.java.net/~avstepan/8077332/webrev.01/jaxws/src/java.activation/share/classes/javax/activation/MimetypesFileTypeMap.java.udiff.html Hopefully that's all for this bug... Thanks, Alexander On 16.04.2015 15:48, alexander stepanov wrote: Please note also that a couple of new files were touched: http://cr.openjdk.java.net/~avstepan/8077332/webrev.01/jaxws/src/java.annotations.common/share/classes/javax/annotation/PostConstruct.java.udiff.html http://cr.openjdk.java.net/%7Eavstepan/8077332/webrev.01/jaxws/src/java.annotations.common/share/classes/javax/annotation/PostConstruct.java.udiff.html http://cr.openjdk.java.net/~avstepan/8077332/webrev.01/jaxws/src/java.annotations.common/share/classes/javax/annotation/PreDestroy.java.udiff.html http://cr.openjdk.java.net/%7Eavstepan/8077332/webrev.01/jaxws/src/java.annotations.common/share/classes/javax/annotation/PreDestroy.java.udiff.html On 15.04.2015 19:12, alexander stepanov wrote: Hello Joe, The copyright changes were reverted. Please review the updated fix: http://cr.openjdk.java.net/~avstepan/8077332/webrev.01/ (code/code replaced with {@code}, removed unnecessary /p, used @literal tag). Thanks, Alexander On 13.04.2015 21:19, huizhe wang wrote: On 4/13/2015 4:42 AM, Alan Bateman wrote: On 13/04/2015 12:22, alexander stepanov wrote: Hello Joe, Thank you for the notes; Copyright year shall not be changed. That seems to be a bit controversial point; sometimes (while cleaning docs) I was asked to do that, other times - not to do that. Our internal policy seemingly assigns to change the 2nd date every time the sources were touched (but that may be a question of ambiguous interpretation). But of course I can easily revert these changes if you're totally sure it should be done. This has always been confusing. Some areas insist on updating the copyright dates, others don't. AFAIK, it has always been optional. I think the original assumption was that the update_copyright_year script (in the top-level repo) be run periodically to do bulk updates. Unfortunately that script doesn't seem to be run very often now and this strengthens the case to update the dates on a continuous basis. I have not come across the argument that html tidy tasks that don't change the javadoc should not update the copyright date. The general topic probably should move to jdk9-dev and
Re: RFR 9: 8077350 Process API Updates Implementation Review
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 this call, this may fail with EPERM. In this case, isAlive() will return false, but the process exists, which strictly spoken is a lie. Caller may base actions on this, e.g. try to clean up resources for a process he thinks is dead. We actually had this problem, and our version of isAlive() grew to be more elaborate over time. Basically, we do: 1) first try kill(0). If EPERM: 2) try read /proc on OSes which have /proc. 3) do a system(ps). Alternativly, one may report EPERM to the caller - with an exception or an extension of the return value - and leave it up to him what to do. Good to have the experienced input. I'd been trying to keep the code simple but it seems more complexity is needed to be robust. I'm not sure I'd go as far as invoking ps; it too might fail for a number of reasons. That would be a good case to throw an exception if the liveness cannot be determined. Thanks, Roger Kind Regards, Thomas On Thu, Apr 9, 2015 at 10:00 PM, Roger Riggs roger.ri...@oracle.com mailto:roger.ri...@oracle.com 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/ http://cr.openjdk.java.net/%7Erriggs/ph-apidraft/ The webrev: http://cr.openjdk.java.net/~rriggs/webrev-ph http://cr.openjdk.java.net/%7Erriggs/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
Re: RFR 9: 8077350 Process API Updates Implementation Review
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 to a specific tests (similar to jtreg tests) in case of errors or hangs. We have test machines running large workloads of tests in parallel and we reach pid wraparound - depending on the OS - quite fast. We solved this by adding process groups to Process.java and we are very happy with this solution. We are able to quickly kill a whole process tree, cleanly and completely, without ambiguity or risk to other tests. Of course we had to add this support as a sideways hack in order to not change the official Process.java interface. Therefore I was hoping that with JEP 102, we would get official support for process groups. Unfortunately, seems the decision is already done and we are too late in the discussion :( It would be interesting to see a description of what you added to/around the API. The reason to avoid them was one of simplicity and non-interference with processes spawned by native libraries. If that complexity can be understood process groups/jobs could fulfill a need in a scalable system. At this point, I'd like to deal with it as a separate request for enhancement. see my other comments inline. On Sat, Apr 11, 2015 at 8:55 PM, Roger Riggs roger.ri...@oracle.com mailto:roger.ri...@oracle.com wrote: 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 destroy a process tree.), by returning a collection of PIDs which are the children of the process and then killing them? Earlier versions included a killProcess tree method but it was recommended to leave the exact algorithm to kill processes to the caller. However, I am not sure that this can be implemented in a safe way, at least on UNIX, because - as Martin already pointed out - of PID recycling. I do not see how you can prevent allChildren() from returning PIDs which may be already reaped and recyled when you use them later. How do you prevent that? Unless there is an extended time between getting the children and destroying them the pids will still be valid. Why? Child process may be getting reaped the instant you are done reading it from /proc, and pid may have been recycled by the OS right away and already pointing to another process when allChildren() returns. If a process lives about as long as it takes the system to reach a pid wraparound to the same pid value, its pid could be recycled right after it is reaped, or? Sure, the longer you wait, the higher the chance of this to happen, but it may happen right away. As Martin said, we had those races in the kill() code since a long time, but children()/allChildren() could make those error more probable, because now more processes are involved. Especially if you use allChildren to kill a deep process tree. And there is nothing in the javadoc warning the user about this scenario. You would just happen from time to time to kill an unrelated process. Those problems are hard to debug. The technique of caching the start time can prevent that case; though it has AFAIK not been a problem. How would that work? User should, before issuing the kill, compare start time of process to kill with cached start time? See Peter's email, he described it more thoroughly that I have in previous emails. Note even if your coding is bulletproof, that allChildren() will also return PIDs of sub processes which are completely unrelated to you and Process.java - they could have been forked by some third party native code which just happens to run in parallel in the same process. There, you have no control about when it gets reaped. It might already have been reaped by the time allChildren() returns, and now the same PID got recycled as another, unrelated process. Of course, the best case is for an application to spawn and manage its own processes and handle there proper termination. The use cases for children/allChildren are focused on supervisory/executive functions that monitor a running system and can cleanup even in the case of unexpected failures. All management of processes is subject to OS limitations, if the PID were from a completely different process tree, the ordinary destroy/info functions would not be available unless the process was running as a privileged os user (same as any other native application). Could you explain this please? If both trees run
Re: RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)
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/destroyForcible descriptions have more consistent descriptions - ProcessHandle.destroy now returns ProcessHandle to enable fluent use. - Corrected description of default implementation ProcessHandle.onExit 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. Two issues raised have been filed to be fixed after the current commit: - JDK-8078099 https://bugs.openjdk.java.net/browse/JDK-8078099 (process) ProcessHandle should uniquely identify processes - JDK-8078108 https://bugs.openjdk.java.net/browse/JDK-8078108 (process) ProcessHandle.isAlive should be robust
Re: RFR 9: 8048264 : StringBuffer's codePoint methods throw unspecified IndexOutOfBoundsException
On 4/14/15 12:10 AM, Alan Bateman wrote: On 13/04/2015 23:49, Brent Christian wrote: Hello, Please review this small javadoc change. It was discovered that some codePoint-related methods in StringBuffer are missing documentation for throwing IndexOutOfBoundsException. The methods are: codePointAt(int) codePointBefore(int) codePointCount(int,int) offsetByCodePoints(int,int) The StringBuilder JavaDoc does have @throws tags for those methods. This looks okay. Do you have cycles to check if there are any more of these? It seems that every few years we find more cases where we have missed the inheritDoc. I'll see if I can find some time to check around. Thanks, -Brent
Re: RFR 9: 8077350 Process API Updates Implementation Review
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 would then use pid *and* start time to check the identity of the process before killing it. Yes, though it does raise a question about how to specify PH.equals(). Either the start time is explicitly mentioned (and is repeatable and testable) or it is vague and refers to some implementation specific value that uniquely identifies the process; and is less testable. Even with timestamps though, you cannot prevent the PID from being reused in the time window after you verify its timestamp but before you kill it unless it is a direct child process (on UNIX anyway; who knows what Windows does...). I think that this inevitable race should be mentioned in the docs regardless of whether the timestamp thing is implemented (or doc'd) to prevent unrealistic expectations (the api draft link seems to be dead so I didn't see if it already includes this kind of language). I can add a note about race conditions to the existing class level javadoc about process information changing asynchronously. Race conditions can exist between checking the status of a process and acting upon it. But I'm still struck by the oddity of Java needing to provide a better interface to processes than the native OS does. In the native OS, a pid is a pid and the only handle given to applications. So both the app and the os need to be conservative about pid re-use. The link[1] was broken while I replaced it with an update reflecting the recent comments. Thanks, Roger [1] http://cr.openjdk.java.net/~rriggs/phdoc/ At least on Linux (/proc/pid/stat) and Solaris (/proc/pid/status) it is not necessary to open those special files and read them. Just doing stat() on them and using the st_mtime will get you the process start time. I see AIX shares native code with Linux (unix), so perhaps AIX does the same. Mac OSX and Windows have special calls... That is less expensive. But opening /proc/pid/stat is necessary for allChildren to get and filter on the parent and can provide the starttime as well. stat would be useful for allProcesses which does not need the parent. In case OS does not allow retrieving the start time (not owner or similar), it should be cached as some undefined value and treated the same when destroying. If while obtaining the ProcessHandle *and* while destroying the process, the start time of the process with a particular pid is undefined then there are two options: 1 - don't kill the process 2 - kill the process They might actually be no different as the reason of not being able to retrieve the start time (not owner) might prevent the process from being killed too, so option 2 could be used to allow killing on any hypothetical platforms that don't support retrieving start time and it is no worse than current implementation anyway. Destroy is a best-effort method; it is not guaranteed to result in termination, though the docs are a bit weak on this point. I'd go for option 2, anyone using the API is looking for results on processes they already know something about (mostly), so there's no reason to be particularly conservative about. If the caller is trying to be more conservative, they can maintain their own extra information about the processes they are managing. What do you think? I'd like to pick this up as a separate change, and get the current one in first. Roger
Optional.orElseChain ?
Hi guys, I was trying to write a code that uses Optional and I think one method is missing. Let suppose I want to load a type (like a class, an interface, etc) that can come either by reflection, or by using ASM. I will write an interface TypeProvider that is able to load a Type and i will chain the different type providers like this: TypeProvider asmTypeProvider = ... TypeProvider reflectionTypeProvider = ... TypeProvider provider = asmTypeProvider.chain(reflectionTypeProvider).orFail(); so I've implemented TypeProvider like this: public interface TypeProvider { public OptionalType loadType(String name); public default TypeProvider chain(TypeProvider provider) { return name - { OptionalType type = loadType(name); return type.isPresent()? type: provider.loadType(name); }; } public default TypeProvider orFail() { return chain(fail()); } public static TypeProvider fail() { return name - Optional.empty(); } } As you can see the code is not bad but the code of chain() could be simplified if there was a way on Optional to call a Supplier of Optional if an Optional is empty. Currently, orElse() takes a value, orElseGet takes a lambda that will return a value and there is no method that takes a lambda and return an Optional (like flatMap but but with a supplier that will be called if the Optional is empty). If we add the method orElseChain(Supplier? extends OptionalT supplier) perhaps with a better name ?, then the code of chain is better: public default TypeProvider chain(TypeProvider provider) { return name - loadType(name).orElseChain(() - provider.loadType(name)); } Am i the only one to think that this method is missing ? regards, Rémi
Re: Optional.orElseChain ?
On Apr 17, 2015, at 2:37 PM, Remi Forax fo...@univ-mlv.fr wrote: As you can see the code is not bad but the code of chain() could be simplified if there was a way on Optional to call a Supplier of Optional if an Optional is empty. Currently, orElse() takes a value, orElseGet takes a lambda that will return a value and there is no method that takes a lambda and return an Optional (like flatMap but but with a supplier that will be called if the Optional is empty). If we add the method orElseChain(Supplier? extends OptionalT supplier) perhaps with a better name ?, then the code of chain is better: public default TypeProvider chain(TypeProvider provider) { return name - loadType(name).orElseChain(() - provider.loadType(name)); } Am i the only one to think that this method is missing ? We actually ran into the exact same problem, and wrote the following helper method: public static X OptionalX unlessOpt(@Nonnull OptionalX first, SupplierOptionalX second) { return first.isPresent() ? first : second.get(); } I don't think it's precisely the same as your solution, but it definitely indicates a missing method.
Re: RFR: 8071968: javax/xml/ws/8046817/GenerateEnumSchema.java failed on Windows platform
Hi Aleksej, To be clear, you are just removing the test from the ProblemList.txt as the bug has since been fixed. The change looks OK. Best Lance On Apr 17, 2015, at 12:29 PM, Aleksej Efimov aleksej.efi...@oracle.com wrote: Hello, Please, review an exclusion of GenerateEnumSchema.java from ProblemList.txt [1]. The problem was fixed in upstream JAXWS project and was bringed over to JDK as part of sync-up process [2]. The test passes on all platforms after exclusion on JDK9 JPRT builds. With Best Regards, Aleksej [1] Webrev: http://cr.openjdk.java.net/~aefimov/8071968/9/00 [2] JAXWS integration: https://bugs.openjdk.java.net/browse/JDK-8076549 [3] JBS: https://bugs.openjdk.java.net/browse/JDK-8071968 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: Optional.orElseChain ?
On Fri, Apr 17, 2015 at 03:01:29PM -0700, Steven Schlansker wrote: On Apr 17, 2015, at 2:37 PM, Remi Forax fo...@univ-mlv.fr wrote: As you can see the code is not bad but the code of chain() could be simplified if there was a way on Optional to call a Supplier of Optional if an Optional is empty. Currently, orElse() takes a value, orElseGet takes a lambda that will return a value and there is no method that takes a lambda and return an Optional (like flatMap but but with a supplier that will be called if the Optional is empty). If we add the method orElseChain(Supplier? extends OptionalT supplier) perhaps with a better name ?, then the code of chain is better: public default TypeProvider chain(TypeProvider provider) { return name - loadType(name).orElseChain(() - provider.loadType(name)); } Am i the only one to think that this method is missing ? We actually ran into the exact same problem, and wrote the following helper method: public static X OptionalX unlessOpt(@Nonnull OptionalX first, SupplierOptionalX second) { return first.isPresent() ? first : second.get(); } I don't think it's precisely the same as your solution, but it definitely indicates a missing method. There are similar discussion here: http://stackoverflow.com/questions/2456/get-value-from-one-optional-or-another and here: http://stackoverflow.com/questions/28818506/java-8-optional-orelse-optional -- Andreas