Re: RFR: 8132306: java/lang/ref/ReferenceEnqueue.java fails with RuntimeException: Error: poll() returned null; expected ref object
On 07/30/2015 03:56 PM, Kim Barrett wrote: New webrev, with both changes: http://cr.openjdk.java.net/~kbarrett/8132306/webrev.01/ Thanks for fixing this. The change looks fine to me. Sorry for the belated reply as I just got back from vacation. I agree that this patch includes both of your change and Peter's change. Mandy
Re: RFR(XS): 8133105: Fix getFinalAttributes() on Windows to handle more special cases
On Tue, Aug 11, 2015 at 6:38 AM, Roger Riggs roger.ri...@oracle.com wrote: Hi Volker, Looks fine. Thanks! Is there any way to test this? It seems like it needs a special file system state that would not be readily available. Yes, it's not easy to reproduce the problem. I've tried to create a file with the same attributes and access control lists like the offending PGP files but couldn't succeed to reproduce the problem with them. Even if I copy the offending file, the problem will not show on the copy any more. So there must be something special about these files but I couldn't find out what. Thanks, Roger On 8/10/15 9:57 AM, Volker Simonis wrote: Hi, can somebody please review this trivial fix? Thanks, Volker On Thu, Aug 6, 2015 at 5:27 PM, Volker Simonis volker.simo...@gmail.com wrote: Hi, can somebody please review the following small fix contributed by matthias.baes...@sap.com: http://cr.openjdk.java.net/~simonis/webrevs/2015/8133105/ https://bugs.openjdk.java.net/browse/JDK-8133105 Getting file attributes on Windows via GetFileAttributesExW() can fail for some special system files. There is already code in getFinalAttributes() which handles some of these special cases by using FindFirstFileW(). However there are still cases which are not covered until now. For example on PGP WDE (Whole Disk Encryption) – encrypted machines the test java/io/File/WinSpecialFiles still fails for PGP files like C:\pgpwde01. This small change fixes the issue. Thank you and best regards, Volker
Re: RFR [9] 8133188: docs: replace tt tags (obsolete in html5) for java.util
Hello Martin,
the changes were reverted for the following files:
java.base/share/classes/java/util/concurrent/CopyOnWriteArrayList.java
java.base/share/classes/java/util/concurrent/CopyOnWriteArraySet.java
java.base/share/classes/java/util/AbstractQueue.java
java.base/share/classes/java/util/Deque.java
java.base/share/classes/java/util/NavigableMap.java
java.base/share/classes/java/util/NavigableSet.java
webrev updated:
http://cr.openjdk.java.net/~avstepan/8133188/webrev.02
specdiff report (please update the web page):
http://cr.openjdk.java.net/~avstepan/8133188/specdiff.util/overview-summary.html
(the changes in concurrent are seemingly due to inherited documentation).
Regards,
Alexander
On 8/10/2015 5:28 PM, Martin Buchholz wrote:
Hi Alexander,
I've been hoping that someone does cleanups of the jdk as you are
doing now.
Some of these source files are part of jsr166 and are maintained
separately, in jsr166 CVS.
http://g.oswego.edu/dl/concurrency-interest/
And that has already been tt/code-cleaned (although it has been
done slightly differently) and will be integrated into jdk9 in an
upcoming merge. So please leave out:
concurrent/
*Queue.java
*Deque.java
Navigable*.java
On Mon, Aug 10, 2015 at 6:39 AM, Alexander Stepanov
alexander.v.stepa...@oracle.com
mailto:alexander.v.stepa...@oracle.com wrote:
Hello,
Could you please review the following fix:
http://cr.openjdk.java.net/~avstepan/8133188/webrev.01
http://cr.openjdk.java.net/%7Eavstepan/8133188/webrev.01
for
https://bugs.openjdk.java.net/browse/JDK-8133188
Just a next portion of tt tags removed.
specdiff report:
http://cr.openjdk.java.net/~avstepan/8133188/specdiff.util/overview-summary.html
http://cr.openjdk.java.net/%7Eavstepan/8133188/specdiff.util/overview-summary.html
- some constructs like
tt(o==nullnbsp;?nbsp;e==nullnbsp;:nbsp;o.equals(e))/tt
were replaced with {@code Objects.equals(o, e)}
(e.g.,
http://cr.openjdk.java.net/~avstepan/8133188/webrev.01/src/java.base/share/classes/java/util/LinkedList.java.udiff.html
http://cr.openjdk.java.net/%7Eavstepan/8133188/webrev.01/src/java.base/share/classes/java/util/LinkedList.java.udiff.html);
- please see JDK-8133188 comments.
Thanks,
Alexander
Re: [9] RFR: 8060717: [TESTBUG] Improve test coverage of MethodHandles.explicitCastArguments()
Kindly reminder.
06.08.2015 17:49, Konstantin Shefov пишет:
Please, look at the modified test
http://cr.openjdk.java.net/~kshefov/8060717/webrev.01/
-Konstantin
On 08/06/2015 02:06 PM, Konstantin Shefov wrote:
Hi Vladimir
Thanks for reviewing
On 08/06/2015 01:02 PM, Vladimir Ivanov wrote:
Konstantin,
Overall, looks good.
Why do you create a new ClassLoader here and not simply reference
them directly (they are loaded by application class loader)?
You a right. Because of class loader hierarchy, all Test* classes
in this case are loaded by app class loader as an ancestor of url
class loader, so it is the same as just reference them directly.
To make the Test* classes loaded by ucl, they need to be outside of
classpath, which will produce extra folders and .java files in test
workspace.
I think non-bcp to bcp test will play just the same role (work with
classes loaded by different class loaders).
So I will correct the code. I will add BCP-to-non-BCP
non-BCP-to-BCP cases and remove url classloader.
-Konstantin
+ public static void testNonBCPRef2Ref() throws Throwable {
+String testClassPath = System.getProperty(test.classes,.);
+URL[] classpath = {(new
File(testClassPath)).getCanonicalFile()
+.toURI().toURL()};
+URLClassLoader ucl = URLClassLoader.newInstance(classpath);
+Class testInterface = ucl.loadClass(THIS_CLASS.getSimpleName()
++ $TestInterface);
+Class testSuperClass =
ucl.loadClass(THIS_CLASS.getSimpleName()
++ $TestSuperClass);
+Class testSubClass1 = ucl.loadClass(THIS_CLASS.getSimpleName()
++ $TestSubClass1);
I see BCP-to-BCP non-BCP-to-non-BCP ref-to-ref cases covered. What
about BCP-to-non-BCP non-BCP-to-BCP cases?
Best regards,
Vladimir Ivanov
On 8/3/15 6:06 PM, Konstantin Shefov wrote:
Michael, thanks for reviewing!
Vladimir, could you take a look, please?
-Konstantin
On 08/02/2015 05:31 PM, Michael Haupt wrote:
Hi Konstantin,
Am 31.07.2015 um 18:37 schrieb Konstantin Shefov
konstantin.she...@oracle.com
mailto:konstantin.she...@oracle.com:
Please review a test improvement. Covered more cases for
MethodHandles.explicitCastArguments().
Bug: https://bugs.openjdk.java.net/browse/JDK-8060717
Webrev: http://cr.openjdk.java.net/~kshefov/8060717/webrev.00/
http://cr.openjdk.java.net/%7Ekshefov/8060717/webrev.00/
note that mine is a lower-case review and does not count, but: thumbs
up. The level of detail at which the API is tested improves
significantly with these changes.
Best,
Michael
--
Oracle http://www.oracle.com/
Dr. Michael Haupt | Principal Member of Technical Staff
Phone: +49 331 200 7277 | Fax: +49 331 200 7561
OracleJava Platform Group | LangTools Team | Nashorn
Oracle Deutschland B.V. Co. KG, Schiffbauergasse 14 | 14467
Potsdam,
Germany
Green Oracle http://www.oracle.com/commitment Oracle is committed
to developing practices and products that help protect the
environment
[JDK-8080741] sigsegv while heap dumping in java_lang_Class::signers(oopDesc*)+0x1b
Hello,
very similiar to JDK-8080741 we had a crash with 8u51 on Linux. It looks
like it is happening while dumping the heap in a out of memory condition.
The bug talks about it is happenign on constrained heap, but it looks more
like related to OutOfMemory dumps (in both cases).
https://bugs.openjdk.java.net/browse/JDK-8080741
I do have a corefile and a javacore hs_err file, butcant post it
publically. Do you think it is worth to reconsider opening that bug again
(its closed as incomplete currently)? Let me know if you need more extracts
from the log.
# Java VM: Java HotSpot(TM) 64-Bit Server VM (25.51-b03 mixed mode
linux-amd64 compressed oops)
# Problematic frame:
# V [libjvm.so+0x683c5b] java_lang_Class::signers(oopDesc*)+0x1b
...
Stack: [0x7fc56d05c000,0x7fc56d15d000], sp=0x7fc56d15b740,
free space=1021k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native
code)
V [libjvm.so+0x683c5b] java_lang_Class::signers(oopDesc*)+0x1b
V [libjvm.so+0x611072]
DumperSupport::dump_class_and_array_classes(DumpWriter*, Klass*)+0xb2
V [libjvm.so+0x461d36] ClassLoaderDataGraph::classes_do(void
(*)(Klass*))+0x36
V [libjvm.so+0x6144ce] VM_HeapDumper::doit()+0x23e
V [libjvm.so+0xab5d25] VM_Operation::evaluate()+0x55
V [libjvm.so+0xab40fa] VMThread::evaluate_operation(VM_Operation*)+0xba
V [libjvm.so+0xab447e] VMThread::loop()+0x1ce
V [libjvm.so+0xab48f0] VMThread::run()+0x70
V [libjvm.so+0x911048] java_start(Thread*)+0x108
VM_Operation (0x7fc52d6592a0): HeapDumper, mode: safepoint, requested
by thread 0x7fc41dbc5000
Other Threads:
=0x7fc5a969b800 VMThread [stack:
0x7fc56d05c000,0x7fc56d15d000] [id=43113]
Heap:
garbage-first heap total 16777216K, used 16640829K [0x0003c000,
0x0003c0804000, 0x0007c000)
region size 8192K, 0 young (0K), 0 survivors (0K)
Metaspace used 132422K, capacity 147372K, committed 150824K,
reserved 1179648K
class spaceused 15699K, capacity 20230K, committed 20888K, reserved
1048576K
...
GC Heap History (10 events):
Event: 56981.031 GC heap before
{Heap before GC invocations=922 (full 190):
garbage-first heap total 16777216K, used 16640829K [0x0003c000,
0x0003c0804000, 0x0007c000)
region size 8192K, 0 young (0K), 0 survivors (0K)
Metaspace used 132416K, capacity 147372K, committed 150824K,
reserved 1179648K
class spaceused 15698K, capacity 20230K, committed 20888K, reserved
1048576K
Event: 56981.063 GC heap after
Heap after GC invocations=923 (full 190):
garbage-first heap total 16777216K, used 16640829K [0x0003c000,
0x0003c0804000, 0x0007c000)
region size 8192K, 0 young (0K), 0 survivors (0K)
Metaspace used 132416K, capacity 147372K, committed 150824K,
reserved 1179648K
class spaceused 15698K, capacity 20230K, committed 20888K, reserved
1048576K
}
Event: 56981.108 GC heap before
{Heap before GC invocations=923 (full 190):
garbage-first heap total 16777216K, used 16640829K [0x0003c000,
0x0003c0804000, 0x0007c000)
region size 8192K, 0 young (0K), 0 survivors (0K)
Metaspace used 132420K, capacity 147372K, committed 150824K,
reserved 1179648K
class spaceused 15699K, capacity 20230K, committed 20888K, reserved
1048576K
Internal exceptions (10 events):
Event: 56980.917 Thread 0x7fc55bb3f000 Exception a
'java/lang/NoSuchMethodError': clinit (0x0007bfa969b0) thrown at
[/HUDSON/workspace/8-2-build-linux-amd64/jdk8u51/3951/hotspot/src/share/vm/prims/jni.cpp,
line 1598]
Event: 56980.917 Thread 0x7fc51c8af000 Exception a
'java/lang/NoSuchMethodError': clinit (0x0007bfa39b38) thrown at
[/HUDSON/workspace/8-2-build-linux-amd64/jdk8u51/3951/hotspot/src/share/vm/prims/jni.cpp,
line 1598]
...
OS:LSB_VERSION=core-2.0-noarch:core-3.2-noarch:core-4.0-noarch:core-2.0-x86_64:core-3.2-x86_64:core-4.0-x86_64
uname:Linux 3.0.101-0.35-default #1 SMP Wed Jul 9 11:43:04 UTC 2014
(c36987d) x86_64
libc:glibc 2.11.3 NPTL 2.11.3
rlimit: STACK 8192k, CORE infinity, NPROC 16384, NOFILE 65535, AS 122580320k
load average:2.57 2.05 2.90
...
vm_info: Java HotSpot(TM) 64-Bit Server VM (25.51-b03) for linux-amd64 JRE
(1.8.0_51-b16), built on Jun 8 2015 19:28:07 by java_re with gcc 4.3.0
20080428 (Red Hat 4.3.0-8)
Re: RFR JDK-8039390: Unexpected behaviour of String.format with null arguments
Hi Sherman, The spec clarifications and the new test look fine. Thanks, Roger On 8/7/15 1:24 PM, Xueming Shen wrote: Hi, Please help review fix for issue: https://bugs.openjdk.java.net/browse/JDK-8039390 webrev: http://cr.openjdk.java.net/~sherman/8039390 The j.u.Formatter implementation outputs null/Null for all conversions if the argument is null (except 'b'/'B', in which the result is false/FALSE). However the API doc only explicitly specifies this behavior for 'b', 'h' and 's'. With the Unless otherwise specified, passing a null argument to any method or constructor in this class will cause a NullPointerException to be thrown at the bottom of the spec, it is confusing which one should be the expected behavior. The proposed change here is to add explicit wording to cover all conversions. (Will go through CCC is approved here). Thanks, Sherman
Re: RFR (M/L): 8131168: Refactor ProcessHandleImpl_*.c and add implememtation for AIX
Hi Volker,
Thanks for checking into the details of the OS X sysctl. I'm fine with
the current implementation.
The rest of the updates and the additional tests look fine also.
But I need to check on the CCC status.
Thanks, Roger
On 8/10/15 10:13 AM, Volker Simonis wrote:
On Fri, Aug 7, 2015 at 8:54 PM, Roger Riggs roger.ri...@oracle.com wrote:
Hi Volker,
1) ProcessHandle.java:243
For the definition of the new commandLine method I would:
- Use @link instead of @code for references to commands() and arguments()
for easy navigation
- @implNote[1] should I think be changed to @apiNote:
the text describes not the JDK implementation but is information about the
information returned and is use to the application developer, not the JDK
implementation.
- The specific references to Linux implementation command line length
parameters seems out of place
and should be omitted.
/**
* Returns the command line of the process.
* p
* If {@link #command command()} and {@link #arguments arguments()}
return non-null
* optionals, this is simply a convenience method which concatenates
* the values of the two functions separated by spaces. Otherwise it
will return a
* best-effort, platform dependent representation of the command
line.
*
* @apiNote Note that the returned executable pathname and the
* arguments may be truncated on some platforms due to
system
* limitations.
* p
* The executable pathname may contain only the
* name of the executable without the full path
information.
* It is undecideable whether white space separates
different
* arguments or is part of a single argument.
*
* @return an {@code OptionalString} of the command line
* of the process
*/
public OptionalString commandLine();
ProcessHandle.java:252: in arguments() method - @apiNote is a better fit for
the note
ProcessHandleImpl_macosx.c:276: - indentation +4
Thanks for the correction. I've taken your wording you suggested.
2) ProcessHandleImpl_macosx.c:192:
if (errno != EINVAL) ... There was previously this test, I'm concerned
that if the pid is invalid,
it will now throw a RuntimeException instead of returning -1.
I recall a discussion from May that recommended testing for EINVAL.
The sysctl in getCmdlineAndUserInfo also does not throw if errno !=
EINVAL, so the usage
is not consistent (probably my coding) but needs investigation.
Not sure about this one and couldn't find any previous discussion
about the topic.
But, according to the sysctl man-page, EINVAL is only returned if:
- The name array is less than two or greater than CTL_MAXNAME.
- A non-null newp is given and its specified length in newlen is too
large or too small.
The first case can not happen because we always statically allocate
arrays of the correct size.
The second case can not happen as well, because we always have 'newp == NULL'.
So according to this information I don't see any reason why we should
check for EINVAL. I think the right solution is to check for 'oldlenp
0' which we already do. By the way, this is also the check applied
by the psutils (see the implementation of 'get_kinfo_proc()' in [1]).
So I wnated to also removed the last check for EINVAL in
getCmdlineAndUserInfo(). But for some reason, that seems to be really
necessary. Without it, we will get a RuntinmeException if we call
sysinfo for pid==0 for example. Further research showed that the
kernel seems to really return EINVAL for KERN_PROCARGS2 (see function
sysctl_procargsx() in [2]). But KERN_PROCARGS2 isn't specified as a
supported constant in the sysctl man-page, so the information there is
still valid :)
On the other hand, I found that the psutils alo handles EINVAL only
for KERN_PROCARGS2 (see get_arg_list() in [1]).
So to cut a long story short, I think the current implementation is
safe as it is now!
[1] http://psutil.googlecode.com/svn/trunk/psutil/arch/osx/process_info.c
[2]
http://www.opensource.apple.com/source/xnu/xnu-1699.24.23/bsd/kern/kern_sysctl.c
3) ProcessHandleImpl_solaris.c can do without the includes:
#include jni_util.h
#include java_lang_ProcessHandleImpl.h
#include java_lang_ProcessHandleImpl_Info.h
#include stdio.h
4) Ditto ProcessHandleImpl_aix.c
Thanks, fixed.
5) ProcessHandleImpl_unix.c: 618: typo fuctions - functions
Fixed.
6) ProcessHandleImpl_unix.c:463: Would it be worthwhile to put
sysconf(_SC_GETPW_R_SIZE_MAX) in a static?
Good point! While doing this I realized that 'clock_ticks_per_second'
is only used on Linux. So I moved the declaration of
'clock_ticks_per_second' from ProcessHandleImpl_unix.hpp to
ProcessHandleImpl_linux.cpp and its initialization to os_initNative()
in the same file.
I also
Re: RFR [9] 8133188: docs: replace tt tags (obsolete in html5) for java.util
Thanks! Looks good to me.
On Tue, Aug 11, 2015 at 4:33 AM, Alexander Stepanov
alexander.v.stepa...@oracle.com wrote:
Hello Martin,
the changes were reverted for the following files:
java.base/share/classes/java/util/concurrent/CopyOnWriteArrayList.java
java.base/share/classes/java/util/concurrent/CopyOnWriteArraySet.java
java.base/share/classes/java/util/AbstractQueue.java
java.base/share/classes/java/util/Deque.java
java.base/share/classes/java/util/NavigableMap.java
java.base/share/classes/java/util/NavigableSet.java
webrev updated:
http://cr.openjdk.java.net/~avstepan/8133188/webrev.02
specdiff report (please update the web page):
http://cr.openjdk.java.net/~avstepan/8133188/specdiff.util/overview-summary.html
(the changes in concurrent are seemingly due to inherited documentation).
Regards,
Alexander
On 8/10/2015 5:28 PM, Martin Buchholz wrote:
Hi Alexander,
I've been hoping that someone does cleanups of the jdk as you are doing
now.
Some of these source files are part of jsr166 and are maintained
separately, in jsr166 CVS.
http://g.oswego.edu/dl/concurrency-interest/
And that has already been tt/code-cleaned (although it has been done
slightly differently) and will be integrated into jdk9 in an upcoming
merge. So please leave out:
concurrent/
*Queue.java
*Deque.java
Navigable*.java
On Mon, Aug 10, 2015 at 6:39 AM, Alexander Stepanov
alexander.v.stepa...@oracle.com wrote:
Hello,
Could you please review the following fix:
http://cr.openjdk.java.net/~avstepan/8133188/webrev.01
for
https://bugs.openjdk.java.net/browse/JDK-8133188
Just a next portion of tt tags removed.
specdiff report:
http://cr.openjdk.java.net/~avstepan/8133188/specdiff.util/overview-summary.html
- some constructs like
tt(o==nullnbsp;?nbsp;e==nullnbsp;:nbsp;o.equals(e))/tt
were replaced with {@code Objects.equals(o, e)}
%7B@codeObjects.equals(o,e)%7D
(e.g.,
http://cr.openjdk.java.net/~avstepan/8133188/webrev.01/src/java.base/share/classes/java/util/LinkedList.java.udiff.html);
- please see JDK-8133188 comments.
Thanks,
Alexander
Re: RFR (M/L): 8131168: Refactor ProcessHandleImpl_*.c and add implememtation for AIX
On Tue, Aug 11, 2015 at 5:08 PM, Roger Riggs roger.ri...@oracle.com wrote:
Hi Volker,
Thanks for checking into the details of the OS X sysctl. I'm fine with the
current implementation.
The rest of the updates and the additional tests look fine also.
Phew! I was already afraid I would have to switch to double-digit
versions for my webrevs :)
But I need to check on the CCC status.
OK, please let me know once it is ready/approved.
Regards,
Volker
Thanks, Roger
On 8/10/15 10:13 AM, Volker Simonis wrote:
On Fri, Aug 7, 2015 at 8:54 PM, Roger Riggs roger.ri...@oracle.com wrote:
Hi Volker,
1) ProcessHandle.java:243
For the definition of the new commandLine method I would:
- Use @link instead of @code for references to commands() and arguments()
for easy navigation
- @implNote[1] should I think be changed to @apiNote:
the text describes not the JDK implementation but is information about the
information returned and is use to the application developer, not the JDK
implementation.
- The specific references to Linux implementation command line length
parameters seems out of place
and should be omitted.
/**
* Returns the command line of the process.
* p
* If {@link #command command()} and {@link #arguments arguments()}
return non-null
* optionals, this is simply a convenience method which concatenates
* the values of the two functions separated by spaces. Otherwise it
will return a
* best-effort, platform dependent representation of the command
line.
*
* @apiNote Note that the returned executable pathname and the
* arguments may be truncated on some platforms due to
system
* limitations.
* p
* The executable pathname may contain only the
* name of the executable without the full path
information.
* It is undecideable whether white space separates
different
* arguments or is part of a single argument.
*
* @return an {@code OptionalString} of the command line
* of the process
*/
public OptionalString commandLine();
ProcessHandle.java:252: in arguments() method - @apiNote is a better fit for
the note
ProcessHandleImpl_macosx.c:276: - indentation +4
Thanks for the correction. I've taken your wording you suggested.
2) ProcessHandleImpl_macosx.c:192:
if (errno != EINVAL) ... There was previously this test, I'm concerned
that if the pid is invalid,
it will now throw a RuntimeException instead of returning -1.
I recall a discussion from May that recommended testing for EINVAL.
The sysctl in getCmdlineAndUserInfo also does not throw if errno !=
EINVAL, so the usage
is not consistent (probably my coding) but needs investigation.
Not sure about this one and couldn't find any previous discussion
about the topic.
But, according to the sysctl man-page, EINVAL is only returned if:
- The name array is less than two or greater than CTL_MAXNAME.
- A non-null newp is given and its specified length in newlen is too
large or too small.
The first case can not happen because we always statically allocate
arrays of the correct size.
The second case can not happen as well, because we always have 'newp ==
NULL'.
So according to this information I don't see any reason why we should
check for EINVAL. I think the right solution is to check for 'oldlenp
0' which we already do. By the way, this is also the check applied
by the psutils (see the implementation of 'get_kinfo_proc()' in [1]).
So I wnated to also removed the last check for EINVAL in
getCmdlineAndUserInfo(). But for some reason, that seems to be really
necessary. Without it, we will get a RuntinmeException if we call
sysinfo for pid==0 for example. Further research showed that the
kernel seems to really return EINVAL for KERN_PROCARGS2 (see function
sysctl_procargsx() in [2]). But KERN_PROCARGS2 isn't specified as a
supported constant in the sysctl man-page, so the information there is
still valid :)
On the other hand, I found that the psutils alo handles EINVAL only
for KERN_PROCARGS2 (see get_arg_list() in [1]).
So to cut a long story short, I think the current implementation is
safe as it is now!
[1] http://psutil.googlecode.com/svn/trunk/psutil/arch/osx/process_info.c
[2]
http://www.opensource.apple.com/source/xnu/xnu-1699.24.23/bsd/kern/kern_sysctl.c
3) ProcessHandleImpl_solaris.c can do without the includes:
#include jni_util.h
#include java_lang_ProcessHandleImpl.h
#include java_lang_ProcessHandleImpl_Info.h
#include stdio.h
4) Ditto ProcessHandleImpl_aix.c
Thanks, fixed.
5) ProcessHandleImpl_unix.c: 618: typo fuctions - functions
Fixed.
6) ProcessHandleImpl_unix.c:463: Would it be worthwhile to put
sysconf(_SC_GETPW_R_SIZE_MAX) in a
Re: RFR [9] 8133188: docs: replace tt tags (obsolete in html5) for java.util
Thanks!
Regards,
Alexander
On 8/11/2015 7:10 PM, Martin Buchholz wrote:
Thanks! Looks good to me.
On Tue, Aug 11, 2015 at 4:33 AM, Alexander Stepanov
alexander.v.stepa...@oracle.com
mailto:alexander.v.stepa...@oracle.com wrote:
Hello Martin,
the changes were reverted for the following files:
java.base/share/classes/java/util/concurrent/CopyOnWriteArrayList.java
java.base/share/classes/java/util/concurrent/CopyOnWriteArraySet.java
java.base/share/classes/java/util/AbstractQueue.java
java.base/share/classes/java/util/Deque.java
java.base/share/classes/java/util/NavigableMap.java
java.base/share/classes/java/util/NavigableSet.java
webrev updated:
http://cr.openjdk.java.net/~avstepan/8133188/webrev.02
http://cr.openjdk.java.net/%7Eavstepan/8133188/webrev.02
specdiff report (please update the web page):
http://cr.openjdk.java.net/~avstepan/8133188/specdiff.util/overview-summary.html
http://cr.openjdk.java.net/%7Eavstepan/8133188/specdiff.util/overview-summary.html
(the changes in concurrent are seemingly due to inherited
documentation).
Regards,
Alexander
On 8/10/2015 5:28 PM, Martin Buchholz wrote:
Hi Alexander,
I've been hoping that someone does cleanups of the jdk as you are
doing now.
Some of these source files are part of jsr166 and are maintained
separately, in jsr166 CVS.
http://g.oswego.edu/dl/concurrency-interest/
And that has already been tt/code-cleaned (although it has
been done slightly differently) and will be integrated into jdk9
in an upcoming merge. So please leave out:
concurrent/
*Queue.java
*Deque.java
Navigable*.java
On Mon, Aug 10, 2015 at 6:39 AM, Alexander Stepanov
alexander.v.stepa...@oracle.com
mailto:alexander.v.stepa...@oracle.com wrote:
Hello,
Could you please review the following fix:
http://cr.openjdk.java.net/~avstepan/8133188/webrev.01
http://cr.openjdk.java.net/%7Eavstepan/8133188/webrev.01
for
https://bugs.openjdk.java.net/browse/JDK-8133188
Just a next portion of tt tags removed.
specdiff report:
http://cr.openjdk.java.net/~avstepan/8133188/specdiff.util/overview-summary.html
http://cr.openjdk.java.net/%7Eavstepan/8133188/specdiff.util/overview-summary.html
- some constructs like
tt(o==nullnbsp;?nbsp;e==nullnbsp;:nbsp;o.equals(e))/tt
were replaced with {@code Objects.equals(o, e)}
mailto:%7B@codeObjects.equals%28o,e%29%7D
(e.g.,
http://cr.openjdk.java.net/~avstepan/8133188/webrev.01/src/java.base/share/classes/java/util/LinkedList.java.udiff.html
http://cr.openjdk.java.net/%7Eavstepan/8133188/webrev.01/src/java.base/share/classes/java/util/LinkedList.java.udiff.html);
- please see JDK-8133188 comments.
Thanks,
Alexander
Re: RFR (M/L): 8131168: Refactor ProcessHandleImpl_*.c and add implememtation for AIX
Hi Volker,
I looked at the proposed specification of commandLine() after the most recent
round of reviews (which is 8131168.v6 I believe) and it looks fine to me. It
expresses the intent pretty well. Oh, and the name commandLine is fine and it
fits well with the names of the other methods.
Thanks,
s'marks
On 8/11/15 8:52 AM, Volker Simonis wrote:
On Tue, Aug 11, 2015 at 5:08 PM, Roger Riggs roger.ri...@oracle.com wrote:
Hi Volker,
Thanks for checking into the details of the OS X sysctl. I'm fine with the
current implementation.
The rest of the updates and the additional tests look fine also.
Phew! I was already afraid I would have to switch to double-digit
versions for my webrevs :)
But I need to check on the CCC status.
OK, please let me know once it is ready/approved.
Regards,
Volker
Thanks, Roger
On 8/10/15 10:13 AM, Volker Simonis wrote:
On Fri, Aug 7, 2015 at 8:54 PM, Roger Riggs roger.ri...@oracle.com wrote:
Hi Volker,
1) ProcessHandle.java:243
For the definition of the new commandLine method I would:
- Use @link instead of @code for references to commands() and arguments()
for easy navigation
- @implNote[1] should I think be changed to @apiNote:
the text describes not the JDK implementation but is information about the
information returned and is use to the application developer, not the JDK
implementation.
- The specific references to Linux implementation command line length
parameters seems out of place
and should be omitted.
/**
* Returns the command line of the process.
* p
* If {@link #command command()} and {@link #arguments arguments()}
return non-null
* optionals, this is simply a convenience method which concatenates
* the values of the two functions separated by spaces. Otherwise it
will return a
* best-effort, platform dependent representation of the command
line.
*
* @apiNote Note that the returned executable pathname and the
* arguments may be truncated on some platforms due to
system
* limitations.
* p
* The executable pathname may contain only the
* name of the executable without the full path
information.
* It is undecideable whether white space separates
different
* arguments or is part of a single argument.
*
* @return an {@code OptionalString} of the command line
* of the process
*/
public OptionalString commandLine();
ProcessHandle.java:252: in arguments() method - @apiNote is a better fit for
the note
ProcessHandleImpl_macosx.c:276: - indentation +4
Thanks for the correction. I've taken your wording you suggested.
2) ProcessHandleImpl_macosx.c:192:
if (errno != EINVAL) ... There was previously this test, I'm concerned
that if the pid is invalid,
it will now throw a RuntimeException instead of returning -1.
I recall a discussion from May that recommended testing for EINVAL.
The sysctl in getCmdlineAndUserInfo also does not throw if errno !=
EINVAL, so the usage
is not consistent (probably my coding) but needs investigation.
Not sure about this one and couldn't find any previous discussion
about the topic.
But, according to the sysctl man-page, EINVAL is only returned if:
- The name array is less than two or greater than CTL_MAXNAME.
- A non-null newp is given and its specified length in newlen is too
large or too small.
The first case can not happen because we always statically allocate
arrays of the correct size.
The second case can not happen as well, because we always have 'newp ==
NULL'.
So according to this information I don't see any reason why we should
check for EINVAL. I think the right solution is to check for 'oldlenp
0' which we already do. By the way, this is also the check applied
by the psutils (see the implementation of 'get_kinfo_proc()' in [1]).
So I wnated to also removed the last check for EINVAL in
getCmdlineAndUserInfo(). But for some reason, that seems to be really
necessary. Without it, we will get a RuntinmeException if we call
sysinfo for pid==0 for example. Further research showed that the
kernel seems to really return EINVAL for KERN_PROCARGS2 (see function
sysctl_procargsx() in [2]). But KERN_PROCARGS2 isn't specified as a
supported constant in the sysctl man-page, so the information there is
still valid :)
On the other hand, I found that the psutils alo handles EINVAL only
for KERN_PROCARGS2 (see get_arg_list() in [1]).
So to cut a long story short, I think the current implementation is
safe as it is now!
[1] http://psutil.googlecode.com/svn/trunk/psutil/arch/osx/process_info.c
[2]
http://www.opensource.apple.com/source/xnu/xnu-1699.24.23/bsd/kern/kern_sysctl.c
3) ProcessHandleImpl_solaris.c can do without the includes:
#include jni_util.h
#include
Re: RFR (M/L): 8131168: Refactor ProcessHandleImpl_*.c and add implememtation for AIX
.. and of course right after I sent my previous message, I ran across something
worth noting.
The proposed spec for commandLine() says,
* If {@link #command command()} and {@link #arguments arguments()} return
non-null
* optionals,
The preferred term is non-empty instead of non-null. This is kind of nitpicky
but in fact command() and arguments() should NEVER return an actual null; they
should always return an Optional that is either empty or that has a value. So I
think this is important to change lest someone be misled into writing
if (info.command() == null info.arguments() == null) ...
Thanks,
s'marks
On 8/11/15 3:07 PM, Stuart Marks wrote:
Hi Volker,
I looked at the proposed specification of commandLine() after the most recent
round of reviews (which is 8131168.v6 I believe) and it looks fine to me. It
expresses the intent pretty well. Oh, and the name commandLine is fine and it
fits well with the names of the other methods.
Thanks,
s'marks
On 8/11/15 8:52 AM, Volker Simonis wrote:
On Tue, Aug 11, 2015 at 5:08 PM, Roger Riggs roger.ri...@oracle.com wrote:
Hi Volker,
Thanks for checking into the details of the OS X sysctl. I'm fine with the
current implementation.
The rest of the updates and the additional tests look fine also.
Phew! I was already afraid I would have to switch to double-digit
versions for my webrevs :)
But I need to check on the CCC status.
OK, please let me know once it is ready/approved.
Regards,
Volker
Thanks, Roger
On 8/10/15 10:13 AM, Volker Simonis wrote:
On Fri, Aug 7, 2015 at 8:54 PM, Roger Riggs roger.ri...@oracle.com wrote:
Hi Volker,
1) ProcessHandle.java:243
For the definition of the new commandLine method I would:
- Use @link instead of @code for references to commands() and arguments()
for easy navigation
- @implNote[1] should I think be changed to @apiNote:
the text describes not the JDK implementation but is information about the
information returned and is use to the application developer, not the JDK
implementation.
- The specific references to Linux implementation command line length
parameters seems out of place
and should be omitted.
/**
* Returns the command line of the process.
* p
* If {@link #command command()} and {@link #arguments arguments()}
return non-null
* optionals, this is simply a convenience method which concatenates
* the values of the two functions separated by spaces. Otherwise it
will return a
* best-effort, platform dependent representation of the command
line.
*
* @apiNote Note that the returned executable pathname and the
* arguments may be truncated on some platforms due to
system
* limitations.
* p
* The executable pathname may contain only the
* name of the executable without the full path
information.
* It is undecideable whether white space separates
different
* arguments or is part of a single argument.
*
* @return an {@code OptionalString} of the command line
* of the process
*/
public OptionalString commandLine();
ProcessHandle.java:252: in arguments() method - @apiNote is a better fit for
the note
ProcessHandleImpl_macosx.c:276: - indentation +4
Thanks for the correction. I've taken your wording you suggested.
2) ProcessHandleImpl_macosx.c:192:
if (errno != EINVAL) ... There was previously this test, I'm concerned
that if the pid is invalid,
it will now throw a RuntimeException instead of returning -1.
I recall a discussion from May that recommended testing for EINVAL.
The sysctl in getCmdlineAndUserInfo also does not throw if errno !=
EINVAL, so the usage
is not consistent (probably my coding) but needs investigation.
Not sure about this one and couldn't find any previous discussion
about the topic.
But, according to the sysctl man-page, EINVAL is only returned if:
- The name array is less than two or greater than CTL_MAXNAME.
- A non-null newp is given and its specified length in newlen is too
large or too small.
The first case can not happen because we always statically allocate
arrays of the correct size.
The second case can not happen as well, because we always have 'newp ==
NULL'.
So according to this information I don't see any reason why we should
check for EINVAL. I think the right solution is to check for 'oldlenp
0' which we already do. By the way, this is also the check applied
by the psutils (see the implementation of 'get_kinfo_proc()' in [1]).
So I wnated to also removed the last check for EINVAL in
getCmdlineAndUserInfo(). But for some reason, that seems to be really
necessary. Without it, we will get a RuntinmeException if we call
sysinfo for pid==0 for example. Further research showed that the
kernel seems to really
