Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently
On 01/20/2014 02:51 AM, David Holmes wrote: Hi Peter, On 17/01/2014 11:24 PM, Peter Levart wrote: On 01/17/2014 02:13 PM, Peter Levart wrote: // Fast path for cleaners boolean isCleaner = false; try { isCleaner = r instanceof Cleaner; } catch (OutofMemoryError oome) { continue; } if (isCleaner) { ((Cleaner)r).clean(); continue; } Hi David, Kalyan, I've caught-up now. Just thinking: is instanceof Cleaner throwing OOME as a result of loading the Cleaner class? Wouldn't the above code then throw some error also in ((Cleaner)r) - the checkcast, since Cleaner class would not be successfully initialized? Well, no. The above code would just skip Cleaner processing in this situation. And will never be doing it again after the heap is freed... So it might be good to load and initialize Cleaner class as part of ReferenceHandler initialization to ensure correct operation... Well, yes and no. Let me try once more: Above code will skip Cleaner processing if the 1st time instanceof Cleaner is executed, OOME is thrown as a consequence of full heap while loading and initializing the Cleaner class. Yes - I was assuming that this would not fail the very first time and so the Cleaner class would already be loaded. Failing to be able to load the Cleaner class was one of the potential issues flagged earlier with this problem. I was actually assuming that Cleaner would be loaded already due to some actual Cleaner subclasses being used, but this does not happen as part of the default initialization. :( The irony being that if the Cleaner class is not loaded then r can not be an instance of Cleaner and so we would fail to load the class in a case where we didn't need it anyway. What I wanted to focus on here was an OOME from the instanceof itself, but as you say that might trigger classloading of Cleaner (which is not what I was interested in). The 2nd time the instanceof Cleaner is executed after such OOME, the same line would throw NoClassDefFoundError as a consequence of referencing a class that failed initialization. Am I right? instanceof is not one of the class initialization triggers, so we should not see an OOME generated due to a class initialization exception and so the class will not be put into the Erroneous state and so subsequent attempts to use the class will not automatically trigger NoClassdefFoundError. If OOME occurs during actual loading/linking of the class Cleaner it is unclear what would happen on subsequent attempts. OOME is not a LinkageError that must be rethrown on subsequent attempts, and it is potentially a transient condition, so I would expect a re-load attempt to be allowed. However we are now deep into the details of the VM and it may well depend on the exact place from which the OOME originates. The bottom line with the current problem is that there are multiple non-obvious paths by which the ReferenceHandler can encounter an OOME. In such cases we do not want the ReferenceHandler to terminate - which implies catching the OOME and continuing. However we also do not want to silently skip Cleaner processing or reference queue processing - as that would lead to hard to diagnoze bugs. But trying to report the problem may not be possible due to being out-of-memory. It may be that we need to break things up into multiple try/catch blocks, where each catch does a System.gc() and then reports that the OOME occurred. Of course the reporting must still be in a try/catch for the OOME. Though at some point letting the ReferenceHandler die may be the only way to report a major memory problem. David Hm... If I give -verbose:class option to run a simple test program: public class Test { public static void main(String... a) {} } I see Cleaner class being loaded before Test class. I don't see by which tread or if it might get loaded after main() starts, but I suspect that loading of Cleaner is not a problem here. Initialization of Cleaner class is not performed by ReferenceHandler thread as you pointed out. The instanceof does not trigger it and if it returns true then Cleaner has already been initialized. So there must be some other cause for instanceof throwing OOME... What do you say about this variant of ReferenceHandler.run() method: public void run() { for (;;) { Reference r; Cleaner c; synchronized (lock) { r = pending; if (r != null) { // instanceof operator might throw OOME sometimes. Just retry after // yielding - might have better luck next time... try { c = r instanceof Cleaner ? (Cleaner) r : null; } catch (OutOfMemoryError x) { Thread.yield(); continue; } pending = r.discovered;
Re: Review request for JDK-6760902: inconsistent behavior in bootstrap class loader for classes and resources
On Jan 17, 2014, at 10:10 PM, Mandy Chung mandy.ch...@oracle.com wrote: Paul, Here is the updated webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6760902/webrev.01/ This cleans up GetResource.sh and merges with GetResource2.sh. Also fixed if (pos - lastPoc 0) line and javadoc typo. Looks good. -- In GetResource.sh: 89 GetResource $expected || exit 10 Should it be exit $count ?, i suppose it does not matter. Regardless no need for another review round for that. Paul.
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
On Fri, Jan 17, 2014 at 10:15 PM, Volker Simonis volker.simo...@gmail.com wrote: On Tue, Jan 14, 2014 at 10:19 AM, Alan Bateman alan.bate...@oracle.com wrote: On 14/01/2014 08:40, Volker Simonis wrote: Hi, could you please review the following changes for the ppc-aix-port stage/stage-9 repositories (the changes are planned for integration into ppc-aix-port/stage-9 and subsequent backporting to ppc-aix-port/stage): I'd like to review this but I won't have time until later in the week. From an initial look then there are a few things are not pretty (the changes to fix the AIX problems with I/O cancellation in particular) and I suspect that some refactoring is going to be required to handle some of this cleanly. A minor comment is that bug synopsis doesn't really communicate what these changes are about. -Alan. Just forwarded the following message from another thread here where it belongs: On 17/01/2014 16:57, Alan Bateman wrote: I've finally got to this one. As the event translation issue is now a separate issue then I've ignored that part. I'm not comfortable with the changes to FileDispatcherImpl.c as I don't think we shouldn't be calling into IO_ or NET_* functions here. I think I get the issue that you have on AIX (and assume it's the preClose/dup2 that blocks rather than close) but need a bit of time to suggest alternatives. It may be that it will require an AIX specific SocketDispatcher. Do you happen to know which tests fail due to this part? The other changes look okay. There is a typo in the change to zip_util.c, s/legel/legal/. In DatagramChannelImpl.c then you handle connect failing with EAFNOSUPPORT. I would be tempted to replace the comment to say that it EAFNOSUPPORT can be ignored on AIX. A minor comment but the indentation for rv = errno can be fixed (I see the BSD code has it wrong too). On 17/01/2014 21:23, Volker Simonis wrote: You're right, one race is with preClose/dup2 but also with other calls like read/fcntl/... There were several tests that failed and once I fixed it they all succeeded. But I can recreate some of the failures for you. The symptoms are always the same: the VMis locked. If you trigger a stack trace you can see that at least on thread is blocked in a I/O operation on a file descriptor like fcntl (e.g. for file locking), read, etc. while another thread is trying to close that socket. As it happens, we have some carry over issues from the Mac port, one of which is that async close of FileChannels will block indefinitely in dup2 when there is another thread blocked (on fnctl or reading from a pipe ...). I haven't time time to work on it but this discussion has reminded me that we need to sort it out. I've put a preliminary webrev with the changes here: http://cr.openjdk.java.net/~alanb/7133499/webrev/ The important part is that it's using signal consistently on Linux/Solaris/OSX so that any blocked threads are interrupted. My guess is that if NativeThread.c is updated to define a signal on AIX they this should resolve some of the issues on AIX. I would like to see the list of tests failing. If there is an issue with dup2 with sockets (and OS X doesn't seem to have that issue) then it will require further work but I would at least like to start by understanding if this patch will help with the FileChannel issues. Hi Alan, yes, that's interesting. Sounds like a very similar problem on Mac. I would suggest the following: I cut out the Async Close AIX FIX stuff from this change (i.e. 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests and send out a new webrev for the remaining part. I think that the remaining part was more or less reviewed and we can then push it faster. In the mean time, I'll recheck which tests exactly fail with my missing Async Close AIX FIX stuff and which of these tests will be fixed by your 7133499 webrev. Maybe we can really get trough with it or with it and a few enhancements. I'll let you know my results later today. By the way, my webrev already contained a AixNativeThread.c implementation in src/aix/native/sun/nio/ch. The only remaining problem I see with this approach is that we would need to downport your 7133499 change to 8u-dev in the 8u20 time frame to make our AIX port work. Would this be OK for you? Regards, Volker
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
On 20/01/2014 09:59, Volker Simonis wrote: : Hi Alan, yes, that's interesting. Sounds like a very similar problem on Mac. I would suggest the following: I cut out the Async Close AIX FIX stuff from this change (i.e. 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests and send out a new webrev for the remaining part. I think that the remaining part was more or less reviewed and we can then push it faster. In the mean time, I'll recheck which tests exactly fail with my missing Async Close AIX FIX stuff and which of these tests will be fixed by your 7133499 webrev. Maybe we can really get trough with it or with it and a few enhancements. I'll let you know my results later today. By the way, my webrev already contained a AixNativeThread.c implementation in src/aix/native/sun/nio/ch. The only remaining problem I see with this approach is that we would need to downport your 7133499 change to 8u-dev in the 8u20 time frame to make our AIX port work. Would this be OK for you? I'm okay with this plan and if you re-generate the webrev without the async close changes then I can look at it quickly so that you can get it into the stage-9 forest. On 7133499 then it would be a good candidate for 8u-dev too, I don't expect any problems but we will need to get it approved on the jdk8u-dev list. -Alan.
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
On Mon, Jan 20, 2014 at 12:41 PM, Alan Bateman alan.bate...@oracle.com wrote: On 20/01/2014 09:59, Volker Simonis wrote: : Hi Alan, yes, that's interesting. Sounds like a very similar problem on Mac. I would suggest the following: I cut out the Async Close AIX FIX stuff from this change (i.e. 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests and send out a new webrev for the remaining part. I think that the remaining part was more or less reviewed and we can then push it faster. In the mean time, I'll recheck which tests exactly fail with my missing Async Close AIX FIX stuff and which of these tests will be fixed by your 7133499 webrev. Maybe we can really get trough with it or with it and a few enhancements. I'll let you know my results later today. By the way, my webrev already contained a AixNativeThread.c implementation in src/aix/native/sun/nio/ch. The only remaining problem I see with this approach is that we would need to downport your 7133499 change to 8u-dev in the 8u20 time frame to make our AIX port work. Would this be OK for you? I'm okay with this plan and if you re-generate the webrev without the async close changes then I can look at it quickly so that you can get it into the stage-9 forest. On 7133499 then it would be a good candidate for 8u-dev too, I don't expect any problems but we will need to get it approved on the jdk8u-dev list. -Alan. Hi everybody, so here's the second version of this webrev: http://cr.openjdk.java.net/~simonis/webrevs/8031581_2/ The main changes compared to the first webrew are as follows: - the POLL-constants related stuff has been factored out into its own webrev (8031997: PPC64: Make the various POLL constants system dependant - https://bugs.openjdk.java.net/browse/JDK-8031997). - the Async close on AIX workarounds have been taken out as well and will be handled separately (probably together with Alans fix for http://cr.openjdk.java.net/~alanb/7133499/webrev/). - in the remaining files I've applied the changes suggested by Staffan, so I think the changes to the following files can be considered as reviewed: src/share/native/sun/management/DiagnosticCommandImpl.c src/solaris/native/sun/management/OperatingSystemImpl.c src/share/transport/socket/socketTransport.c src/share/classes/sun/tools/attach/META-INF/services/com.sun.tools.attach.spi.AttachProvider - I've added the following additional files to the change: src/aix/classes/sun/nio/ch/sctp/SctpChannelImpl.java src/aix/classes/sun/nio/ch/sctp/SctpMultiChannelImpl.java src/aix/classes/sun/nio/ch/sctp/SctpServerChannelImpl.java which are just empty stub implementations of the SCTP classes needed to pass the SCTP jtreg tests. All other changes should be the same like in the first review round. Thanks, Volker
Re: RFR: 8032190 It's unclear that flatMap will ensure each stream will be closed.
It is good to clarify that the streams are closed. I find the following updated wording a little odd, If a mapped stream is {@code null} then it treated as if it was an empty stream. I thought the previous wording was better, but that could be just me. -Chris. On 20/01/14 10:38, Paul Sandoz wrote: Hi, For the flatMap operations of streams we forgot to say what it does with the mapped streams after it has processed them i.e. closes them, which is important for I/O backed streams (e.g. map Path - StreamString for lines of a file). The following patch fixes that omission in the docs: https://bugs.openjdk.java.net/browse/JDK-8032190 I think this should be backported to 8. Paul. diff -r 9bf43f25eacd src/share/classes/java/util/stream/DoubleStream.java --- a/src/share/classes/java/util/stream/DoubleStream.java Sat Jan 18 10:57:41 2014 -0800 +++ b/src/share/classes/java/util/stream/DoubleStream.java Mon Jan 20 11:34:17 2014 +0100 @@ -150,10 +150,11 @@ /** * Returns a stream consisting of the results of replacing each element of - * this stream with the contents of the stream produced by applying the - * provided mapping function to each element. (If the result of the mapping - * function is {@code null}, this is treated as if the result was an empty - * stream.) + * this stream with the contents of a mapped stream produced by applying + * the provided mapping function to each element. Each mapped stream is + * closed (see {@link java.util.stream.BaseStream#close()}) after its + * contents have been placed into this stream. (If a mapped stream is + * {@code null} then it treated as if it was an empty stream.) * * pThis is an a href=package-summary.html#StreamOpsintermediate * operation/a. diff -r 9bf43f25eacd src/share/classes/java/util/stream/IntStream.java --- a/src/share/classes/java/util/stream/IntStream.java Sat Jan 18 10:57:41 2014 -0800 +++ b/src/share/classes/java/util/stream/IntStream.java Mon Jan 20 11:34:17 2014 +0100 @@ -146,10 +146,11 @@ /** * Returns a stream consisting of the results of replacing each element of - * this stream with the contents of the stream produced by applying the - * provided mapping function to each element. (If the result of the mapping - * function is {@code null}, this is treated as if the result was an empty - * stream.) + * this stream with the contents of a mapped stream produced by applying + * the provided mapping function to each element. Each mapped stream is + * closed (see {@link java.util.stream.BaseStream#close()}) after its + * contents have been placed into this stream. (If a mapped stream is + * {@code null} then it treated as if it was an empty stream.) * * pThis is an a href=package-summary.html#StreamOpsintermediate * operation/a. diff -r 9bf43f25eacd src/share/classes/java/util/stream/LongStream.java --- a/src/share/classes/java/util/stream/LongStream.javaSat Jan 18 10:57:41 2014 -0800 +++ b/src/share/classes/java/util/stream/LongStream.javaMon Jan 20 11:34:17 2014 +0100 @@ -151,10 +151,11 @@ /** * Returns a stream consisting of the results of replacing each element of - * this stream with the contents of the stream produced by applying the - * provided mapping function to each element. (If the result of the mapping - * function is {@code null}, this is treated as if the result was an empty - * stream.) + * this stream with the contents of a mapped stream produced by applying + * the provided mapping function to each element. Each mapped stream is + * closed (see {@link java.util.stream.BaseStream#close()}) after its + * contents have been placed into this stream. (If a mapped stream is + * {@code null} then it treated as if it was an empty stream.) * * pThis is an a href=package-summary.html#StreamOpsintermediate * operation/a. diff -r 9bf43f25eacd src/share/classes/java/util/stream/Stream.java --- a/src/share/classes/java/util/stream/Stream.javaSat Jan 18 10:57:41 2014 -0800 +++ b/src/share/classes/java/util/stream/Stream.javaMon Jan 20 11:34:17 2014 +0100 @@ -227,10 +227,11 @@ /** * Returns a stream consisting of the results of replacing each element of - * this stream with the contents of the stream produced by applying the - * provided mapping function to each element. (If the result of the mapping - * function is {@code null}, this is treated as if the result was an empty - * stream.) + * this stream with the contents of a mapped stream produced by applying + * the provided mapping function to each element. Each mapped stream is + * closed (see {@link java.util.stream.BaseStream#close()}) after its + * contents have been placed into this stream. (If a mapped stream is + * {@code null} then it treated as if it was an
Re: RFR [8027348] (process) Enhancement of handling async close of ProcessInputStream
Hello everybody! Can I consider this change approved for jdk9? Thanks in advance, Ivan Gerasimov On 20.11.2013 0:56, Martin Buchholz wrote: Ivan, Thanks for taking ownership of my proposed change. Looks good to me! (I can't actually push changes like this myself because I only do Linux these days) On Tue, Nov 19, 2013 at 12:42 PM, Ivan Gerasimov ivan.gerasi...@oracle.com mailto:ivan.gerasi...@oracle.com wrote: Hello all! May I have a review for the improvement contributed by Martin Buchholz? https://bugs.openjdk.java.net/browse/JDK-8027348 First, it the change performs the code cleanup, and second it makes the test much faster. This should also help to resolve the issue with the current version of the test, which was reported to fail intermittently by timeout. https://bugs.openjdk.java.net/browse/JDK-8028574 Here's the webrev with the change: http://cr.openjdk.java.net/~igerasim/8027348/0/webrev/ http://cr.openjdk.java.net/%7Eigerasim/8027348/0/webrev/ Sincerely yours, Ivan Gerasimov
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
On 20/01/2014 13:45, Volker Simonis wrote: : Hi everybody, so here's the second version of this webrev: http://cr.openjdk.java.net/~simonis/webrevs/8031581_2/ This looks okay to me. The typo (legel - legal) still exists in zip_util.c and maybe that can be fixed before you push this (no need to generate a few webrev of course). For the JDWP socket transport then it's interesting that shutdown is being used to cause the reader thread to be preempted. That may be useful when it comes to addressing the bigger async close issue. The main changes compared to the first webrew are as follows: - the POLL-constants related stuff has been factored out into its own webrev (8031997: PPC64: Make the various POLL constants system dependant - https://bugs.openjdk.java.net/browse/JDK-8031997). I see this has been pushed to ppc-aix-port/stage-9. Would you have any objection if I brought this into jdk9/dev (minus the AixPollPort change)? We can use a different bug number so as not to cause duplicate bug issues. It should trivially merge when you come to sync'ing up the staging forest. - the Async close on AIX workarounds have been taken out as well and will be handled separately Thanks for separating this one out as I suspect this that doing this cleanly is going to involve changes for all platforms. -Alan.
Re: RFR: 8032190 It's unclear that flatMap will ensure each stream will be closed.
On Jan 20, 2014, at 3:18 PM, Chris Hegarty chris.hega...@oracle.com wrote: It is good to clarify that the streams are closed. I find the following updated wording a little odd, If a mapped stream is {@code null} then it treated as if it was an empty stream. I thought the previous wording was better, but that could be just me. I was hopping to use the term mapped stream to avoid some repetition for the result of the mapping function etc, but wording seems a little garbled. How about: If a mapped stream is {@code null} an empty stream is used, instead. Paul.
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
On Mon, Jan 20, 2014 at 4:24 PM, Alan Bateman alan.bate...@oracle.com wrote: On 20/01/2014 13:45, Volker Simonis wrote: : Hi everybody, so here's the second version of this webrev: http://cr.openjdk.java.net/~simonis/webrevs/8031581_2/ This looks okay to me. Thanks. The typo (legel - legal) still exists in zip_util.c and maybe that can be fixed before you push this (no need to generate a few webrev of course). Sorry, I've just fixed it in my patch queue and will used the fixed version for pushing. @Vladimir: could you please run this change (http://cr.openjdk.java.net/~simonis/webrevs/8031581_2) through JPRT as well. I'll push it (together with the fixed typo in the comment) if everything is OK. For the JDWP socket transport then it's interesting that shutdown is being used to cause the reader thread to be preempted. That may be useful when it comes to addressing the bigger async close issue. The main changes compared to the first webrew are as follows: - the POLL-constants related stuff has been factored out into its own webrev (8031997: PPC64: Make the various POLL constants system dependant - https://bugs.openjdk.java.net/browse/JDK-8031997). I see this has been pushed to ppc-aix-port/stage-9. Would you have any objection if I brought this into jdk9/dev (minus the AixPollPort change)? We can use a different bug number so as not to cause duplicate bug issues. It should trivially merge when you come to sync'ing up the staging forest. I have no objections of course. I'm just not sure what exact implications this will have. @Vladimir: what do you think - can Alan push 8031997: PPC64: Make the various POLL constants system dependant minus the Aix-specific stuff to jdk9/dev now, without causing you any harm during integration. @Alan: on the other hand, the bulk integration from ppc-aix-port/stage-9 to jdk9/dev is planned for next week anyway, so maybe you could wait until that happens? Thanks, Volker - the Async close on AIX workarounds have been taken out as well and will be handled separately Thanks for separating this one out as I suspect this that doing this cleanly is going to involve changes for all platforms. -Alan.
Re: RFR(L): 8031581: PPC64: Addons and fixes for AIX to pass the jdk regression tests
On 20/01/2014 16:29, Volker Simonis wrote: : @Alan: on the other hand, the bulk integration from ppc-aix-port/stage-9 to jdk9/dev is planned for next week anyway, so maybe you could wait until that happens? In that case then ignore my request, I assumed it would not be pushed to jdk9/dev until end-Feb. -Alan
Re: RFR: JDK-8032012, , String.toLowerCase/toUpperCase performance improvement
On Jan 16, 2014, at 7:08 PM, Xueming Shen xueming.s...@oracle.com wrote: Hi, The proposed changeset is to improve the performance (both speed and memory usage) of String.toLowerCase/toUpperCase, by (1) to separate the most likely use scenario (non supplementary character, no special case mapping handling) into simple/quick iteration loop code path (2) to use the package private constructor String(char[], boolean) to avoid unnecessary target char[] copy in fast-path case. (3) some tiny code cleanup. Since it's supposed to be a simple/quick round of code cleanup, I did not go too far to optimize the supplementary/special mapping code. The webrev is http://cr.openjdk.java.net/~sherman/8032012/ Some quick comments. In String.toLowerCase: - it would be nice to get rid of the pseudo goto using the scan labelled block. - there is no need for the localeDependent local variable. - you might be able to optimize by doing (could depend on the answer to the next point): int c = (int)value[i]; int lc = Character.toLowerCase(c); if (.) { result[i] = (char)lc; } else { return toLowerCaseEx(result, i, locale, localeDependent); } - Do you need to check ERROR for the result of toLowerCase? 2586 if (c == Character.ERROR || Paul.
Re: JDK 9 RFR for 8031980: Add new j.u.l deadlock test for JDK-8027670 and JDK-8029281
On 1/17/2014 6:51 AM, Daniel Fuchs wrote: Hi, Please find below a webrev for: 8031980: Add new j.u.l deadlock test for JDK-8027670 and JDK-8029281 https://bugs.openjdk.java.net/browse/JDK-8031980 http://cr.openjdk.java.net/~dfuchs/webrev_8031980/webrev.00/ The new test looks reasonable.foo.bar5.level = INFO is commented out in deadlock.conf.properties. Is this intentional? The test runs in the same VM first without security manager and then with security manager. The loggers have been created and may/may not be GC'ed. I wonder if it's worth breaking them into two separate runs so that each run starts with no logger. Have you also tried running this with -Xcomp and see if any intermittent failure due to GC? It looks fine when reviewing the test but better to verify it. Mandy
Re: JDK 9 RFR for 8031980: Add new j.u.l deadlock test for JDK-8027670 and JDK-8029281
On 1/20/14 7:17 PM, Mandy Chung wrote: On 1/17/2014 6:51 AM, Daniel Fuchs wrote: Hi, Please find below a webrev for: 8031980: Add new j.u.l deadlock test for JDK-8027670 and JDK-8029281 https://bugs.openjdk.java.net/browse/JDK-8031980 http://cr.openjdk.java.net/~dfuchs/webrev_8031980/webrev.00/ The new test looks reasonable.foo.bar5.level = INFO is commented out in deadlock.conf.properties. Is this intentional? Yes - I still wanted to have a case where a logger wouldn't have a parent in the configuration file - just in case (although that should already be tested by the 'old' test). The test runs in the same VM first without security manager and then with security manager. The loggers have been created and may/may not be GC'ed. I wonder if it's worth breaking them into two separate runs so that each run starts with no logger. hmm. good point. I don't think it really needs to though. Most of the time the deadlock will happen with with no security, but sometimes you have to wait for the with security before it does happen. And sometime you have to rerun the test - but that is rare. IMHO a bit of randomness is not bad for that kind of test. Have you also tried running this with -Xcomp and see if any intermittent failure due to GC? It looks fine when reviewing the test but better to verify it. No - haven't tried yet. Will do. However I saw it fail with errors indicating that loggers which should not have been gc'ed had been gc'ed when I ran it on a platform that didn't have JDK-8029281, which means that it's also a good test for the changes in LoggerWeakRef.dispose() from JDK-8029281. May I push after verifying that it doesn't fail with -Xcomp? best regards, -- daniel Mandy
Re: JDK 9 RFR for 8031980: Add new j.u.l deadlock test for JDK-8027670 and JDK-8029281
On 1/20/2014 10:35 AM, Daniel Fuchs wrote: On 1/20/14 7:17 PM, Mandy Chung wrote: On 1/17/2014 6:51 AM, Daniel Fuchs wrote: Hi, Please find below a webrev for: 8031980: Add new j.u.l deadlock test for JDK-8027670 and JDK-8029281 https://bugs.openjdk.java.net/browse/JDK-8031980 http://cr.openjdk.java.net/~dfuchs/webrev_8031980/webrev.00/ The new test looks reasonable.foo.bar5.level = INFO is commented out in deadlock.conf.properties. Is this intentional? Yes - I still wanted to have a case where a logger wouldn't have a parent in the configuration file - just in case (although that should already be tested by the 'old' test). That's what I suspect too - I suggest adding a comment to make it clear. Have you also tried running this with -Xcomp and see if any intermittent failure due to GC? It looks fine when reviewing the test but better to verify it. No - haven't tried yet. Will do. However I saw it fail with errors indicating that loggers which should not have been gc'ed had been gc'ed when I ran it on a platform that didn't have JDK-8029281, which means that it's also a good test for the changes in LoggerWeakRef.dispose() from JDK-8029281. May I push after verifying that it doesn't fail with -Xcomp? Yes and thanks for being proactive to verify that. Mandy
Re: RFR: 8032190 It's unclear that flatMap will ensure each stream will be closed.
On 20 Jan 2014, at 15:18, Paul Sandoz paul.san...@oracle.com wrote: On Jan 20, 2014, at 3:18 PM, Chris Hegarty chris.hega...@oracle.com wrote: It is good to clarify that the streams are closed. I find the following updated wording a little odd, If a mapped stream is {@code null} then it treated as if it was an empty stream. I thought the previous wording was better, but that could be just me. I was hopping to use the term mapped stream to avoid some repetition for the result of the mapping function etc, but wording seems a little garbled. How about: If a mapped stream is {@code null} an empty stream is used, instead. Thanks Paul. Seems fine to me. -Chris. Paul.
Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently
On 01/21/2014 03:22 AM, David Holmes wrote: Hi Peter, I do not see Cleaner being loaded prior to the main class on either Windows or Linux. Which platform are you on? Did you see it loaded before the main class or as part of executing it? Before. The main class is empty: public class Test { public static void main(String... a) {} } Here's last few lines of -verbose:class: [Loaded java.util.TimeZone from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded sun.util.calendar.ZoneInfo from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded sun.util.calendar.ZoneInfoFile from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded sun.util.calendar.ZoneInfoFile$1 from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.io.DataInput from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.io.DataInputStream from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] *[Loaded sun.misc.Cleaner from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar]* [Loaded java.io.ByteArrayInputStream from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded sun.util.calendar.ZoneInfoFile$ZoneOffsetTransitionRule from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.util.zip.Checksum from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.util.zip.CRC32 from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded sun.util.calendar.ZoneInfoFile$Checksum from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.util.TimeZone$1 from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded sun.util.calendar.CalendarDate from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded sun.util.calendar.BaseCalendar$Date from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded sun.util.calendar.Gregorian$Date from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded sun.util.calendar.CalendarUtils from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.util.jar.JarEntry from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.util.jar.JarFile$JarFileEntry from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.util.zip.ZipFile$ZipFileInputStream from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.util.AbstractSequentialList from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.util.LinkedList from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.util.LinkedList$Node from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.security.PrivilegedActionException from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded sun.misc.URLClassPath$FileLoader from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded sun.misc.Resource from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded sun.misc.URLClassPath$FileLoader$1 from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded sun.nio.ByteBuffered from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.security.PermissionCollection from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.security.Permissions from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.net.URLConnection from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded sun.net.www.URLConnection from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded sun.net.www.protocol.file.FileURLConnection from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded sun.net.www.MessageHeader from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.io.FilePermission from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.io.FilePermission$1 from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.io.FilePermissionCollection from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.security.AllPermission from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.security.UnresolvedPermission from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.security.BasicPermissionCollection from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] *[Loaded Test from file:/tmp/]* [Loaded sun.launcher.LauncherHelper$FXHelper from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.lang.Shutdown from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] [Loaded java.lang.Shutdown$Lock from /home/peter/Apps64/jdk1.8.0-ea-b121/jre/lib/rt.jar] I'm on linux, 64bit and using official EA build 121 of JDK 8... But if I try with JDK 7u45, I don't see it. So perhaps it would be good to trigger Cleaner loading and initialization as part of ReferenceHandler initialization to play things safe. Also, it is not that I think ReferenceHandler is responsible for reporting OOME, but that it is responsible for reporting that it was unable to perform a clean or enqueue because of OOME. This would be necessary if we