Re: RFR 9: 8087286 Need a way to handle control-C and possibly some other signals

[Stuart Marks]

On 2/5/16 4:54 PM, David Holmes wrote:

Regardless of whether I agree with this API or not, it does, as Stuart points
out, require a JEP and to go through the normal rigorous process of determining
whether an API is suitable for inclusion in the Java platform.


Note, it was Thomas Stüfe who suggested a JEP for this.

s'marks

Re: We don't need jdk.internal.ref.Cleaner any more

[Alan Bateman]

On 07/02/2016 22:20, Peter Levart wrote:

:

If the decision to remove sun.misc.Cleaner was partly influenced by 
the desire to maintain just 2 instead of 3 Cleaner(s), then my 
proposal to migrate JDK code to the public API might enable Oracle to 
reconsider keeping sun.misc.Cleaner.


The main issue driving this is the design principles that we have listed 
in JEP 200. We don't want a standard module (java.base in this case) 
exporting a non-standard API. This means surgery to jettison the 
sun.misc package from java.base and move it to its own module 
(jdk.unsupported is the proposal in JEP 260). This is painful of course 
but we are at least in good shape for the most critical internal API, 
sun.misc.Unsafe.


For sun.misc.Cleaner then the original proposal was for it to be a 
critical internal API too but it become clear that changing the type of 
internal/private fields in the NIO buffer and channel classes would 
break libraries that have been hacking into those fields. If they are 
changing away then there seems little motive to keep sun.misc.Cleaner so 
Chris moved into into jdk.internal.ref for now. As to whether we even 
need to keep jdk.internal.ref.Cleaner then I think the only remaining 
question was whether the special handling of Cleaner in the Reference 
implementation was worth it or not. It may not be, in which case your 
current proposal to just remove seems right.


-Alan.

Re: RFR JDK-8141491: Unaligned memory access in Bits.c

[David Holmes]

On 6/02/2016 8:21 AM, Mikael Vidstedt wrote:


I fully agree that moving the arguments checking up to Java makes more
sense, and I've prepared new webrevs which do exactly that, including
changes to address the other feedback from David, John and others:


Shouldn't the lowest-level do_conjoint_swap routines at least check 
preconditions with asserts to catch the cases where the calling Java 
code has failed to do the right thing? The other Copy methods seem to do 
this.


David
-


hotspot:
http://cr.openjdk.java.net/~mikael/webrevs/8141491/webrev.04/hotspot/webrev/

jdk:
http://cr.openjdk.java.net/~mikael/webrevs/8141491/webrev.04/jdk/webrev/

Incremental webrevs for your convenience:

hotspot:
http://cr.openjdk.java.net/~mikael/webrevs/8141491/webrev.04.incr/hotspot/webrev/

jdk:
http://cr.openjdk.java.net/~mikael/webrevs/8141491/webrev.04.incr/jdk/webrev/



I have done some benchmarking of this code and for large copies (16MB+)
this outperforms the old Bits.c implementation by *30-100%* depending on
platform and exact element sizes! For smaller copies the additional
checks which are now performed hurt performance on client VMs (80-90% of
old impl), but with the server VMs I see performance on par with, or in
most cases 5-10% better than the old implementation. There's a
potentially statistically significant regression of ~3-4% for
elemSize=2, but for now I'm going to declare success. There's certainly
room for further improvements here, but this should at least do for
addressing the original problem.


I filed https://bugs.openjdk.java.net/browse/JDK-8149159 for moving the
checks for Unsafe.copyMemory to Java, and will work on that next. I also
filed https://bugs.openjdk.java.net/browse/JDK-8149162 to cover the
potential renaming of the Bits methods to have more informative names.
Finally, I filed https://bugs.openjdk.java.net/browse/JDK-8149163 to
look at improving the behavior of Unsafe.addressSize(), after having
spent too much time trying to understand why the performance of the new
U.copySwapMemory Java checks wasn't quite living up to my expectations
(spoiler alert: Unsafe.addressSize() is not intrinsified, so will always
result in a call into the VM/unsafe.cpp).


Finally, I - too - would like to see the copy-swap logic moved into
Java, and as I mentioned I played around with that first before I
decided to do the native implementation to address the immediate
problem. Looking forward to what you find Paul!

Cheers,
Mikael

On 2016-02-05 05:00, Paul Sandoz wrote:

Hi,

Nice use of C++ templates :-)

Overall looks good.

I too would prefer if we could move the argument checking out, perhaps
even to the point of requiring callers do that rather than providing
another method, for example for Buffer i think the arguments are known
to be valid? I think in either case it is important to improve the
documentation on the method stating the constraints on arguments,
atomicity guarantees etc.

