Re: RFR 9: 8165641 : Deprecate Object.finalize

On 16/03/2017 3:06 PM, Timo Kinnunen wrote: Hi, Yes, indeed it does. I find it intriguing that finalization as currently specified is so unsuited for managing the one native resource that from a Java programmers point-of-view the JVM is managing perfectly. This resource being of course not

Re: RRF(XS)(10): 8176797: [TESTBUG] tools/launcher/Settings.java -Xss size is too small

Hi Chris, Looks good to me. Thanks, David On 16/03/2017 2:59 PM, Chris Plummer wrote: Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8176797 http://cr.openjdk.java.net/~cjplummer/8176797/webrev.00/webrev.jdk After fixing 8175342 (see the other RFR I just

Re: RFR(L) : 8176176 : fix @modules in jdk_svc tests

Igor, It was my guess but it is nice to have it explicitly explained. Thanks! Serguei On 3/15/17 22:07, Igor Ignatyev wrote: Hi Serguei, 1s of all, thank you for your review. since these tests have dependencies on more modules than corresponding TEST.properties file declares, we have to

Re: RFR(L) : 8176176 : fix @modules in jdk_svc tests

Hi Serguei, 1s of all, thank you for your review. since these tests have dependencies on more modules than corresponding TEST.properties file declares, we have to specify @modules tag in them, and because @modules in a test overrides modules from TEST.properties, jdk.jdi module should be

RE: RFR 9: 8165641 : Deprecate Object.finalize

Hi, Yes, indeed it does. I find it intriguing that finalization as currently specified is so unsuited for managing the one native resource that from a Java programmers point-of-view the JVM is managing perfectly. This resource being of course not memory, but thread handles. I think by using

RRF(XS)(10): 8176797: [TESTBUG] tools/launcher/Settings.java -Xss size is too small

Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8176797 http://cr.openjdk.java.net/~cjplummer/8176797/webrev.00/webrev.jdk After fixing 8175342 (see the other RFR I just posted), this test started to fail instead of assert. The problem is 256000 is too small of a

RFR(XS)(10): 8175342: assert(InstanceKlass::cast(k)->is_initialized()) failed: need to increase java_thread_min_stack_allowed calculation

Hello, Please review the following: https://bugs.openjdk.java.net/browse/JDK-8175342 http://cr.openjdk.java.net/~cjplummer/8175342/webrev.00/webrev.jdk The assert is somewhat misleading, although it did properly detect a "too small" stack issue. The test was run with -Xss256k on a system with

Re: RFR(L) : 8176176 : fix @modules in jdk_svc tests

Hi Igor, This looks good to me. A couple of questions below. http://cr.openjdk.java.net/~iignatyev//8176176/webrev.01/test/com/sun/jdi/InvokeHangTest.java.udiff.html - * @modules jdk.jdi * @library /test/lib + * @modules java.management + * jdk.jdiShould the jdk.jdi be removed from

Re: RFR of JDK-8176563: @since value errors in apis of java.base/java.logging module

> On Mar 15, 2017, at 7:40 PM, Hamlin Li wrote: > > Hi, > > Thank you for reviewing, as StackFramePermission was just removed in > JDK-8176815, so I remove the @since change for StackFramePermission, and the > rest code change is pushed. > Thanks. The updated change

Re: RFR of JDK-8176563: @since value errors in apis of java.base/java.logging module

Hi, Thank you for reviewing, as StackFramePermission was just removed in JDK-8176815, so I remove the @since change for StackFramePermission, and the rest code change is pushed. Thank you -Hamlin On 2017/3/15 19:55, Alan Bateman wrote: On 15/03/2017 11:35, Daniel Fuchs wrote: Hi

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

Looks like I have a chance to tweak the comments for the JAF changes. Any final comments before I apply these changes to all 4 copies of this code? $ diff -u MailcapCommandMap.java.orig MailcapCommandMap.java --- MailcapCommandMap.java.orig 2017-03-15 15:10:43.731176917 -0700 +++

Re: RFR: 8176834: jdk/nio/zipfs/MultiReleaseJarTest.java test fails after JDK-8176709

On 2017-03-15 23:05, Mandy Chung wrote: On Mar 15, 2017, at 3:01 PM, Claes Redestad wrote: Hi, it appears the test we wrote to test the Multi-Release checking in JarFile, based on the jar manifest specification[1] and now adapted for JDK-8176709 is too strict

