Re: FutureTask.cancel(true) should run thread.interrupt within doPrivileged
On 3/10/2013 2:11 PM, Martin Buchholz wrote: I was responding to Peter Levart's suggestion of checking for the presence of a security manager before calling doPrivileged. Which is not an important question now, given that the primary question is whether we should allow future.cancel() to interrupt within a doPrivileged. Ah I see. Alternatively, is there a reasonable way for a security manager to enable such usages without enabling arbitrary modifyThread? I'm not quite sure what you are asking. Thread permissions are notoriously coarse-grained. I thought that was a big mistake when the security architecture was updated in 1.2 (?) but here we are a decade (or more) later and it seems noone really complained. So be it. Hypothetically we could define finer-grained permissions to differ interrupt from nasty things like stop/suspend. But in practice unless we change how we assign that permission then you would still require it and wouldn't have it unless using a custom security policy - which would allow you to deal with the modifyThread permission too. David - On Wed, Oct 2, 2013 at 9:03 PM, David Holmes wrote: On 3/10/2013 1:55 PM, Martin Buchholz wrote: On Wed, Oct 2, 2013 at 7:13 PM, David Holmes ** wrote: On 3/10/2013 2:54 AM, Martin Buchholz wrote: On Wed, Oct 2, 2013 at 9:49 AM, Peter Levart wrote: Hi Martin, If you want to optimize for without-security-manager case I want to optimize for the case that Thread.interrupt does not throw SecurityException How is your proposal optimizing that case ??? Instead of doing extra work to avoid a SecurityException, I am assuming a SecurityException is rare, and risk having to throw it twice. Sorry I'm missing something - what extra work are you avoiding and where? The original code just did t.interrupt() now you've added try/catch with a second privileged interrupt attempt. I don't see anything being avoided. Are you referring to caller code that catches the SecurityException itself and somehow retries? David
Re: FutureTask.cancel(true) should run thread.interrupt within doPrivileged
On 3/10/2013 1:55 PM, Martin Buchholz wrote: On Wed, Oct 2, 2013 at 7:13 PM, David Holmes wrote: On 3/10/2013 2:54 AM, Martin Buchholz wrote: On Wed, Oct 2, 2013 at 9:49 AM, Peter Levart wrote: Hi Martin, If you want to optimize for without-security-manager case I want to optimize for the case that Thread.interrupt does not throw SecurityException How is your proposal optimizing that case ??? Instead of doing extra work to avoid a SecurityException, I am assuming a SecurityException is rare, and risk having to throw it twice. Sorry I'm missing something - what extra work are you avoiding and where? The original code just did t.interrupt() now you've added try/catch with a second privileged interrupt attempt. I don't see anything being avoided. Are you referring to caller code that catches the SecurityException itself and somehow retries? David
Re: FutureTask.cancel(true) should run thread.interrupt within doPrivileged
On 3/10/2013 2:54 AM, Martin Buchholz wrote: On Wed, Oct 2, 2013 at 9:49 AM, Peter Levart wrote: Hi Martin, If you want to optimize for without-security-manager case I want to optimize for the case that Thread.interrupt does not throw SecurityException How is your proposal optimizing that case ??? David
Re: [concurrency-interest] FutureTask.cancel(true) should run thread.interrupt within doPrivileged
On 3/10/2013 3:02 AM, Doug Lea wrote: On 10/02/2013 12:29 PM, Martin Buchholz wrote: FutureTask.cancel(true) invokes thread.interrupt on the thread (if any) currently running the task. This should succeed even if modifyThread permission is denied by the security manager. We haven't interpreted "should" in this way in the past here or in related contexts, but I don't see are reason not to, pending any objections by security folks. We have been tightening the conditions under which interrupts can be issued, so relaxing them in this way is simply not acceptable. David - -Doug Here's a proposed fix for jdk8+: --- src/main/java/util/concurrent/FutureTask.java15 May 2013 02:39:59 -1.103 +++ src/main/java/util/concurrent/FutureTask.java2 Oct 2013 16:25:23 - @@ -132,6 +132,12 @@ return state != NEW; } +private static void privilegedInterrupt(Thread t) { +java.security.PrivilegedAction doInterrupt = +() -> { t.interrupt(); return null; }; +java.security.AccessController.doPrivileged(doInterrupt); +} + public boolean cancel(boolean mayInterruptIfRunning) { if (!(state == NEW && UNSAFE.compareAndSwapInt(this, stateOffset, NEW, @@ -142,7 +148,11 @@ try { Thread t = runner; if (t != null) -t.interrupt(); +try { +t.interrupt(); +} catch (SecurityException e) { +privilegedInterrupt(t); +} } finally { // final state UNSAFE.putOrderedInt(this, stateOffset, INTERRUPTED); } ___ Concurrency-interest mailing list concurrency-inter...@cs.oswego.edu http://cs.oswego.edu/mailman/listinfo/concurrency-interest
hg: jdk8/tl/jdk: 8020981: Update methods of java.lang.reflect.Parameter to throw correct exceptions
Changeset: cb8946eda85b Author:emc Date: 2013-10-02 19:13 -0400 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/cb8946eda85b 8020981: Update methods of java.lang.reflect.Parameter to throw correct exceptions Summary: Fix behavior of parameter reflection API for malformed class files. Reviewed-by: darcy ! src/share/classes/java/lang/reflect/Executable.java + src/share/classes/java/lang/reflect/MalformedParametersException.java ! src/share/classes/java/lang/reflect/Parameter.java + test/java/lang/reflect/Parameter/BadClassFiles.java
Re: [concurrency-interest] FutureTask.cancel(true) should run thread.interrupt within doPrivileged
On 10/02/2013 12:29 PM, Martin Buchholz wrote: FutureTask.cancel(true) invokes thread.interrupt on the thread (if any) currently running the task. This should succeed even if modifyThread permission is denied by the security manager. We haven't interpreted "should" in this way in the past here or in related contexts, but I don't see are reason not to, pending any objections by security folks. -Doug Here's a proposed fix for jdk8+: --- src/main/java/util/concurrent/FutureTask.java15 May 2013 02:39:59 -1.103 +++ src/main/java/util/concurrent/FutureTask.java2 Oct 2013 16:25:23 - @@ -132,6 +132,12 @@ return state != NEW; } +private static void privilegedInterrupt(Thread t) { +java.security.PrivilegedAction doInterrupt = +() -> { t.interrupt(); return null; }; +java.security.AccessController.doPrivileged(doInterrupt); +} + public boolean cancel(boolean mayInterruptIfRunning) { if (!(state == NEW && UNSAFE.compareAndSwapInt(this, stateOffset, NEW, @@ -142,7 +148,11 @@ try { Thread t = runner; if (t != null) -t.interrupt(); +try { +t.interrupt(); +} catch (SecurityException e) { +privilegedInterrupt(t); +} } finally { // final state UNSAFE.putOrderedInt(this, stateOffset, INTERRUPTED); } ___ Concurrency-interest mailing list concurrency-inter...@cs.oswego.edu http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Re: FutureTask.cancel(true) should run thread.interrupt within doPrivileged
Hi Martin, If you want to optimize for without-security-manager case, then it would be better this way: private static void privilegedInterrupt(Thread t) { if (System.getSecurityManager() == null) { t.interrupt(); } else { PrivilegedAction doInterrupt = () -> { t.interrupt(); return null; }; AccessController.doPrivileged(doInterrupt); } } ...and no security exception catching. This way you don't trigger double security checks for case with-security-manager and not enough permission... Regards, Peter On 10/02/2013 06:29 PM, Martin Buchholz wrote: FutureTask.cancel(true) invokes thread.interrupt on the thread (if any) currently running the task. This should succeed even if modifyThread permission is denied by the security manager. Here's a proposed fix for jdk8+: --- src/main/java/util/concurrent/FutureTask.java 15 May 2013 02:39:59 - 1.103 +++ src/main/java/util/concurrent/FutureTask.java 2 Oct 2013 16:25:23 - @@ -132,6 +132,12 @@ return state != NEW; } +private static void privilegedInterrupt(Thread t) { +java.security.PrivilegedAction doInterrupt = +() -> { t.interrupt(); return null; }; +java.security.AccessController.doPrivileged(doInterrupt); +} + public boolean cancel(boolean mayInterruptIfRunning) { if (!(state == NEW && UNSAFE.compareAndSwapInt(this, stateOffset, NEW, @@ -142,7 +148,11 @@ try { Thread t = runner; if (t != null) -t.interrupt(); +try { +t.interrupt(); +} catch (SecurityException e) { +privilegedInterrupt(t); +} } finally { // final state UNSAFE.putOrderedInt(this, stateOffset, INTERRUPTED); }
hg: jdk8/tl/jdk: 8025694: Rename getStrongSecureRandom based on feedback; ...
Changeset: a6ac824b463d Author:wetmore Date: 2013-10-02 09:38 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a6ac824b463d 8025694: Rename getStrongSecureRandom based on feedback 8014838: getStrongSecureRandom() should require at least one implementation Reviewed-by: mullan, darcy ! src/share/classes/java/security/SecureRandom.java ! src/share/lib/security/java.security-windows ! test/sun/security/provider/SecureRandom/StrongSecureRandom.java
hg: jdk8/tl/langtools: 8023679: Improve error message for '_' used as a lambda parameter name
Changeset: 1e6088da1740 Author:vromero Date: 2013-10-02 17:04 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/1e6088da1740 8023679: Improve error message for '_' used as a lambda parameter name Reviewed-by: jjg, dlsmith ! src/share/classes/com/sun/tools/javac/parser/JavacParser.java ! src/share/classes/com/sun/tools/javac/resources/compiler.properties + test/tools/javac/diags/examples/UnderscoreInLambdaExpression.java
hg: jdk8/tl/jdk: 6696975: JTop plugin fails if connected readonly to target JVM
Changeset: 3bb89c509d59 Author:egahlin Date: 2013-10-01 17:48 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3bb89c509d59 6696975: JTop plugin fails if connected readonly to target JVM Reviewed-by: mchung, jbachorik, sla, sjiang ! src/share/demo/management/JTop/JTop.java
Re: RFR: JDK-8014838: getStrongSecureRandom() should require at least one implementation
Looks good to me. --Sean On 10/02/2013 12:28 AM, Bradford Wetmore wrote: During the internal CCC review, the CCC lead suggested a change to the method name to more closely follow the existing getInstance() methods, so I've changed the name from: java.security.SecureRandom.getStrongSecureRandom() to java.security.SecureRandom.getInstanceStrong() This particular change will be tracked via: 8025694: Rename getStrongSecureRandom based on feedback This will collate better (i.e. javadoc/NetBeans) than getStrongInstance(). It was also pointed out that the current strong Window MSCAPI-based implementation may not be available under the various profiles, so I've added a SHA1PRNG:SUN fallback in case the MSCAPI is not available. https://bugs.openjdk.java.net/browse/JDK-8014838 https://bugs.openjdk.java.net/browse/JDK-8025694 http://cr.openjdk.java.net/~wetmore/8025694/webrev.00 (contains both 8025694/8014838) Even though we are not currently building profiles on Windows currently, we could in the future, so the profiles team agrees this is a good compromise. Brad On 9/26/2013 4:04 PM, Bradford Wetmore wrote: This minor suggestion came up in May from our JCK team: https://bugs.openjdk.java.net/browse/JDK-8014838 http://cr.openjdk.java.net/~wetmore/8014838/webrev.00/ JCK suggested all implementations should have at least one strong random implementation. I've also changed the error case to throw NoSuchAlgorithmException instead of returning null. CCC review is also underway concurrently, but I'm not expecting any issues. Brad
hg: jdk8/tl/jdk: 8025535: Unsafe typecast in java.util.stream.SortedOps
Changeset: e1b04fd49204 Author:psandoz Date: 2013-10-01 18:20 +0200 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e1b04fd49204 8025535: Unsafe typecast in java.util.stream.SortedOps Reviewed-by: mduigou, chegar ! src/share/classes/java/util/stream/SortedOps.java ! test/java/util/stream/test/org/openjdk/tests/java/util/stream/SortedOpTest.java
hg: jdk8/tl/jdk: 8022666: java.util.Calendar.set(int, int, int, int, int, int) documentation typo
Changeset: 82e3150778e0 Author:okutsu Date: 2013-10-02 17:57 +0900 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/82e3150778e0 8022666: java.util.Calendar.set(int,int,int,int,int,int) documentation typo Reviewed-by: peytoia ! src/share/classes/java/util/Calendar.java
hg: jdk8/tl/jdk: 8024952: ClassCastException in PlainSocketImpl.accept() when using custom socketImpl
Changeset: 368172cb6dc5 Author:coffeys Date: 2013-10-02 09:21 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/368172cb6dc5 8024952: ClassCastException in PlainSocketImpl.accept() when using custom socketImpl Reviewed-by: chegar ! src/windows/classes/java/net/PlainSocketImpl.java + test/java/net/PlainSocketImpl/CustomSocketImplFactory.java
hg: jdk8/tl/jdk: 6902861: (cal) GregorianCalendar roll WEEK_OF_YEAR is broken for January 1 2010
Changeset: 914c29c10bce Author:okutsu Date: 2013-10-02 15:31 +0900 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/914c29c10bce 6902861: (cal) GregorianCalendar roll WEEK_OF_YEAR is broken for January 1 2010 Reviewed-by: peytoia ! src/share/classes/java/util/GregorianCalendar.java + test/java/util/Calendar/Bug6902861.java