I have a hunch that for the particular case of copying-with-swap for
buffers i could get this to work work efficiently using Unsafe (three
separate methods for each unit type of 2, 4 and 8 bytes), since IIUC
the range is bounded to be less than Integer.MAX_VALUE so an int loop
rather than a long loop can be used and therefore safe points checks
will not be placed within the loop.

However, i think what you have done is more generally applicable and
could be made intrinsic. It would be a nice at some future point if it
could be made a pure Java implementation and intrinsified where
appropriate.

—

John, regarding array mismatch there were issues with the efficiency
of the unrolled loops with Unsafe access. (Since the loops were int
bases there were no issues with safe point checks.) Roland recently
fixed that so now code is generated that is competitive with direct
array accesses. We drop into the stub intrinsic and leverage 128bits
or 256bits where supported. Interestingly it seems the unrolled loop
using Unsafe is now slightly faster than the stub using 128bit
registers. I don’t know if that is due to unluckly alignment, and/or
the stub needs to do some manual unrolling. In terms of code-cache
efficiency the intrinsic is better.

Paul.






On 4 Feb 2016, at 06:27, John Rose  wrote:

On Feb 2, 2016, at 11:25 AM, Mikael Vidstedt
 wrote:

Please review this change which introduces a Copy::conjoint_swap and
an Unsafe.copySwapMemory method to call it from Java, along with the
necessary changes to have java.nio.Bits call it instead of the
Bits.c code.

http://cr.openjdk.java.net/~mikael/webrevs/8141491/webrev.03/hotspot/webrev/

http://cr.openjdk.java.net/~mikael/webrevs/8141491/webrev.03/jdk/webrev/


This is very good.

I have some nit-picks:

These days, when we introduce a new intrinsic (@HSIntrCand),
we write the argument checking code separately in a non-intrinsic
bytecode method.  In this case, we don't (yet) have an intrinsic
binding for U.copy*, but we might in the future.  (C intrinsifies
memcpy, which 

RE: We don't need jdk.internal.ref.Cleaner any more

[Uwe Schindler]

Hi Peter,

as discussed before in the other thread about move to jdk.internal: this looks 
fine to Apache Lucene. We have a separate issue to fix this for Java 9: 
https://issues.apache.org/jira/browse/LUCENE-6989

Currently the patch on the Lucene issue tries to cast the jdk.internal.Cleaner 
to Runnable; but with your patch, one would just need to cast to 
java.lang.ref.Cleaner$Cleanable (which is public anyways) and call clean().

The Runnable workaround was done for Lucene, Hadoop, and Netty, but with your 
current patch this extra hack will be obsolete:
http://cr.openjdk.java.net/~chegar/8148117/src/java.base/share/classes/jdk/internal/ref/Cleaner.java.udiff.html

So, I am fine with both solutions as workaround, if we can enforce unmapping - 
until an official and "safe" solution is found.

I was discussing with Andrew Haley and Mark Reinhold on FOSDEM about an 
"official way" to unmap ByteBuffers and there seems to be a really cool idea to 
make MappedByteBuffer/DirectByteBuffer implement Closeable, so unmapping may 
work without the horrible performance degradion on every access by using some 
extra safepoint and changed volatile semantics using a annotation in Hotspot.

Uwe

-
Uwe Schindler
uschind...@apache.org 
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
http://lucene.apache.org/
> -Original Message-
> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
> Behalf Of Peter Levart
> Sent: Sunday, February 07, 2016 11:57 PM
> To: Jeremy Manson 
> Cc: Core-Libs-Dev 
> Subject: Re: We don't need jdk.internal.ref.Cleaner any more
> 
> 
> 
> On 02/07/2016 11:20 PM, Peter Levart wrote:
> >
> >
> > On 02/07/2016 08:01 PM, Jeremy Manson wrote:
> >> Hadoop seems to use sun.misc.Cleaner:
> >>
> >>
> http://grepcode.com/file/repo1.maven.org/maven2/org.apache.hadoop/ha
> doop-common/2.7.1/org/apache/hadoop/crypto/CryptoStreamUtils.java
> >>
> >> So you may want to keep it around transitionally (a la Unsafe).
> >
> > JEP 260 [1] was listing sun.misc.Cleaner as a critical API, but
> > recently, it was decided to "hide" it away into jdk.internal.ref
> > package which will not be exported [2]. The reasoning was this:
> >
> > "sun.misc.Cleaner ( was previously listed as a critical internal API,
> > but on further investigation has been moved to an open issue, for the
> > following reasons: 1) its primary use in the JDK is within NIO direct
> > buffers to release native memory. The base module cannot have a
> > dependency on jdk.unsupported so will need to be updated to use an
> > alternative cleaner, 2) the usage of Cleaner outside the JDK, as
> > determined by corpus analysis, has largely been observed to hack into
> > private fields of the internal NIO direct buffer classes to explicitly
> > release native memory. As stated in 1), the type of the cleaner used
> > by NIO direct buffers will have to change. Given this, and the fact
> > that JDK 9 has a new general purposed cleaner API,
> > java.lang.ref.Cleaner, the value of keep sun.misc.Cleaner is
> > questionable."
> >
> > If the decision to remove sun.misc.Cleaner was partly influenced by
> > the desire to maintain just 2 instead of 3 Cleaner(s), then my
> > proposal to migrate JDK code to the public API might enable Oracle to
> > reconsider keeping sun.misc.Cleaner.
> 
> OTOH, what hadoop is doing is exactly the usage described in the above
> reasoning for removing sun.misc.Cleaner.
> 
> Hadoop use case:
> 
>  public static void freeDB(ByteBuffer buffer) {
>  if (buffer instanceof sun.nio.ch.DirectBuffer) {
>  final sun.misc.Cleaner bufferCleaner =
>  ((sun.nio.ch.DirectBuffer) buffer).cleaner();
>  bufferCleaner.clean();
>  }
>  }
> 
> can be rewritten using reflection to be compatible with either
> sun.misc.Cleaner (on JDK 8 or less) or java.lang.ref.Cleaner$Cleanable
> (on JDK 9 or more):
> 
>  static final Method cleanMethod;
> 
>  static {
>  try {
>  Class cleanerOrCleanable;
>  try {
>  cleanerOrCleanable = Class.forName("sun.misc.Cleaner");
>  } catch (ClassNotFoundException e1) {
>  try {
>  cleanerOrCleanable =
> Class.forName("java.lang.ref.Cleaner$Cleanable");
>  } catch (ClassNotFoundException e2) {
>  e2.addSuppressed(e1);
>  throw e2;
>  }
>  }
>  cleanMethod = cleanerOrCleanable.getDeclaredMethod("clean");
>  } catch (Exception e) {
>  throw new Error(e);
>  }
>  }
> 
>  public static void freeDB_JDK8or9(ByteBuffer buffer) {
>  if (buffer instanceof sun.nio.ch.DirectBuffer) {
>  final Object bufferCleaner =
>  ((sun.nio.ch.DirectBuffer) buffer).cleaner();
> 
>  try {
>  if (bufferCleaner != null) {
>  cleanMethod.invoke(buff

Re: RFR: 8148446: (tz) Support tzdata2016a

[Masayoshi Okutsu]

Looks good to me.

Masayoshi

On 2/4/2016 5:55 PM, Ramanand Patil wrote:

Hi Masayoshi/all,
Please review the updated Webrev at: 
http://cr.openjdk.java.net/~rpatil/8148446/webrev.01/

Regards,
Ramanand.

-Original Message-
From: Masayoshi Okutsu
Sent: Thursday, February 04, 2016 12:17 PM
To: Ramanand Patil; i18n-...@openjdk.java.net
Cc: core-libs-dev@openjdk.java.net
Subject: Re: RFR: 8148446: (tz) Support tzdata2016a

Hi Ramanand,

I noticed that the zones in Yakutsk Time [1] seem to have their own names, such as 
"Khandyga Time" for Asia/Khandyga, and you seem to follow that convention for 
Asia/Chita. That causes some mismatch between the long names and abbreviations.

I dag out some past tzdata fixes to see how that happened. What I found out is that the 2013b fix 
[2] used "Yakutsk Time", but that the 2013c [3] fix used "Khandyga Time". 2013b 
went to JDK 7 updates and earlier ones and 2013c went to 8. JDK 9 inherits the 8 fix.

I prefer to restore the 2013b convention for Asia/Yakutsk, Asia/Chita, and 
Asia/Khandyga to have:

  {"Yakutsk Time", "YAKT",
   "Yakutsk Summer Time", "YAKST",
   "Yakutsk Time", "YAKT"}

Thanks,
Masayoshi

[1] https://en.wikipedia.org/wiki/Yakutsk_Time
[2] http://hg.openjdk.java.net/jdk7u/jdk7u-dev/jdk/rev/b8009df64dc8
[3] http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/ae35fdbab949

On 2/2/2016 8:00 PM, Ramanand Patil wrote:

HI all,

Please review the latest TZDATA integration (tzdata2016a) to JDK9.

Bug  - https://bugs.openjdk.java.net/browse/JDK-8148446

Webrev - http://cr.openjdk.java.net/~rpatil/8148446/webrev.00/

   


All the TimeZone related tests are passed after integration.

   


Regards,

Ramanand.

Re: We don't need jdk.internal.ref.Cleaner any more

[Peter Levart]

On 02/07/2016 11:20 PM, Peter Levart wrote:



On 02/07/2016 08:01 PM, Jeremy Manson wrote:

Hadoop seems to use sun.misc.Cleaner:

http://grepcode.com/file/repo1.maven.org/maven2/org.apache.hadoop/hadoop-common/2.7.1/org/apache/hadoop/crypto/CryptoStreamUtils.java

So you may want to keep it around transitionally (a la Unsafe).


JEP 260 [1] was listing sun.misc.Cleaner as a critical API, but 
recently, it was decided to "hide" it away into jdk.internal.ref 
package which will not be exported [2]. The reasoning was this:


"sun.misc.Cleaner ( was previously listed as a critical internal API, 
but on further investigation has been moved to an open issue, for the 
following reasons: 1) its primary use in the JDK is within NIO direct 
buffers to release native memory. The base module cannot have a 
dependency on jdk.unsupported so will need to be updated to use an 
alternative cleaner, 2) the usage of Cleaner outside the JDK, as 
determined by corpus analysis, has largely been observed to hack into 
private fields of the internal NIO direct buffer classes to explicitly 
release native memory. As stated in 1), the type of the cleaner used 
by NIO direct buffers will have to change. Given this, and the fact 
that JDK 9 has a new general purposed cleaner API, 
java.lang.ref.Cleaner, the value of keep sun.misc.Cleaner is 
questionable."


If the decision to remove sun.misc.Cleaner was partly influenced by 
the desire to maintain just 2 instead of 3 Cleaner(s), then my 
proposal to migrate JDK code to the public API might enable Oracle to 
reconsider keeping sun.misc.Cleaner.


OTOH, what hadoop is doing is exactly the usage described in the above 
reasoning for removing sun.misc.Cleaner.


Hadoop use case:

public static void freeDB(ByteBuffer buffer) {
if (buffer instanceof sun.nio.ch.DirectBuffer) {
final sun.misc.Cleaner bufferCleaner =
((sun.nio.ch.DirectBuffer) buffer).cleaner();
bufferCleaner.clean();
}
}

can be rewritten using reflection to be compatible with either 
sun.misc.Cleaner (on JDK 8 or less) or java.lang.ref.Cleaner$Cleanable 
(on JDK 9 or more):


static final Method cleanMethod;

static {
try {
Class cleanerOrCleanable;
try {
cleanerOrCleanable = Class.forName("sun.misc.Cleaner");
} catch (ClassNotFoundException e1) {
try {
cleanerOrCleanable = 
Class.forName("java.lang.ref.Cleaner$Cleanable");

} catch (ClassNotFoundException e2) {
e2.addSuppressed(e1);
throw e2;
}
}
cleanMethod = cleanerOrCleanable.getDeclaredMethod("clean");
} catch (Exception e) {
throw new Error(e);
}
}

public static void freeDB_JDK8or9(ByteBuffer buffer) {
if (buffer instanceof sun.nio.ch.DirectBuffer) {
final Object bufferCleaner =
((sun.nio.ch.DirectBuffer) buffer).cleaner();

try {
if (bufferCleaner != null) {
cleanMethod.invoke(bufferCleaner);
}
} catch (InvocationTargetException | IllegalAccessException 
e) {

throw new Error(e);
}
}
}


I thought about this use case and even kept the name of the method on 
sun.nio.ch.DirectBuffer::cleaner to make things easier although the 
return type in my patched DirectBufer is called 
java.lang.ref.Cleaner$Cleanable.



Regards, Peter



[1] https://bugs.openjdk.java.net/browse/JDK-8132928
[2] https://bugs.openjdk.java.net/browse/JDK-8148117

Regards, Peter



Jeremy

On Sun, Feb 7, 2016 at 2:53 AM, Peter Levart > wrote:


Hi,

sun.misc.Cleaner has been moved to internal package
jdk.internal.ref recently [1] to clean-up sun.misc namespace. But
now that:

- we have comparable public API (java.lang.ref.Cleaner &
Cleanable) [2]
- we have an internal shared java.lang.ref.Cleaner instance
(jdk.internal.ref.CleanerFactory.cleaner())
- sun.misc.Cleaner is not a special kind of Reference any more in
the JVM [3]

...I think there's no reason to keep this special internal API
any more. It can be replaced with public API.

I propose to remove jdk.internal.ref.Cleaner class and replace
its usages with java.lang.ref.Cleaner and friends [4].

What do you say?

Regards, Peter


[1] https://bugs.openjdk.java.net/browse/JDK-8148117
[2] https://bugs.openjdk.java.net/browse/JDK-8138696
[3] https://bugs.openjdk.java.net/browse/JDK-8143847
[4]

http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.01/

Re: We don't need jdk.internal.ref.Cleaner any more

[Peter Levart]

Hi,

I 1st thought that my version of jtreg did not want to parse the new JDK 
9 version strings as it kept saying:


Error: cannot determine version for JDK: 
build/linux-x86_64-normal-server-release/images/jdk


...but the fact was that the patched JDK did not even want to boot-up.

So in order to make this actually work, I had to make some adjustments:

- lambdas and method references in the java.lang.ref.Cleaner 
implementation had to be replaced with alternatives as the Cleaner is 
needed early in the startup sequence.
- [sun.misc|jdk.internal.ref].Cleaner.create(referent, action) allowed 
null action and just returned null in that case while 
java.lang.ref.Cleaner.register(referent, action) throws NPE. There was 
only one such usage where null action could get passed-in - when a 
zero-sized MappedByteBuffer was constructed.


Here's the version which passes java/lang/ref and java/nio tests:

http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.02/


Regards, Peter


On 02/07/2016 11:53 AM, Peter Levart wrote:

Hi,

sun.misc.Cleaner has been moved to internal package jdk.internal.ref 
recently [1] to clean-up sun.misc namespace. But now that:


- we have comparable public API (java.lang.ref.Cleaner & Cleanable) [2]
- we have an internal shared java.lang.ref.Cleaner instance 
(jdk.internal.ref.CleanerFactory.cleaner())
- sun.misc.Cleaner is not a special kind of Reference any more in the 
JVM [3]


...I think there's no reason to keep this special internal API any 
more. It can be replaced with public API.


I propose to remove jdk.internal.ref.Cleaner class and replace its 
usages with java.lang.ref.Cleaner and friends [4].


What do you say?

Regards, Peter


[1] https://bugs.openjdk.java.net/browse/JDK-8148117
[2] https://bugs.openjdk.java.net/browse/JDK-8138696
[3] https://bugs.openjdk.java.net/browse/JDK-8143847
[4] 
http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.01/

Re: We don't need jdk.internal.ref.Cleaner any more

[Peter Levart]

On 02/07/2016 08:01 PM, Jeremy Manson wrote:

Hadoop seems to use sun.misc.Cleaner:

http://grepcode.com/file/repo1.maven.org/maven2/org.apache.hadoop/hadoop-common/2.7.1/org/apache/hadoop/crypto/CryptoStreamUtils.java

So you may want to keep it around transitionally (a la Unsafe).


JEP 260 [1] was listing sun.misc.Cleaner as a critical API, but 
recently, it was decided to "hide" it away into jdk.internal.ref package 
which will not be exported [2]. The reasoning was this:


"sun.misc.Cleaner ( was previously listed as a critical internal API, 
but on further investigation has been moved to an open issue, for the 
following reasons: 1) its primary use in the JDK is within NIO direct 
buffers to release native memory. The base module cannot have a 
dependency on jdk.unsupported so will need to be updated to use an 
alternative cleaner, 2) the usage of Cleaner outside the JDK, as 
determined by corpus analysis, has largely been observed to hack into 
private fields of the internal NIO direct buffer classes to explicitly 
release native memory. As stated in 1), the type of the cleaner used by 
NIO direct buffers will have to change. Given this, and the fact that 
JDK 9 has a new general purposed cleaner API, java.lang.ref.Cleaner, the 
value of keep sun.misc.Cleaner is questionable."