Re: RFR: 8176834: jdk/nio/zipfs/MultiReleaseJarTest.java test fails after JDK-8176709

> On Mar 15, 2017, at 3:01 PM, Claes Redestad wrote: > > Hi, > > it appears the test we wrote to test the Multi-Release checking in > JarFile, based on the jar manifest specification[1] and now adapted > for JDK-8176709 is too strict when actually using >

RFR: 8176834: jdk/nio/zipfs/MultiReleaseJarTest.java test fails after JDK-8176709

Hi, it appears the test we wrote to test the Multi-Release checking in JarFile, based on the jar manifest specification[1] and now adapted for JDK-8176709 is too strict when actually using java.util.jar.Manifest, which means the test I added for JDK-8176709 fails on the version that I pushed

Re: Review Request: JDK-8176815: Remove StackFramePermission and use RuntimePermission for stack walking

> On Mar 15, 2017, at 1:30 PM, Alan Bateman wrote: > > On 15/03/2017 19:42, Mandy Chung wrote: > >> StackWalker::getInstance is currently specified to check for >> StackFramePermission("retainClassReference“) due to an early review feedback >> . Given it has only

Re: Review Request: JDK-8176815: Remove StackFramePermission and use RuntimePermission for stack walking

On 15/03/2017 19:42, Mandy Chung wrote: StackWalker::getInstance is currently specified to check for StackFramePermission("retainClassReference“) due to an early review feedback . Given it has only one target, it’s overkill to define a specific permission type for stack walking use. This

Re: Review Request: JDK-8176815: Remove StackFramePermission and use RuntimePermission for stack walking

Hi, Mandy I agree that the new permission type is overkill. The changes look good to me. -Brent On 3/15/17 12:42 PM, Mandy Chung wrote: StackWalker::getInstance is currently specified to check for StackFramePermission("retainClassReference“) due to an early review feedback . Given it has

Re: RFR 9: 8165641 : Deprecate Object.finalize

> On Mar 15, 2017, at 11:39 AM, Roger Riggs wrote: > > Hi Mandy, > > I re-checked and @deprecated is not inherited; and if it was it would include > a generic message from the super > so the @deprecated content is needed. Sample of >

Review Request: JDK-8176815: Remove StackFramePermission and use RuntimePermission for stack walking

StackWalker::getInstance is currently specified to check for StackFramePermission("retainClassReference“) due to an early review feedback . Given it has only one target, it’s overkill to define a specific permission type for stack walking use. This patch proposes to replace the StackWalker

Re: RFR 9: 8176272 : (process) ProcessHandle::onExit fails to wait for non-child process

Hi Thomas, Good idea. Though it is unlikely that the pid would be re-used between the checks of isAlive but that will remove any window. Updated webrev: http://cr.openjdk.java.net/~rriggs/webrev-processhandle-onexit-8176272/index.html Thanks, Roger On 3/15/2017 4:19 AM, Thomas Stüfe

Re: RFR 9: 8165641 : Deprecate Object.finalize

Hi Mandy, I re-checked and @deprecated is not inherited; and if it was it would include a generic message from the super so the @deprecated content is needed. Sample of javax.imageio.stream.FileImageInputStream.[1] Only minor cleanup to the webrev:

Re: RFR: 8176709: JarFileSystem::isMultiReleaseJar is incorrect

On 03/15/2017 07:33 PM, Mandy Chung wrote: On Mar 15, 2017, at 11:25 AM, Claes Redestad wrote: On 03/15/2017 05:36 PM, Mandy Chung wrote: I would prefer to separate the performance issue from this fix and keep this fix simple. There may be more to tease out

Re: RFR: 8176709: JarFileSystem::isMultiReleaseJar is incorrect

> On Mar 15, 2017, at 11:25 AM, Claes Redestad > wrote: > > > > On 03/15/2017 05:36 PM, Mandy Chung wrote: >> I would prefer to separate the performance issue from this fix and keep this >> fix simple. There may be more to tease out for performance regression

Re: RFR: 8176709: JarFileSystem::isMultiReleaseJar is incorrect

The import j.u.Objects is no longer needed? The rest looks fine. -Sherman On 3/15/17, 11:25 AM, Claes Redestad wrote: On 03/15/2017 05:36 PM, Mandy Chung wrote: I would prefer to separate the performance issue from this fix and keep this fix simple. There may be more to tease out for

Re: RFR: 8176709: JarFileSystem::isMultiReleaseJar is incorrect

On 03/15/2017 05:36 PM, Mandy Chung wrote: I would prefer to separate the performance issue from this fix and keep this fix simple. There may be more to tease out for performance regression that I defer to Sherman to discuss with you. Ok:

Re: RFR(L) : 8176176 : fix @modules in jdk_svc tests

Shura, Thank you for your review. I have update test/java/lang/management/PlatformLoggingMXBean/TEST.properties[1] and checked that there are no similar things in other TEST.properties files. Still looking for a review from a Reviewer. [1] > ---

Re: RFR(L) : 8176176 : fix @modules in jdk_svc tests

Igor, I have looked through a bunch of tests where @modules is changed - that looks good. One minor thing I noticed is that, in test/java/lang/management/PlatformLoggingMXBean/TEST.properties you are explicitly calling out java.management. You do not have to do that because “modules”

Re: RFR: 8176709: JarFileSystem::isMultiReleaseJar is incorrect

> On Mar 15, 2017, at 4:06 AM, Claes Redestad wrote: > > Hi Mandy, > > On 03/15/2017 12:32 AM, Mandy Chung wrote: >> I agree with the goal to reduce the number of qualified exports, which I >> always like to keep. >> >> Duplicating code like this isn’t ideal

Re: RFR 9: 8165641 : Deprecate Object.finalize

Hi, yes, the thread should use try...finally to cleanup its resources explicitly. The method to cleanup and release resources should *not* be called finalize and should be idempotent. Finalization is needed as a last resort it should call the cleanup method (and nothing else). Not all

Re: RFR of JDK-8176563: @since value errors in apis of java.base/java.logging module

On 15/03/2017 11:35, Daniel Fuchs wrote: Hi Hamlin, The new version looks good to me. Maybe wait for review of the other interested parties ;-) This one looks right to me too. -Alan

Re: RFR of JDK-8176563: @since value errors in apis of java.base/java.logging module

Hi Hamlin, The new version looks good to me. Maybe wait for review of the other interested parties ;-) best regards, -- daniel On 15/03/2017 04:34, Hamlin Li wrote: BTW, I did *remove* the unnecessary @since for ObjectInputFilter.checkInput. :-) Thank you -Hamlin On 2017/3/15 11:34,

Re: RFR: 8176709: JarFileSystem::isMultiReleaseJar is incorrect

Hi Mandy, On 03/15/2017 12:32 AM, Mandy Chung wrote: I agree with the goal to reduce the number of qualified exports, which I always like to keep. Duplicating code like this isn’t ideal although it’s straight-forward. This is a performance optimization. One solution is to keep using the

Re: RFR 9: 8165641 : Deprecate Object.finalize

On 15/03/17 10:21, Timo Kinnunen wrote: > If we are to come up with a good alternative to finalization, having > to compare disparate things seems unavoidable. Comparing threads and > Threads, suppose there’s a subclass of Thread which holds a native > resource that’s not reachable from any other

Re: RFR 9: 8165641 : Deprecate Object.finalize

On 15/03/2017 8:21 PM, Timo Kinnunen wrote: Hi, If we are to come up with a good alternative to finalization, having to compare disparate things seems unavoidable. Comparing threads and Threads, suppose there’s a subclass of Thread which holds a native resource that’s not reachable from any

RE: RFR 9: 8165641 : Deprecate Object.finalize

Hi, If we are to come up with a good alternative to finalization, having to compare disparate things seems unavoidable. Comparing threads and Threads, suppose there’s a subclass of Thread which holds a native resource that’s not reachable from any other Thread and it has a finalize method.

Re: RFR 9: 8165641 : Deprecate Object.finalize

On 14/03/17 20:44, Hans Boehm wrote: > It's not hard to add an API to tell the garbage collector to pretend > that you're holding onto those 18GB of additional memory when it > decides whether to collect, without actually actually holding onto > all of that memory. It may not be hard to add the

Re: RFR 9: 8176272 : (process) ProcessHandle::onExit fails to wait for non-child process

Hi Roger, when using isAlive0, would it may make sense to - on the first invocation - remember the process start time and on subsequent invocations to check this time against the new return value? That way you could check for process identity in the case of recycled process ids. Kind