Re: RFR 9: 8077350 Process API Updates Implementation Review

2015-04-17 Thread Staffan Larsen

 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

2015-04-17 Thread Thomas Stüfe
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

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() 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

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/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

2015-04-17 Thread Magnus Ihse Bursie
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

2015-04-17 Thread Aleksej Efimov

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

2015-04-17 Thread Mandy Chung

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

2015-04-17 Thread Alan Bateman

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

2015-04-17 Thread alexander stepanov

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

2015-04-17 Thread Aleksej Efimov

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

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, 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

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 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

2015-04-17 Thread huizhe wang

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

2015-04-17 Thread Aleksej Efimov

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

2015-04-17 Thread Aleksej Efimov

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

2015-04-17 Thread alexander stepanov

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

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 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

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 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)

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/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

2015-04-17 Thread Brent Christian

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

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 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 ?

2015-04-17 Thread Remi Forax

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 ?

2015-04-17 Thread Steven Schlansker

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

2015-04-17 Thread Lance Andersen
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 ?

2015-04-17 Thread Andreas Lundblad
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