If the decision to remove sun.misc.Cleaner was partly influenced by the 
desire to maintain just 2 instead of 3 Cleaner(s), then my proposal to 
migrate JDK code to the public API might enable Oracle to reconsider 
keeping sun.misc.Cleaner.


[1] https://bugs.openjdk.java.net/browse/JDK-8132928
[2] https://bugs.openjdk.java.net/browse/JDK-8148117

Regards, Peter



Jeremy

On Sun, Feb 7, 2016 at 2:53 AM, Peter Levart > wrote:


Hi,

sun.misc.Cleaner has been moved to internal package
jdk.internal.ref recently [1] to clean-up sun.misc namespace. But
now that:

- we have comparable public API (java.lang.ref.Cleaner &
Cleanable) [2]
- we have an internal shared java.lang.ref.Cleaner instance
(jdk.internal.ref.CleanerFactory.cleaner())
- sun.misc.Cleaner is not a special kind of Reference any more in
the JVM [3]

...I think there's no reason to keep this special internal API any
more. It can be replaced with public API.

I propose to remove jdk.internal.ref.Cleaner class and replace its
usages with java.lang.ref.Cleaner and friends [4].

What do you say?

Regards, Peter


[1] https://bugs.openjdk.java.net/browse/JDK-8148117
[2] https://bugs.openjdk.java.net/browse/JDK-8138696
[3] https://bugs.openjdk.java.net/browse/JDK-8143847
[4]

http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.01/

We don't need jdk.internal.ref.Cleaner any more

[Peter Levart]

Hi,

sun.misc.Cleaner has been moved to internal package jdk.internal.ref 
recently [1] to clean-up sun.misc namespace. But now that:


- we have comparable public API (java.lang.ref.Cleaner & Cleanable) [2]
- we have an internal shared java.lang.ref.Cleaner instance 
(jdk.internal.ref.CleanerFactory.cleaner())
- sun.misc.Cleaner is not a special kind of Reference any more in the 
JVM [3]


...I think there's no reason to keep this special internal API any more. 
It can be replaced with public API.


I propose to remove jdk.internal.ref.Cleaner class and replace its 
usages with java.lang.ref.Cleaner and friends [4].


What do you say?

Regards, Peter


[1] https://bugs.openjdk.java.net/browse/JDK-8148117
[2] https://bugs.openjdk.java.net/browse/JDK-8138696
[3] https://bugs.openjdk.java.net/browse/JDK-8143847
[4] 
http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.01/

Fwd: Re: [concurrency-interest] ClassLoader deadlock

[Peter Firmstone]

Thanks Peter & David,

That's good to know.

I've altered the SecurityManager to perform a permission check prior to
becoming the security manager, this ensures the cache has been created,
to avoid any recursive deadlock prone calls.

The class that both threads are attempting to load and init is called
org.apache.river.concurrent.ReferenceIterator.

The code can be found here:

https://github.com/pfirmstone/river-internet/tree/Input-validation-for-Serialization

Cheers,

Peter.


On 7/02/2016 6:37 PM, Peter Levart wrote:



 On 02/06/2016 10:32 PM, Peter wrote:

 The 0x040ebee8 monitor is most likely being held by native code.

 Regards,

 Peter Firmstone.


 Ok, but where? The deadlock report says it is held by main thread.
 Somewhere between the following two java frames?

 "main" #1 prio=5 os_prio=0 tid=0x017cf400 nid=0x1284 waiting for
 monitor entry [0x0185e000]
 ...
 ...
 at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
 --here--
 at
 
org.apache.river.concurrent.ReferenceCollection.iterator(ReferenceCollection.java:124)


 As I understand this is where JVM resolves some class that for the 1st
 time while ReferenceCollection.iterator is being executed. So before
 calling-back into ClassLoader.loadClass, it acquires a monitor lock on
 some int[] object?


 Anyway. Looking at the following part of Thread-1 stacktrace:

 at
 
org.apache.river.api.security.CombinerSecurityManager.checkPermission(CombinerSecurityManager.java:202)
 at
 java.net.NetworkInterface.getHardwareAddress(NetworkInterface.java:447)
 at
 java.util.concurrent.ThreadLocalRandom.initialSeed(ThreadLocalRandom.java:158)
 at
 java.util.concurrent.ThreadLocalRandom.(ThreadLocalRandom.java:137)
 at
 
java.util.concurrent.ConcurrentHashMap.fullAddCount(ConcurrentHashMap.java:2526)
 at
 java.util.concurrent.ConcurrentHashMap.addCount(ConcurrentHashMap.java:2266)
 at
 java.util.concurrent.ConcurrentHashMap.putVal(ConcurrentHashMap.java:1070)
 at
 java.util.concurrent.ConcurrentHashMap.putIfAbsent(ConcurrentHashMap.java:1535)

 ...I can see you are using some early release of JDK 8 where
 ThreadLocalRandom initialization involved obtaining a hardware address
 of some interface. This in turn checks for security manager. Since JDK
 8u45 this code has been removed and only current time is used to
 derive initial seed, so you might not see this deadlock any more with
 JDK 8u45 and later.

 Regards, Peter




 On 7/02/2016 4:27 AM, Peter Levart wrote:



 On 02/06/2016 01:17 PM, Peter Firmstone wrote:

 The security manager is non blocking, unfortunately java system
 classes aren't.


 It doesn't seem so. At least, I can't find where main thread locks
 the 0x040ebee8 monitor:

 "Thread-1":
   waiting to lock monitor 0x142766ac (object 0x040ebee8, a [I),
   which is held by "main"


 "main" #1 prio=5 os_prio=0 tid=0x017cf400 nid=0x1284 waiting for
 monitor entry [0x0185e000]
java.lang.Thread.State: BLOCKED (on object monitor)
 at java.lang.ClassLoader.loadClass(ClassLoader.java:406)
 - waiting to lock<0x03f624b8>  (a java.lang.Object)
 at
 sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:308)
 at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
 at
 
org.apache.river.concurrent.ReferenceCollection.iterator(ReferenceCollection.java:124)
 at
 org.apache.river.concurrent.ReferenceSet.hashCode(ReferenceSet.java:65)
 at
 org.apache.river.concurrent.StrongReference.(StrongReference.java:44)
 at
 org.apache.river.concurrent.StrongReference.(StrongReference.java:57)
 at
 org.apache.river.concurrent.ReferenceFactory.create(ReferenceFactory.java:64)
 at
 
org.apache.river.concurrent.ReferenceProcessor.referenced(ReferenceProcessor.java:128)
 at
 
org.apache.river.concurrent.ReferenceProcessor.referenced(ReferenceProcessor.java:44)
 at
 org.apache.river.concurrent.ReferenceMap.wrapVal(ReferenceMap.java:244)
 at
 
org.apache.river.concurrent.ReferenceConcurrentMap.putIfAbsent(ReferenceConcurrentMap.java:68)
 at
 
org.apache.river.api.security.CombinerSecurityManager.checkPermission(CombinerSecurityManager.java:261)
 at
 
org.apache.river.api.security.CombinerSecurityManager.checkPermission(CombinerSecurityManager.java:202)
 at java.lang.System.checkIO(System.java:253)
 at java.lang.System.setErr(System.java:199)
 at
 org.apache.river.qa.harness.MasterTest.main(MasterTest.java:84)


 ...no mention of 0x040ebee8 in main thread...?!

 Peter



 The policy provider in use thread confines PermissionCollection
 instances, which never leave the cpu cache, they are discarded
 immediately after use.

 The problem here is that two different threads are attempting to
 load the same class at the same time.

 I'll have to make sure that all classes are loaded before the
 security manager becomes the system security manager, eith

Using BaseStream.isParallel() after calling terminal operation

[Tagir F. Valeev]

Hello!

Currently the isParallel() spec says [1]:

> Calling this method after invoking an terminal stream operation
> method may yield unpredictable results.

The spliterator() spec says [2]:

> This is a terminal operation.

As a consequence, we cannot legally call isParallel() after calling
spliterator(). However it's actually called in Stream.concat()
implementation [3] as well as in Stream.takeWhile()/dropWhile()
default implementations [4], [5]. The same is for primitive streams.
In all of these cases (especially in concat case) it's possible that
third-party Stream interface implementation is passed, which,
according to spec, can legally return garbage or even throw
IllegalStateException upon calling isParallel(). So to my
understanding, either spec or implementations should be corrected to
match each other.

My suggestion is to make exceptions for calling isParallel() after
spliterator() or iterator() terminal operation: isParallel() should
still return correct value. Though probably it should be additionally
specified what is considered as correct value in this case.

What do you think? Should we bother about such small thing?

With best regards,
Tagir Valeev

[1] 
http://download.java.net/jdk9/docs/api/java/util/stream/BaseStream.html#isParallel--
[2] 
http://download.java.net/jdk9/docs/api/java/util/stream/BaseStream.html#spliterator--
[3] 
http://hg.openjdk.java.net/jdk9/dev/jdk/file/fddcdea594f5/src/java.base/share/classes/java/util/stream/Stream.java#l1248
[4] 
http://hg.openjdk.java.net/jdk9/dev/jdk/file/fddcdea594f5/src/java.base/share/classes/java/util/stream/Stream.java#l542
[5] 
http://hg.openjdk.java.net/jdk9/dev/jdk/file/fddcdea594f5/src/java.base/share/classes/java/util/stream/Stream.java#l608

Re: [concurrency-interest] ClassLoader deadlock

[Peter Levart]

On 02/06/2016 10:32 PM, Peter wrote:

The 0x040ebee8 monitor is most likely being held by native code.

Regards,

Peter Firmstone.


Ok, but where? The deadlock report says it is held by main thread. 
Somewhere between the following two java frames?


"main" #1 prio=5 os_prio=0 tid=0x017cf400 nid=0x1284 waiting for monitor 
entry [0x0185e000]

...
...
at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
--here--
at 
org.apache.river.concurrent.ReferenceCollection.iterator(ReferenceCollection.java:124)



As I understand this is where JVM resolves some class that for the 1st 
time while ReferenceCollection.iterator is being executed. So before 
calling-back into ClassLoader.loadClass, it acquires a monitor lock on 
some int[] object?



Anyway. Looking at the following part of Thread-1 stacktrace:

at 
org.apache.river.api.security.CombinerSecurityManager.checkPermission(CombinerSecurityManager.java:202)
at 
java.net.NetworkInterface.getHardwareAddress(NetworkInterface.java:447)
at 
java.util.concurrent.ThreadLocalRandom.initialSeed(ThreadLocalRandom.java:158)
at 
java.util.concurrent.ThreadLocalRandom.(ThreadLocalRandom.java:137)
at 
java.util.concurrent.ConcurrentHashMap.fullAddCount(ConcurrentHashMap.java:2526)
at 
java.util.concurrent.ConcurrentHashMap.addCount(ConcurrentHashMap.java:2266)
at 
java.util.concurrent.ConcurrentHashMap.putVal(ConcurrentHashMap.java:1070)
at 
java.util.concurrent.ConcurrentHashMap.putIfAbsent(ConcurrentHashMap.java:1535)


...I can see you are using some early release of JDK 8 where 
ThreadLocalRandom initialization involved obtaining a hardware address 
of some interface. This in turn checks for security manager. Since JDK 
8u45 this code has been removed and only current time is used to derive 
initial seed, so you might not see this deadlock any more with JDK 8u45 
and later.


Regards, Peter




On 7/02/2016 4:27 AM, Peter Levart wrote:



On 02/06/2016 01:17 PM, Peter Firmstone wrote:
The security manager is non blocking, unfortunately java system 
classes aren't.


It doesn't seem so. At least, I can't find where main thread locks 
the 0x040ebee8 monitor:


"Thread-1":
  waiting to lock monitor 0x142766ac (object 0x040ebee8, a [I),
  which is held by "main"


"main" #1 prio=5 os_prio=0 tid=0x017cf400 nid=0x1284 waiting for 
monitor entry [0x0185e000]

   java.lang.Thread.State: BLOCKED (on object monitor)
at java.lang.ClassLoader.loadClass(ClassLoader.java:406)
- waiting to lock <0x03f624b8> (a java.lang.Object)
at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:308)
at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
at 
org.apache.river.concurrent.ReferenceCollection.iterator(ReferenceCollection.java:124)
at 
org.apache.river.concurrent.ReferenceSet.hashCode(ReferenceSet.java:65)
at 
org.apache.river.concurrent.StrongReference.(StrongReference.java:44)
at 
org.apache.river.concurrent.StrongReference.(StrongReference.java:57)
at 
org.apache.river.concurrent.ReferenceFactory.create(ReferenceFactory.java:64)
at 
org.apache.river.concurrent.ReferenceProcessor.referenced(ReferenceProcessor.java:128)
at 
org.apache.river.concurrent.ReferenceProcessor.referenced(ReferenceProcessor.java:44)
at 
org.apache.river.concurrent.ReferenceMap.wrapVal(ReferenceMap.java:244)
at 
org.apache.river.concurrent.ReferenceConcurrentMap.putIfAbsent(ReferenceConcurrentMap.java:68)
at 
org.apache.river.api.security.CombinerSecurityManager.checkPermission(CombinerSecurityManager.java:261)
at 
org.apache.river.api.security.CombinerSecurityManager.checkPermission(CombinerSecurityManager.java:202)

at java.lang.System.checkIO(System.java:253)
at java.lang.System.setErr(System.java:199)
at 
org.apache.river.qa.harness.MasterTest.main(MasterTest.java:84)



...no mention of 0x040ebee8 in main thread...?!

Peter



The policy provider in use thread confines PermissionCollection 
instances, which never leave the cpu cache, they are discarded 
immediately after use.


The problem here is that two different threads are attempting to 
load the same class at the same time.


I'll have to make sure that all classes are loaded before the 
security manager becomes the system security manager, either that or 
eliminate the AccessControlContext check permission cache.


While the standard java security manager and policy provider consume 
around 10% cpu, this consumes less than 1%.


On 6/02/2016 9:57 PM, David Holmes wrote:

On 6/02/2016 9:39 PM, Peter Firmstone wrote:

Hmm, thought the new parallel lock stategy in ClassLoader wasn't
deadlock prone?  Guess I was wrong.


The deadlock is introduced by a custom security manager. SM's play 
a critical role in many parts of the core libraries so if they 
utilize synchronization then they can easily introduce deadlock.



Ob