Re: Draft JEP on enhanced volatiles
On 02/11/2014 09:46 PM, Martin Buchholz wrote: On Mon, Feb 10, 2014 at 3:43 PM, Mike Duigou mike.dui...@oracle.com wrote: On Feb 10 2014, at 04:33 , Doug Lea d...@cs.oswego.edu wrote: Among the constraints on solution is that several of these methods compile down to no-ops on many common platforms. This is interpreted as a benefit. However, if the are no-ops only for the most common platform (x86) then we are likely to see a rough path for other platforms. Incorrect or inconsistent usage of these methods in user code will, instead of having no effect, result in unexpected and possibly mysterious runtime failures on other platforms. We already have the problem that programs that succeed on x86 may fail on other platforms with a weaker memory model. The JVM has been pretty good about hiding these differences thus far. It would be nice to see a least common denominator non-no-op mode as part of the VM implementation that provided the opportunity to test in the most pessimistic conditions. If you wanted to be able enable robust testing, create an adversarial VM implementation (e.g. via hotspot flag) that did maximal compiler reordering, e.g. between synchronization points write values to main memory in reverse order. But that's a big job that's probably not going to happen? Valgrind has a race checker that can be used for this kind of thing, and it can probably be utilized for Java. I wouldn't be surprised if we found some races in HotSpot. Andrew.
Re: Time to remove sun.misc.Service?
On 11/02/2014 11:44, Paul Sandoz wrote: : Scrub it! AFAICT it is not widely used (looking at the use of s.m.Service static methods on grep code there are only a handful of dependent artifacts). And the upgrade is quite easy. I didn't see any more comments on this and I agree there isn't much of a case for keeping it around. Here's the webrev to remove it: http://cr.openjdk.java.net/~alanb/8034776/webrev/ -Alan.
Re: Time to remove sun.misc.Service?
Looks good. -Sundar On Wednesday 12 February 2014 06:17 PM, Alan Bateman wrote: On 11/02/2014 11:44, Paul Sandoz wrote: : Scrub it! AFAICT it is not widely used (looking at the use of s.m.Service static methods on grep code there are only a handful of dependent artifacts). And the upgrade is quite easy. I didn't see any more comments on this and I agree there isn't much of a case for keeping it around. Here's the webrev to remove it: http://cr.openjdk.java.net/~alanb/8034776/webrev/ -Alan.
Re: RFR for JDK-8028711: TEST_BUG: Tests should pass through VM options: corelibs tests
On 02/10/2014 05:36 PM, Alan Bateman wrote: A minor comment on test/java/net/URLPermission/nstest/lookup.sh is that the resulting line length is 182 characters and this will likely be annoying for future side-by-side views. So I think I'd split this while you are there. Please review the updated version at http://cr.openjdk.java.net/~tyan/michael/JDK-8028711/webrev.04/ http://cr.openjdk.java.net/%7Etyan/michael/JDK-8028711/webrev.04/ Changes includes : 1. split line if it longer than 80 characters. 2. merge the fix of JDK-8033897 https://bugs.openjdk.java.net/browse/JDK-8033897 3. add few missed scripts. If no further changes need to be made, I would like to find sponsor to push this fix. Michael Cui
Re: Time to remove sun.misc.Service?
+1 On Feb 12, 2014, at 7:47 AM, Alan Bateman alan.bate...@oracle.com wrote: On 11/02/2014 11:44, Paul Sandoz wrote: : Scrub it! AFAICT it is not widely used (looking at the use of s.m.Service static methods on grep code there are only a handful of dependent artifacts). And the upgrade is quite easy. I didn't see any more comments on this and I agree there isn't much of a case for keeping it around. Here's the webrev to remove it: http://cr.openjdk.java.net/~alanb/8034776/webrev/ -Alan. Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
8034780: Remove used imports
I need a reviewer for a trivial change to remove a tiny number of unused imports. The motive is experimental compilation of the JDK as modules rather than one big compilation unit. I've no doubt that there is other code that has unused imports but it's not the goal of this exercise to identify and remove these. The proposal patch is below. -Alan diff --git a/src/share/classes/java/lang/invoke/MethodHandle.java b/src/share/classes/java/lang/invoke/MethodHandle.java --- a/src/share/classes/java/lang/invoke/MethodHandle.java +++ b/src/share/classes/java/lang/invoke/MethodHandle.java @@ -31,8 +31,6 @@ import sun.misc.Unsafe; import static java.lang.invoke.MethodHandleStatics.*; -import java.util.logging.Level; -import java.util.logging.Logger; /** * A method handle is a typed, directly executable reference to an underlying method, diff --git a/src/share/classes/java/lang/invoke/SimpleMethodHandle.java b/src/share/classes/java/lang/invoke/SimpleMethodHandle.java --- a/src/share/classes/java/lang/invoke/SimpleMethodHandle.java +++ b/src/share/classes/java/lang/invoke/SimpleMethodHandle.java @@ -27,8 +27,6 @@ import static java.lang.invoke.LambdaForm.*; import static java.lang.invoke.MethodHandleNatives.Constants.*; -import java.util.logging.Level; -import java.util.logging.Logger; /** * A method handle whose behavior is determined only by its LambdaForm. diff --git a/src/share/classes/sun/applet/AppletViewerPanel.java b/src/share/classes/sun/applet/AppletViewerPanel.java --- a/src/share/classes/sun/applet/AppletViewerPanel.java +++ b/src/share/classes/sun/applet/AppletViewerPanel.java @@ -31,7 +31,6 @@ import java.net.MalformedURLException; import java.awt.*; import java.applet.*; -import sun.tools.jar.*;
Re: Time to remove sun.misc.Service?
++1 -Chris. On 12/02/14 13:15, Lance Andersen wrote: +1 On Feb 12, 2014, at 7:47 AM, Alan Bateman alan.bate...@oracle.com wrote: On 11/02/2014 11:44, Paul Sandoz wrote: : Scrub it! AFAICT it is not widely used (looking at the use of s.m.Service static methods on grep code there are only a handful of dependent artifacts). And the upgrade is quite easy. I didn't see any more comments on this and I agree there isn't much of a case for keeping it around. Here's the webrev to remove it: http://cr.openjdk.java.net/~alanb/8034776/webrev/ -Alan. Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: 8034780: Remove used imports
Looks good to me Alan. -Chris. On 12/02/14 13:19, Alan Bateman wrote: I need a reviewer for a trivial change to remove a tiny number of unused imports. The motive is experimental compilation of the JDK as modules rather than one big compilation unit. I've no doubt that there is other code that has unused imports but it's not the goal of this exercise to identify and remove these. The proposal patch is below. -Alan diff --git a/src/share/classes/java/lang/invoke/MethodHandle.java b/src/share/classes/java/lang/invoke/MethodHandle.java --- a/src/share/classes/java/lang/invoke/MethodHandle.java +++ b/src/share/classes/java/lang/invoke/MethodHandle.java @@ -31,8 +31,6 @@ import sun.misc.Unsafe; import static java.lang.invoke.MethodHandleStatics.*; -import java.util.logging.Level; -import java.util.logging.Logger; /** * A method handle is a typed, directly executable reference to an underlying method, diff --git a/src/share/classes/java/lang/invoke/SimpleMethodHandle.java b/src/share/classes/java/lang/invoke/SimpleMethodHandle.java --- a/src/share/classes/java/lang/invoke/SimpleMethodHandle.java +++ b/src/share/classes/java/lang/invoke/SimpleMethodHandle.java @@ -27,8 +27,6 @@ import static java.lang.invoke.LambdaForm.*; import static java.lang.invoke.MethodHandleNatives.Constants.*; -import java.util.logging.Level; -import java.util.logging.Logger; /** * A method handle whose behavior is determined only by its LambdaForm. diff --git a/src/share/classes/sun/applet/AppletViewerPanel.java b/src/share/classes/sun/applet/AppletViewerPanel.java --- a/src/share/classes/sun/applet/AppletViewerPanel.java +++ b/src/share/classes/sun/applet/AppletViewerPanel.java @@ -31,7 +31,6 @@ import java.net.MalformedURLException; import java.awt.*; import java.applet.*; -import sun.tools.jar.*;
Re: 8034780: Remove used imports
+1 -Sundar On Wednesday 12 February 2014 06:49 PM, Alan Bateman wrote: I need a reviewer for a trivial change to remove a tiny number of unused imports. The motive is experimental compilation of the JDK as modules rather than one big compilation unit. I've no doubt that there is other code that has unused imports but it's not the goal of this exercise to identify and remove these. The proposal patch is below. -Alan diff --git a/src/share/classes/java/lang/invoke/MethodHandle.java b/src/share/classes/java/lang/invoke/MethodHandle.java --- a/src/share/classes/java/lang/invoke/MethodHandle.java +++ b/src/share/classes/java/lang/invoke/MethodHandle.java @@ -31,8 +31,6 @@ import sun.misc.Unsafe; import static java.lang.invoke.MethodHandleStatics.*; -import java.util.logging.Level; -import java.util.logging.Logger; /** * A method handle is a typed, directly executable reference to an underlying method, diff --git a/src/share/classes/java/lang/invoke/SimpleMethodHandle.java b/src/share/classes/java/lang/invoke/SimpleMethodHandle.java --- a/src/share/classes/java/lang/invoke/SimpleMethodHandle.java +++ b/src/share/classes/java/lang/invoke/SimpleMethodHandle.java @@ -27,8 +27,6 @@ import static java.lang.invoke.LambdaForm.*; import static java.lang.invoke.MethodHandleNatives.Constants.*; -import java.util.logging.Level; -import java.util.logging.Logger; /** * A method handle whose behavior is determined only by its LambdaForm. diff --git a/src/share/classes/sun/applet/AppletViewerPanel.java b/src/share/classes/sun/applet/AppletViewerPanel.java --- a/src/share/classes/sun/applet/AppletViewerPanel.java +++ b/src/share/classes/sun/applet/AppletViewerPanel.java @@ -31,7 +31,6 @@ import java.net.MalformedURLException; import java.awt.*; import java.applet.*; -import sun.tools.jar.*;
Re: 8034780: Remove used imports
On 2/12/14 2:19 PM, Alan Bateman wrote: I need a reviewer for a trivial change to remove a tiny number of unused imports. The motive is experimental compilation of the JDK as modules rather than one big compilation unit. I've no doubt that there is other code that has unused imports but it's not the goal of this exercise to identify and remove these. The proposal patch is below. +1 I had noticed the unused imports in java.lang the other day when I looked at which packages were using logging - and I was intending to log an RFE about it :-) -- daniel -Alan diff --git a/src/share/classes/java/lang/invoke/MethodHandle.java b/src/share/classes/java/lang/invoke/MethodHandle.java --- a/src/share/classes/java/lang/invoke/MethodHandle.java +++ b/src/share/classes/java/lang/invoke/MethodHandle.java @@ -31,8 +31,6 @@ import sun.misc.Unsafe; import static java.lang.invoke.MethodHandleStatics.*; -import java.util.logging.Level; -import java.util.logging.Logger; /** * A method handle is a typed, directly executable reference to an underlying method, diff --git a/src/share/classes/java/lang/invoke/SimpleMethodHandle.java b/src/share/classes/java/lang/invoke/SimpleMethodHandle.java --- a/src/share/classes/java/lang/invoke/SimpleMethodHandle.java +++ b/src/share/classes/java/lang/invoke/SimpleMethodHandle.java @@ -27,8 +27,6 @@ import static java.lang.invoke.LambdaForm.*; import static java.lang.invoke.MethodHandleNatives.Constants.*; -import java.util.logging.Level; -import java.util.logging.Logger; /** * A method handle whose behavior is determined only by its LambdaForm. diff --git a/src/share/classes/sun/applet/AppletViewerPanel.java b/src/share/classes/sun/applet/AppletViewerPanel.java --- a/src/share/classes/sun/applet/AppletViewerPanel.java +++ b/src/share/classes/sun/applet/AppletViewerPanel.java @@ -31,7 +31,6 @@ import java.net.MalformedURLException; import java.awt.*; import java.applet.*; -import sun.tools.jar.*;
Re: 8034780: Remove used imports
+1 On Feb 12, 2014, at 8:19 AM, Alan Bateman alan.bate...@oracle.com wrote: I need a reviewer for a trivial change to remove a tiny number of unused imports. The motive is experimental compilation of the JDK as modules rather than one big compilation unit. I've no doubt that there is other code that has unused imports but it's not the goal of this exercise to identify and remove these. The proposal patch is below. -Alan diff --git a/src/share/classes/java/lang/invoke/MethodHandle.java b/src/share/classes/java/lang/invoke/MethodHandle.java --- a/src/share/classes/java/lang/invoke/MethodHandle.java +++ b/src/share/classes/java/lang/invoke/MethodHandle.java @@ -31,8 +31,6 @@ import sun.misc.Unsafe; import static java.lang.invoke.MethodHandleStatics.*; -import java.util.logging.Level; -import java.util.logging.Logger; /** * A method handle is a typed, directly executable reference to an underlying method, diff --git a/src/share/classes/java/lang/invoke/SimpleMethodHandle.java b/src/share/classes/java/lang/invoke/SimpleMethodHandle.java --- a/src/share/classes/java/lang/invoke/SimpleMethodHandle.java +++ b/src/share/classes/java/lang/invoke/SimpleMethodHandle.java @@ -27,8 +27,6 @@ import static java.lang.invoke.LambdaForm.*; import static java.lang.invoke.MethodHandleNatives.Constants.*; -import java.util.logging.Level; -import java.util.logging.Logger; /** * A method handle whose behavior is determined only by its LambdaForm. diff --git a/src/share/classes/sun/applet/AppletViewerPanel.java b/src/share/classes/sun/applet/AppletViewerPanel.java --- a/src/share/classes/sun/applet/AppletViewerPanel.java +++ b/src/share/classes/sun/applet/AppletViewerPanel.java @@ -31,7 +31,6 @@ import java.net.MalformedURLException; import java.awt.*; import java.applet.*; -import sun.tools.jar.*; Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
RFR for JDK-8031563: TEST_BUG: java/nio/channels/Selector/ChangingInterests.java failed once
Hi, Please review two suggested fixes for https://bugs.openjdk.java.net/browse/JDK-8031563 Root cause: selectNow is called right after 50 milliseconds sleep in write1 method which may not be sufficient to make the socket channel readable in some situation. Since selectNow called only once and verification based on its result will fail if readable event wasn't ready at that time. Suggested fix A : simply increase the wait time from 50 to 1000 in write1() method. webrev link : http://cr.openjdk.java.net/~tyan/michael/JDK-8031563/webrev_v1/ http://cr.openjdk.java.net/%7Etyan/michael/JDK-8031563/webrev_v1/ Comments from Shura : Simply increasing the timeout will not fix the problem forever. Is it possible to re-write the tests so that the channel is waited to be readable instead? Suggested fix B : In testChange() , replacing original [select - verify] block by [wait - select - until verify or timeout] block. webrev link : http://cr.openjdk.java.net/~tyan/michael/JDK-8031563/webrev_v2/ http://cr.openjdk.java.net/%7Etyan/michael/JDK-8031563/webrev_v2/ Explanation : Test will fail intermittently when interest setting are (OP_READ | OP_WRITE). The goal is waiting until interested event coming or timeout. However, interest setting is OP_READ | OP_WRITE, so the blocking version api select() / select(long timeout) will return immediately as long as one of interested type event is ready (see selector class api). Moreover, SocketChannel is always writable if its internal buffer doesn't full. So all selection api will return immediately when interest setting contains OP_WRITE. So based on selector's api explanation and my testing, it is impossible to implement [wait - select - until verify or timeout] logic without using loop. The verification logic is adjusted slightly for while - loop. Another thing I would like to mention is the selector's selected-key set will be updated only by selection or manually remove. In other words, the selector.selectedKeys() will only reflect io events of last select operation even if a new interested event has arrived right after last selection. Comments from Shura : I have concern about two areas which might make your version and original version not equivalent. one is code changes in verification logic are different. the other is old version called selectNow() only once, your version replace it by calling it repeatedly in worst case. Both fixes were tested on 4 platforms (win, linux, mac, solaris)for 2000 runs without seeing any error. Any suggestion? Thanks, Michael Cui
methods that throws checked exceptions in a lambda block
This line does not compile Files.list(Paths.get(/secret)).forEach(x - Files.write(x, new byte[Files.size(x)]); because Files.write() and Files.size() throw IOE. Then what's the best way to make it work? (I find some old threads on this on lambda-dev. Maybe we have a recommended way now?) Thanks Max
Re: A hole in the serialization spec
David, others? I'm afraid I'm still not clear what is mean by: ... undefined in cases where the ObjectInputStream cannot resolve the class which defined the writeObject method in question. This does not seem directly related to the issue we are discussing ( whether to invoke default read/write object ). -Chris. On 10/02/14 15:37, David M. Lloyd wrote: I agree that it's a problem; however it's also clear that there are many classes in the wild which have this problem. It would be nice if the behavior could _become_ defined *somehow* though. I can see at least four options: 1) do nothing :( 2) start throwing (or writing) an exception in write/readObject when stream ops are performed without reading fields (maybe can be disabled with a sys prop or something) 3) leave fields cleared and risk protocol issues 4) silently start reading/writing empty field information (risks protocol issues) Maybe there are better options I'm not thinking of. On 02/10/2014 08:53 AM, Chris Hegarty wrote: David, ... undefined in cases where the ObjectInputStream cannot resolve the class which defined the writeObject method in question. I'm not clear as to what this statement is about? I'm sure you already know this, and maybe in your environment do not care much about it, but having a read/writeObject not invoke the appropriate default read/write Object/Fields method is a serious impediment to evolving the serial form ( in a compatible way ). For example, if your class has no serializable fields in one revision, but adds a serializable field(s) in a subsequent revision. This could lead to a StreamCorruptedException, or some other undefined behavior. The OpenJDK sources do seem to be quite tolerant of this situation. I'm not entirely sure if this is a good or a bad thing. That said, I don't think we want to encourage this kind of behavior. -Chris. On 07/02/14 15:07, David M. Lloyd wrote: Since the topic of serialization has come up recently, I'll take it as an excuse to bring up a problem that I've run into a couple of times with the serialization specification, which has resulted in user problems. If you read section 2.3 [1] of the specification, it says: The class's writeObject method, if implemented, is responsible for saving the state of the class. Either ObjectOutputStream's defaultWriteObject or writeFields method must be called once (and only once) before writing any optional data that will be needed by the corresponding readObject method to restore the state of the object; even if no optional data is written, defaultWriteObject or writeFields must still be invoked once. If defaultWriteObject or writeFields is not invoked once prior to the writing of optional data (if any), then the behavior of instance deserialization is undefined in cases where the ObjectInputStream cannot resolve the class which defined the writeObject method in question. If you go to section 3.4 [2] of the specification, it reads: The readObject method of the class, if implemented, is responsible for restoring the state of the class. The values of every field of the object whether transient or not, static or not are set to the default value for the fields type. Either ObjectInputStream's defaultReadObject or readFields method must be called once (and only once) before reading any optional data written by the corresponding writeObject method; even if no optional data is read, defaultReadObject or readFields must still be invoked once. Now the problem: there are many classes in the wild which nevertheless do not write/read fields. We cause an exception in such cases rather than make up some undefined behavior. What I'm wondering is, is there some sensible behavior that could be specified for this case? The Oracle JDK seems to simply leave fields uninitialized in this case, maybe that can be a specified behavior? [1] http://docs.oracle.com/javase/7/docs/platform/serialization/spec/output.html#861 [2] http://docs.oracle.com/javase/7/docs/platform/serialization/spec/input.html#2971
Re: Time to remove sun.misc.Service?
On 02/12/2014 04:47 AM, Alan Bateman wrote: On 11/02/2014 11:44, Paul Sandoz wrote: : Scrub it! AFAICT it is not widely used (looking at the use of s.m.Service static methods on grep code there are only a handful of dependent artifacts). And the upgrade is quite easy. I didn't see any more comments on this and I agree there isn't much of a case for keeping it around. Here's the webrev to remove it: http://cr.openjdk.java.net/~alanb/8034776/webrev/ -Alan. sun.misc.Service be gone! -Joe
Re: A hole in the serialization spec
That's a quote from the serialization spec. I take it to mean, Don't write fields and everything might go to hell. In practice, if the reading side doesn't read fields, things end up more or less OK, as evidenced by various classes in the wild. But it's not hard to imagine a scenario in which a class change could cause protocol corruption. I think the specifics of the quote relate to this kind of class change; in particular, if a class is deleted from the hierarchy on the read side, and that class corresponds to the class that had the misbehaving writeObject, I suspect that things will break at that point as the read side will probably try to consume and discard the field information for that class, which will be missing (it will start reading the next class' fields instead I think). On 02/12/2014 09:08 AM, Chris Hegarty wrote: David, others? I'm afraid I'm still not clear what is mean by: ... undefined in cases where the ObjectInputStream cannot resolve the class which defined the writeObject method in question. This does not seem directly related to the issue we are discussing ( whether to invoke default read/write object ). -Chris. On 10/02/14 15:37, David M. Lloyd wrote: I agree that it's a problem; however it's also clear that there are many classes in the wild which have this problem. It would be nice if the behavior could _become_ defined *somehow* though. I can see at least four options: 1) do nothing :( 2) start throwing (or writing) an exception in write/readObject when stream ops are performed without reading fields (maybe can be disabled with a sys prop or something) 3) leave fields cleared and risk protocol issues 4) silently start reading/writing empty field information (risks protocol issues) Maybe there are better options I'm not thinking of. On 02/10/2014 08:53 AM, Chris Hegarty wrote: David, ... undefined in cases where the ObjectInputStream cannot resolve the class which defined the writeObject method in question. I'm not clear as to what this statement is about? I'm sure you already know this, and maybe in your environment do not care much about it, but having a read/writeObject not invoke the appropriate default read/write Object/Fields method is a serious impediment to evolving the serial form ( in a compatible way ). For example, if your class has no serializable fields in one revision, but adds a serializable field(s) in a subsequent revision. This could lead to a StreamCorruptedException, or some other undefined behavior. The OpenJDK sources do seem to be quite tolerant of this situation. I'm not entirely sure if this is a good or a bad thing. That said, I don't think we want to encourage this kind of behavior. -Chris. On 07/02/14 15:07, David M. Lloyd wrote: Since the topic of serialization has come up recently, I'll take it as an excuse to bring up a problem that I've run into a couple of times with the serialization specification, which has resulted in user problems. If you read section 2.3 [1] of the specification, it says: The class's writeObject method, if implemented, is responsible for saving the state of the class. Either ObjectOutputStream's defaultWriteObject or writeFields method must be called once (and only once) before writing any optional data that will be needed by the corresponding readObject method to restore the state of the object; even if no optional data is written, defaultWriteObject or writeFields must still be invoked once. If defaultWriteObject or writeFields is not invoked once prior to the writing of optional data (if any), then the behavior of instance deserialization is undefined in cases where the ObjectInputStream cannot resolve the class which defined the writeObject method in question. If you go to section 3.4 [2] of the specification, it reads: The readObject method of the class, if implemented, is responsible for restoring the state of the class. The values of every field of the object whether transient or not, static or not are set to the default value for the fields type. Either ObjectInputStream's defaultReadObject or readFields method must be called once (and only once) before reading any optional data written by the corresponding writeObject method; even if no optional data is read, defaultReadObject or readFields must still be invoked once. Now the problem: there are many classes in the wild which nevertheless do not write/read fields. We cause an exception in such cases rather than make up some undefined behavior. What I'm wondering is, is there some sensible behavior that could be specified for this case? The Oracle JDK seems to simply leave fields uninitialized in this case, maybe that can be a specified behavior? [1] http://docs.oracle.com/javase/7/docs/platform/serialization/spec/output.html#861 [2] http://docs.oracle.com/javase/7/docs/platform/serialization/spec/input.html#2971 -- - DML
Re: Time to remove sun.misc.Service?
On 2/12/2014 4:47 AM, Alan Bateman wrote: On 11/02/2014 11:44, Paul Sandoz wrote: : Scrub it! AFAICT it is not widely used (looking at the use of s.m.Service static methods on grep code there are only a handful of dependent artifacts). And the upgrade is quite easy. I didn't see any more comments on this and I agree there isn't much of a case for keeping it around. Here's the webrev to remove it: http://cr.openjdk.java.net/~alanb/8034776/webrev/ Thumbs up. Mandy
Re: methods that throws checked exceptions in a lambda block
On 02/12/2014 07:05 AM, Wang Weijun wrote: This line does not compile Files.list(Paths.get(/secret)).forEach(x - Files.write(x, new byte[Files.size(x)]); because Files.write() and Files.size() throw IOE. Then what's the best way to make it work? (I find some old threads on this on lambda-dev. Maybe we have a recommended way now?) I am not sure there is a recommended way. Depends on how you need to handle those exceptions, I'll put the lambda in a private function to take care those exception, and surpress them in a UncheckedIOException. Cheers, Henry
Re: methods that throws checked exceptions in a lambda block
On Feb 12, 2014, at 4:05 PM, Wang Weijun weijun.w...@oracle.com wrote:
This line does not compile
Files.list(Paths.get(/secret)).forEach(x - Files.write(x, new
byte[Files.size(x)]);
because Files.write() and Files.size() throw IOE. Then what's the best way to
make it work?
It depends...
Sometimes i will shuffle things under a method (static one if there are no
captures and use a method ref).
If i get *really* fed up i have been know to write stuff like the following:
private static interface ST, E extends IOException {
T apply() throws E;
}
private static T, E extends IOException T r(ST, E s) {
try {
return s.apply();
}
catch (IOException t) {
throw new UncheckedIOException(t);
}
}
Paul.
(I find some old threads on this on lambda-dev. Maybe we have a recommended
way now?)
Re: methods that throws checked exceptions in a lambda block
On 12/02/2014 16:36, Henry Jen wrote: I am not sure there is a recommended way. Depends on how you need to handle those exceptions, I'll put the lambda in a private function to take care those exception, and surpress them in a UncheckedIOException. Yes, I think it depends on whether he wants to recover and continue to attempt to zero the remaining files or not. -Alan
Review request for JDK-8030010: cleanup native code warnings
This patch cleans up a few trivial native warnings (mainly remove local unreferenced variable) https://bugs.openjdk.java.net/browse/JDK-8030010 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8030010/webrev.00/ This patch was contributed by Francis Andre [1] and I fixed a couple other trivial ones. Mandy [1] http://mail.openjdk.java.net/pipermail/build-dev/2013-December/011332.html
Re: Review request for JDK-8030010: cleanup native code warnings
Hi Mandy This looks OK to me On Feb 12, 2014, at 12:46 PM, Mandy Chung wrote: This patch cleans up a few trivial native warnings (mainly remove local unreferenced variable) https://bugs.openjdk.java.net/browse/JDK-8030010 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8030010/webrev.00/ This patch was contributed by Francis Andre [1] and I fixed a couple other trivial ones. Mandy [1] http://mail.openjdk.java.net/pipermail/build-dev/2013-December/011332.html Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR: (8031737) CHECK_NULL and CHECK_EXCEPTION macros cleanup
Hi Phil, The changes look fine. (Not a Reviewer). Thanks for doing the backport, Roger On 2/11/2014 6:42 PM, Phil Race wrote: Here's a JDk8u webrev : -http://cr.openjdk.java.net/~prr/8031737.8u/ -phil. On 2/11/14 2:28 PM, Phil Race wrote: So since hg export/import doesn't apply cleanly and the dependency chain seems, long and in order to have some consistency across the releases, I think I should prepare a webrev which essentially backports 8031737 including its small changes to Version.c, if only because otherwise I'd have to have a new bug ID that would not be forwarded ported (one source of confusion) or even worse re-use 8031737 but not fully implement it Agreed ? -phil. On 2/11/2014 2:20 PM, roger riggs wrote: Hi Phil, On 2/11/2014 5:09 PM, Phil Race wrote: Are we talking about the same changesets ? a09982d91fab/8030993 has no change to the macros right (I didn't think this was topic of this conversation) fb89dc4fe8da/8031737 is the one that reimplemented the macros and is the version I'd want. Its the last 'edit' of those macros in that file. yes, c58c6b0fbe34/8030875 is the original addition of these :- Yes. Roger ... changeset: 9229:fb89dc4fe8da user:rriggs date:Mon Feb 03 16:58:02 2014 -0500 summary: 8031737: CHECK_NULL and CHECK_EXCEPTION macros cleanup changeset: 9051:c58c6b0fbe34 user:rriggs date:Fri Jan 10 10:45:56 2014 -0500 summary: 8030875: Macros for checking and returning on exceptions ... -phil. On 2/11/14 1:48 PM, roger riggs wrote: Hi Phil, The later changeset picked up the recommended style of implementing the macros but I don't think it was substantive. You can probably do without it. Version.c had some changes in a different changeset to address the omission of checking for exceptions after some JNI calls. Roger On 2/11/2014 4:39 PM, Phil Race wrote: Roger, That later one seems to be using the macros. I don't see any update to the macros. So I'm not sure why I'm need it .. since I'm not using those calls and neither are the macros. -phil. On 2/11/14 12:28 PM, roger riggs wrote: Hi Phil, Yes, it ended up in two change sets in jdk 9, you should take both to be up to date. changeset: 9245:a09982d91fab user:rriggs date:Wed Feb 05 10:59:53 2014 -0500 files: src/share/native/common/jni_util.c description: 8030993: Check jdk/src/share/native/common/jni_util.c for JNI pending exceptions changeset: 9229:fb89dc4fe8da date:Mon Feb 03 16:58:02 2014 -0500 files: src/share/native/common/jni_util.h src/share/native/sun/misc/Version.c interrupted! description: 8031737: CHECK_NULL and CHECK_EXCEPTION macros cleanup Thanks, Roger On 2/11/2014 2:57 PM, Phil Race wrote: Roger, Yes, I can do that. I see here http://cr.openjdk.java.net/~rriggs/webrev-check-cleanup-8031737/ that 1) There was a previous version of these macros. Looks like no need to worry about that I just need the latest version. 2) There was also a change to Version.c. I can include that if you think it appropriate .. or omit it if you think its not essential. -phil. On 2/11/2014 11:14 AM, roger riggs wrote: Hi Phil, I see your point, there is nothing in the changes unique to 9. Do you want to take care of the back point? Roger On 2/11/2014 2:04 PM, Phil Race wrote: Roger, Why not JDK 8u ? I've got a lot of changes that utilise these that will backport cleanly to JDK 8u only if 8u includes these macros. And since the changes are all over the place I don't fancy copy/pasting them everywhere. I suspect I am not the only one who would like these in 8u .. -phil. On 02/03/2014 01:48 PM, roger riggs wrote: Hi Lance, The convenience macros are only intended for JDK 9. Roger On 2/1/2014 1:58 PM, Lance @ Oracle wrote: Looks fine Which releases are you think of including this in if any besides 9? http://oracle.com/us/design/oracle-email-sig-198324.gifLance Andersen| Principal Member of Technical Staff | +1.781.442.2037 tel:+1.781.442.2037 Oracle Java Engineering 1 Network Drive x-apple-data-detectors://34/0 Burlington, MA 01803 x-apple-data-detectors://34/0 lance.ander...@oracle.com mailto:lance.ander...@oracle.com Sent from my iPad On Feb 1, 2014, at 1:03 PM, roger riggs roger.ri...@oracle.com mailto:roger.ri...@oracle.com wrote:
Re: Review request for JDK-8030010: cleanup native code warnings
On 12/02/2014 17:46, Mandy Chung wrote: This patch cleans up a few trivial native warnings (mainly remove local unreferenced variable) https://bugs.openjdk.java.net/browse/JDK-8030010 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8030010/webrev.00/ This patch was contributed by Francis Andre [1] and I fixed a couple other trivial ones. This looks good and nice to see the native code warnings getting a bit attention. -Alan.
Re: RFR: (8031737) CHECK_NULL and CHECK_EXCEPTION macros cleanup
This looks fine. On Feb 11 2014, at 15:42 , Phil Race philip.r...@oracle.com wrote: Here's a JDk8u webrev : -http://cr.openjdk.java.net/~prr/8031737.8u/ -phil. On 2/11/14 2:28 PM, Phil Race wrote: So since hg export/import doesn't apply cleanly and the dependency chain seems, long and in order to have some consistency across the releases, I think I should prepare a webrev which essentially backports 8031737 including its small changes to Version.c, if only because otherwise I'd have to have a new bug ID that would not be forwarded ported (one source of confusion) or even worse re-use 8031737 but not fully implement it Agreed ? -phil. On 2/11/2014 2:20 PM, roger riggs wrote: Hi Phil, On 2/11/2014 5:09 PM, Phil Race wrote: Are we talking about the same changesets ? a09982d91fab/8030993 has no change to the macros right (I didn't think this was topic of this conversation) fb89dc4fe8da/8031737 is the one that reimplemented the macros and is the version I'd want. Its the last 'edit' of those macros in that file. yes, c58c6b0fbe34/8030875 is the original addition of these :- Yes. Roger ... changeset: 9229:fb89dc4fe8da user:rriggs date:Mon Feb 03 16:58:02 2014 -0500 summary: 8031737: CHECK_NULL and CHECK_EXCEPTION macros cleanup changeset: 9051:c58c6b0fbe34 user:rriggs date:Fri Jan 10 10:45:56 2014 -0500 summary: 8030875: Macros for checking and returning on exceptions ... -phil. On 2/11/14 1:48 PM, roger riggs wrote: Hi Phil, The later changeset picked up the recommended style of implementing the macros but I don't think it was substantive. You can probably do without it. Version.c had some changes in a different changeset to address the omission of checking for exceptions after some JNI calls. Roger On 2/11/2014 4:39 PM, Phil Race wrote: Roger, That later one seems to be using the macros. I don't see any update to the macros. So I'm not sure why I'm need it .. since I'm not using those calls and neither are the macros. -phil. On 2/11/14 12:28 PM, roger riggs wrote: Hi Phil, Yes, it ended up in two change sets in jdk 9, you should take both to be up to date. changeset: 9245:a09982d91fab user:rriggs date:Wed Feb 05 10:59:53 2014 -0500 files: src/share/native/common/jni_util.c description: 8030993: Check jdk/src/share/native/common/jni_util.c for JNI pending exceptions changeset: 9229:fb89dc4fe8da date:Mon Feb 03 16:58:02 2014 -0500 files: src/share/native/common/jni_util.h src/share/native/sun/misc/Version.c interrupted! description: 8031737: CHECK_NULL and CHECK_EXCEPTION macros cleanup Thanks, Roger On 2/11/2014 2:57 PM, Phil Race wrote: Roger, Yes, I can do that. I see here http://cr.openjdk.java.net/~rriggs/webrev-check-cleanup-8031737/ that 1) There was a previous version of these macros. Looks like no need to worry about that I just need the latest version. 2) There was also a change to Version.c. I can include that if you think it appropriate .. or omit it if you think its not essential. -phil. On 2/11/2014 11:14 AM, roger riggs wrote: Hi Phil, I see your point, there is nothing in the changes unique to 9. Do you want to take care of the back point? Roger On 2/11/2014 2:04 PM, Phil Race wrote: Roger, Why not JDK 8u ? I've got a lot of changes that utilise these that will backport cleanly to JDK 8u only if 8u includes these macros. And since the changes are all over the place I don't fancy copy/pasting them everywhere. I suspect I am not the only one who would like these in 8u .. -phil. On 02/03/2014 01:48 PM, roger riggs wrote: Hi Lance, The convenience macros are only intended for JDK 9. Roger On 2/1/2014 1:58 PM, Lance @ Oracle wrote: Looks fine Which releases are you think of including this in if any besides 9? http://oracle.com/us/design/oracle-email-sig-198324.gifLance Andersen| Principal Member of Technical Staff | +1.781.442.2037 tel:+1.781.442.2037 Oracle Java Engineering 1 Network Drive x-apple-data-detectors://34/0 Burlington, MA 01803 x-apple-data-detectors://34/0 lance.ander...@oracle.com mailto:lance.ander...@oracle.com Sent from my iPad On Feb 1, 2014, at 1:03 PM, roger riggs roger.ri...@oracle.com mailto:roger.ri...@oracle.com wrote:
Re: RFR(S): 8034087: XML parser may overwrite element content if that content falls onto the border of an entity scanner buffer
Ok, understand.
Thanks,
Joe
On 2/11/2014 11:40 PM, Schreiber, Steffen wrote:
Hi Joe,
Yes, that's certainly true, if the test is only to show the issue.
We intended the test to work as a regression test as well and wanted to check the
effects of the buffer boundaries on more possible places, i.e. before/at/after
special characters like , or / as well as inside tags and content.
We can easily adapt the loop, if runtime is a concern here.
Regards,
Steffen
-Original Message-
From: huizhe wang [mailto:huizhe.w...@oracle.com]
Sent: Dienstag, 11. Februar 2014 22:57
To: Volker Simonis
Cc: Alan Bateman; Schreiber, Steffen; Java Core Libs
Subject: Re: RFR(S): 8034087: XML parser may overwrite element content if that
content falls onto the border of an entity scanner buffer
Hi Volker,
I agree with the approach below and jdk9/dev is the better forest.
For the test itself, I would suggest reducing the following loop to 1 or
2 cases:
for (int i = 0; i testString.length(); i++) {
test(createDocument(testString.toString(), i), + i);
}
when i=7, the problem starts to show. It's sufficient to demonstrate the
issue then by just entering 7. It's unnecessary to run the test 43 times.
Thanks,
Joe
On 2/11/2014 9:00 AM, Volker Simonis wrote:
Hi Alan,
you're right. I initially didn't saw the test because I just looked at
the change in the jaxp repository.
If it will be approved, I'll put the test in the same directory like
the other test (i.e. test/javax/xml/jaxp/parsers/8027359).
And yes, my plan was to get approval for both, the tests and the fix,
when asking for the permission to downport to jdk8u-dev and jdk7u-dev.
Thanks,
Volker
On Tue, Feb 11, 2014 at 5:44 PM, Alan Bateman alan.bate...@oracle.com wrote:
On 11/02/2014 14:57, Volker Simonis wrote:
Hi,
after opening this bug yesterday for an issue found by my colleague
Steffen Schreiber we realized that this is actually a duplicate of
8027359: XML parser returns incorrect parsing results
(https://bugs.openjdk.java.net/browse/JDK-8027359).
While there's no need now to submit a patch anymore, we'd
nevertheless like to contribute at least our test case for this issue:
http://cr.openjdk.java.net/~simonis/webrevs/8034087/
The webrev is against jdk9-client but we'd like to also downport this
test to jdk7 and jdk8 to track that the fix for 8027359 will be
correctly downported to these releases as well.
I will sponsor this change if somebody would be so kind to review it.
I'll leave it to Joe Wang to comment on the test but just to mention that
jdk9/dev is probably a better forest to aim for because that is where the
XML (and its tests) usually go. Also I wonder if it might be better to put
it in the same directory as the test that Joe pushed with the change?
If you are getting approval to push to jdk8u-dev and jdk7u-dev then it might
be better to request a backport of Joe's change at the same time.
-Alan.
Re: RFR: (8031737) CHECK_NULL and CHECK_EXCEPTION macros cleanup
This looks good to me too. Mandy On 2/12/2014 10:10 AM, roger riggs wrote: Hi Phil, The changes look fine. (Not a Reviewer). Thanks for doing the backport, Roger On 2/11/2014 6:42 PM, Phil Race wrote: Here's a JDk8u webrev : -http://cr.openjdk.java.net/~prr/8031737.8u/ -phil. On 2/11/14 2:28 PM, Phil Race wrote: So since hg export/import doesn't apply cleanly and the dependency chain seems, long and in order to have some consistency across the releases, I think I should prepare a webrev which essentially backports 8031737 including its small changes to Version.c, if only because otherwise I'd have to have a new bug ID that would not be forwarded ported (one source of confusion) or even worse re-use 8031737 but not fully implement it Agreed ? -phil. On 2/11/2014 2:20 PM, roger riggs wrote: Hi Phil, On 2/11/2014 5:09 PM, Phil Race wrote: Are we talking about the same changesets ? a09982d91fab/8030993 has no change to the macros right (I didn't think this was topic of this conversation) fb89dc4fe8da/8031737 is the one that reimplemented the macros and is the version I'd want. Its the last 'edit' of those macros in that file. yes, c58c6b0fbe34/8030875 is the original addition of these :- Yes. Roger ... changeset: 9229:fb89dc4fe8da user:rriggs date:Mon Feb 03 16:58:02 2014 -0500 summary: 8031737: CHECK_NULL and CHECK_EXCEPTION macros cleanup changeset: 9051:c58c6b0fbe34 user:rriggs date:Fri Jan 10 10:45:56 2014 -0500 summary: 8030875: Macros for checking and returning on exceptions ... -phil. On 2/11/14 1:48 PM, roger riggs wrote: Hi Phil, The later changeset picked up the recommended style of implementing the macros but I don't think it was substantive. You can probably do without it. Version.c had some changes in a different changeset to address the omission of checking for exceptions after some JNI calls. Roger On 2/11/2014 4:39 PM, Phil Race wrote: Roger, That later one seems to be using the macros. I don't see any update to the macros. So I'm not sure why I'm need it .. since I'm not using those calls and neither are the macros. -phil. On 2/11/14 12:28 PM, roger riggs wrote: Hi Phil, Yes, it ended up in two change sets in jdk 9, you should take both to be up to date. changeset: 9245:a09982d91fab user:rriggs date:Wed Feb 05 10:59:53 2014 -0500 files: src/share/native/common/jni_util.c description: 8030993: Check jdk/src/share/native/common/jni_util.c for JNI pending exceptions changeset: 9229:fb89dc4fe8da date:Mon Feb 03 16:58:02 2014 -0500 files: src/share/native/common/jni_util.h src/share/native/sun/misc/Version.c interrupted! description: 8031737: CHECK_NULL and CHECK_EXCEPTION macros cleanup Thanks, Roger On 2/11/2014 2:57 PM, Phil Race wrote: Roger, Yes, I can do that. I see here http://cr.openjdk.java.net/~rriggs/webrev-check-cleanup-8031737/ that 1) There was a previous version of these macros. Looks like no need to worry about that I just need the latest version. 2) There was also a change to Version.c. I can include that if you think it appropriate .. or omit it if you think its not essential. -phil. On 2/11/2014 11:14 AM, roger riggs wrote: Hi Phil, I see your point, there is nothing in the changes unique to 9. Do you want to take care of the back point? Roger On 2/11/2014 2:04 PM, Phil Race wrote: Roger, Why not JDK 8u ? I've got a lot of changes that utilise these that will backport cleanly to JDK 8u only if 8u includes these macros. And since the changes are all over the place I don't fancy copy/pasting them everywhere. I suspect I am not the only one who would like these in 8u .. -phil. On 02/03/2014 01:48 PM, roger riggs wrote: Hi Lance, The convenience macros are only intended for JDK 9. Roger On 2/1/2014 1:58 PM, Lance @ Oracle wrote: Looks fine Which releases are you think of including this in if any besides 9? http://oracle.com/us/design/oracle-email-sig-198324.gifLance Andersen| Principal Member of Technical Staff | +1.781.442.2037 tel:+1.781.442.2037 Oracle Java Engineering 1 Network Drive x-apple-data-detectors://34/0 Burlington, MA 01803 x-apple-data-detectors://34/0 lance.ander...@oracle.com mailto:lance.ander...@oracle.com Sent from my iPad On Feb 1, 2014, at 1:03 PM, roger riggs roger.ri...@oracle.com mailto:roger.ri...@oracle.com wrote:
Re: RFR for JDK-8028711: TEST_BUG: Tests should pass through VM options: corelibs tests
On 12/02/2014 12:59, michael cui wrote: On 02/10/2014 05:36 PM, Alan Bateman wrote: A minor comment on test/java/net/URLPermission/nstest/lookup.sh is that the resulting line length is 182 characters and this will likely be annoying for future side-by-side views. So I think I'd split this while you are there. Please review the updated version at http://cr.openjdk.java.net/~tyan/michael/JDK-8028711/webrev.04/ http://cr.openjdk.java.net/%7Etyan/michael/JDK-8028711/webrev.04/ Changes includes : 1. split line if it longer than 80 characters. 2. merge the fix of JDK-8033897 https://bugs.openjdk.java.net/browse/JDK-8033897 3. add few missed scripts. If no further changes need to be made, I would like to find sponsor to push this fix. The changes look okay to me. I see you decided to ignore the javac usages but that is okay and can be done another time. Also thanks for fixing lookup.sh. I don't personally mind lines 80 but that one was 180 which make it difficult to look at side-by-side changes. -Alan.
Re: RFR (JAXP): 8033980 : Xerces Update: datatype XMLGregorianCalendarImpl and DurationImpl
On 12/02/2014 04:26, huizhe wang wrote: Hi Lance, Daniel, I suggest we take this incompatibility and document it in the release notes (we'll likely have more). I reversed DurationImpl and then realized why the Xerces engineers made the incompatibility change when I started on XMLGregorianCalendarImpl. It was because XMLGregorianCalendarImpl did not implement what was recommended, using the lexical form for serialization. In the original implementation therefore, the impl between XMLGregorianCalendarImpl and DurationImpl was not consistent. Keeping this Xerces revision makes the serialization consistent in both classes and adds the benefit of having the same source as that of Xerces. I think we might need to hold the horses and understand this one and compatibility impact a bit more. As I understand, XMLGregorianCalendar is not Serializable but the Xerces implementation (XMLGregorianCalendarImpl) is. If someone is using the JDK and hasn't configured an alternative DatatypeFactory then it's an instance of com.sun.org.apache.xerces.internal.jaxp.datatype.XMLGregorianCalendarImpl that is serialized today. Do I have this right? I assume there is no serialization interop between this and someone using vanilla Xerces as the other end will be dealing with org.apache classes. With the proposed change then you've added a writeReplace so that going forward it will write a com.sun.org.apache.xerces.internal.jaxp.datatype.SerializedDuration into the stream. This isn't going to work if you've got a JDK 8 or older on the other end, right? Also I assume this doesn't help the Xerces interop because the fully qualified class names are still different. XMLGregorianCalendarImpl doesn't appear to have a readObject or other serialization methods so I assume that if you don't change the serialVersionUID then it would at least be possible to deserialize something that was serialized by an older JDK. As you've added a writeReplace then it makes me wonder why the serialVersionUID of XMLGregorianCalendarImpl has changed. Maybe that part could be reversed and leave the long standing value? One thing that might help is to develop a number serialization tests to help better understand the impact of the change. -Alan.
Re: RFR for JDK-8030844:sun/rmi/rmic/classpath/RMICClassPathTest.java timeout in same binaries run
Hi Tristan, Changes look good. Unfortunately the webrev doesn't contain an actual changeset; it just contains a patch. In the webrev header there is the line Patch of Changes. Instead it should say Changeset. Like this one: http://cr.openjdk.java.net/~smarks/reviews/7900311/webrev.1/ Unfortunately webrev doesn't always produce an actual changeset, in particular it doesn't if you use webrev -r. (My example webrev above is a patch that changes webrev to generate the changeset even with -r. It hasn't been pushed yet; possibly it will later today. Or you can apply my webrev changeset to your local webrev.) Or, you can just run webrev without -r and it should check hg outgoing to determine the changesets used in generating the webrev, which does place the changeset into the webrev. Can you regenerate the webrev so that it includes the actual changeset? Sorry, this is a small thing, but as someone who is sponsoring changesets, I'd appreciate an actual changeset in the webrev instead of having to construct one manually. I think other sponsors would appreciate this too. s'marks On 2/11/14 6:59 PM, Tristan Yan wrote: Thank you Stuart http://cr.openjdk.java.net/~tyan/JDK-8030844/webrev.01/ Regards Tristan On Feb 12, 2014, at 10:06 AM, Stuart Marks stuart.ma...@oracle.com mailto:stuart.ma...@oracle.com wrote: Hi, yes, I'll take this one. It's slightly odd that this is creating filenames that already have / in them (as opposed to File.separator) but since these files don't actually have to exist, I suppose it doesn't really matter. I'm not convinced that we actually have any evidence that /home/~user is really causing the hang/timeout (either caused by the automounter hanging on /home or LDAP or other nameservice lookup on ~user), but this is harmless, and it'll fix the problem on the off chance that this really is the root cause. Tristan, please update the test's @bug tag to add 8030844, create a changeset, and create a webrev with the changeset in it (as opposed to a bare patch). I'll then push it for you. s'marks On 2/10/14 4:08 AM, Alan Bateman wrote: On 10/02/2014 10:57, Tristan Yan wrote: Ping: Can anyone give a review on this. Thank you Tristan Changing the test so that it doesn't try to /home/~user seems reasonable to me. Stuart - I see you've been sponsoring Tristan's updates to the RMI tests. Are you going to take this one too? -Alan On Feb 6, 2014, at 5:13 PM, Tristan Yantristan@oracle.com mailto:tristan@oracle.com wrote: Hi All Please help to review a simple fix for https://bugs.openjdk.java.net/browse/JDK-8030844 http://cr.openjdk.java.net/~tyan/JDK-8030844/webrev.00/. Description: Change replace a “/home/~user folder with an test source path. Folder “/home/~user” cause some problem whenever something wrong with the automount filesystem or an username lookup problem. Thank you Tristan
Re: RFR: JDK-8032050: TEST_BUG: java/rmi/activation/Activatable/shutdownGracefully/ShutdownGracefully.java fails intermittently
Hi Tristan, JavaVM.waitFor looks mostly fine. The indentation of the start of the waitFor(timeout) javadoc comment is a bit off, though; please fix. There are still some adjustments that need to be made in ShutdownGracefully.java. Both have to do with the case where the last registration succeeds unexpectedly -- this is the one that we expect to fail. First, if this registration has succeeded unexpectedly, that means rmid is still running. If that occurs, the call to rmid.waitFor(timeout) will inevitably time out. It may be worth calling rmid.destroy() directly at this point. Second, still in the succeeded-unexpectedly case, at line 154 TestLibrary.bomb() is called. This throws an exception, but it's caught by the catch-block at lines 157-158, which calls TestLibrary.bomb() again, saying unexpected exception. Except that this is kind of expected, since it was thrown from an earlier call to TestLibrary.bomb(). This is quite confusing. There are several cases that need to be handled. 1. Normal case. Registration fails as expected, rmid has terminated gracefully. Test passes. 2. Rmid is still running and has processed the registration request successfully. Need to kill rmid and fail the test. 3. Rmid is still running but is in some bad state where the registration request failed. Need to kill rmid and fail the test. 4. Some other unexpected failure. This is what the catch and finally blocks at lines 157-161 are for. These four cases need to be handled independently. Ideally they should be separated from the cleanup code. As noted above, you don't want to throw an exception from the try-block, because it will get caught by your own catch block. Similarly, it's tempting to return from the midst of the try-block in the success case, but this still runs the finally-block. This can be quite confusing. A typical technique for dealing with this kind of issue is to record results of operations from within the try block, and then analyze the results outside, throwing a test failure (TestLibrary.bomb) at that point, where it won't be caught by the test's own catch block. Editoral: - line 153, there should be a space between 'if' and the opening paren - line 156, typo, gracefuuly Finally, it would be helpful if you could get webrev to generate the actual changeset instead of the plain patch, per my other review email. Thanks, s'marks On 2/11/14 9:39 PM, Tristan Yan wrote: Thank you for your thorough mail. This is very educational. I took you advice and generated a new webrev for this. http://cr.openjdk.java.net/~tyan/JDK-8032050/webrev.03/ I appreciate you can review this again. Regards Tristan On Feb 11, 2014, at 8:32 AM, Stuart Marks stuart.ma...@oracle.com mailto:stuart.ma...@oracle.com wrote: Hi Tristan, Sorry about my recurring delays. Several comments on these changes. JavaVM.java -- The waitFor(timeout) method is mostly ok. The new thread started at line 208 and following seems unnecessary, though. This code is reached when a timeout occurs, so throwing TimeoutException is the only thing necessary in this case. Should the code to start the new thread be removed? There should be a similar check for vm == null as in the waitFor() [no args] method. ShutdownGracefully.java -- The condition that's checked after calling rmid.waitFor(SHUTDOWN_TIMEOUT) is incorrect. It's testing the exit status against zero. Offhand, when and if rmid exits, it might exit with a nonzero exit status. If rmid has exited at this point, then the test should succeed. Instead of testing against zero, the code should catch TimeoutException, which means that rmid is still running. It's probably reasonable to catch TimeoutException, print a message, and then let the finally-block destroy the rmid. Calling TestLibrary.bomb() from within the try-block is confusing, since that method throws an exception, which is then caught by the catch-block, when then calls TestLibrary.bomb() again. We should also make sure to test the success case properly. If rmid.waitFor() returns in a timely fashion without throwing TimeoutException, it doesn't matter what the exit status is. (It might be worth printing it out.) At that point we know that rmid *has* exited gracefully, so we need to set rmid to null so that the finally-block doesn't attempt to destroy rmid redundantly. Some additional messages about rmid having exited and the test passing are also warranted for this case. Some additional cleanup can be done here as well, over and above the changes you've proposed. (This stuff is left over from earlier RMI test messes.) In order to shut down an active object, the code here spawns a new thread that sleeps for a while and then deactivates this object. This isn't necessary. (It might have been necessary in the past.) It's sufficient simply to unexport this object and then deactivate it, directly within the shutdown() method. See
Re: methods that throws checked exceptions in a lambda block
On 02/12/2014 05:36 PM, Henry Jen wrote: On 02/12/2014 07:05 AM, Wang Weijun wrote: This line does not compile Files.list(Paths.get(/secret)).forEach(x - Files.write(x, new byte[Files.size(x)]); because Files.write() and Files.size() throw IOE. Then what's the best way to make it work? (I find some old threads on this on lambda-dev. Maybe we have a recommended way now?) I am not sure there is a recommended way. Depends on how you need to handle those exceptions, I'll put the lambda in a private function to take care those exception, and surpress them in a UncheckedIOException. Cheers, Henry The other solution is to use wrappers like these ones: https://github.com/forax/blog/blob/master/src/com/github/forax/blog/UnsafeIO.java and rewrite your code like this: Files.list(Paths.get(/secret)).forEach(unsafeProc(x - Files.write(x, new byte[Files.size(x)])); cheers, Rémi
Re: RFR for JDK-8028711: TEST_BUG: Tests should pass through VM options: corelibs tests
On 02/13/2014 02:35 AM, Alan Bateman wrote: The changes look okay to me. I see you decided to ignore the javac usages but that is okay and can be done another time. Also thanks for fixing lookup.sh. I don't personally mind lines 80 but that one was 180 which make it difficult to look at side-by-side changes. Hi Alan, Thanks so much for reviewing this! Would you mind to sponsor this for me? Regrads, Michael Cui
Re: RFR for JDK-8030844:sun/rmi/rmic/classpath/RMICClassPathTest.java timeout in same binaries run
Thank you Stuart This is a very nice tutorial. I did try both ways. Adopt your fix doesn't seem work for me. It still doesn't generate changeset. But without -r option works. http://cr.openjdk.java.net/~tyan/JDK-8030844/webrev.02/ Could you push it for me? Tristan On 02/13/2014 03:48 AM, Stuart Marks wrote: Hi Tristan, Changes look good. Unfortunately the webrev doesn't contain an actual changeset; it just contains a patch. In the webrev header there is the line Patch of Changes. Instead it should say Changeset. Like this one: http://cr.openjdk.java.net/~smarks/reviews/7900311/webrev.1/ Unfortunately webrev doesn't always produce an actual changeset, in particular it doesn't if you use webrev -r. (My example webrev above is a patch that changes webrev to generate the changeset even with -r. It hasn't been pushed yet; possibly it will later today. Or you can apply my webrev changeset to your local webrev.) Or, you can just run webrev without -r and it should check hg outgoing to determine the changesets used in generating the webrev, which does place the changeset into the webrev. Can you regenerate the webrev so that it includes the actual changeset? Sorry, this is a small thing, but as someone who is sponsoring changesets, I'd appreciate an actual changeset in the webrev instead of having to construct one manually. I think other sponsors would appreciate this too. s'marks On 2/11/14 6:59 PM, Tristan Yan wrote: Thank you Stuart http://cr.openjdk.java.net/~tyan/JDK-8030844/webrev.01/ Regards Tristan On Feb 12, 2014, at 10:06 AM, Stuart Marks stuart.ma...@oracle.com mailto:stuart.ma...@oracle.com wrote: Hi, yes, I'll take this one. It's slightly odd that this is creating filenames that already have / in them (as opposed to File.separator) but since these files don't actually have to exist, I suppose it doesn't really matter. I'm not convinced that we actually have any evidence that /home/~user is really causing the hang/timeout (either caused by the automounter hanging on /home or LDAP or other nameservice lookup on ~user), but this is harmless, and it'll fix the problem on the off chance that this really is the root cause. Tristan, please update the test's @bug tag to add 8030844, create a changeset, and create a webrev with the changeset in it (as opposed to a bare patch). I'll then push it for you. s'marks On 2/10/14 4:08 AM, Alan Bateman wrote: On 10/02/2014 10:57, Tristan Yan wrote: Ping: Can anyone give a review on this. Thank you Tristan Changing the test so that it doesn't try to /home/~user seems reasonable to me. Stuart - I see you've been sponsoring Tristan's updates to the RMI tests. Are you going to take this one too? -Alan On Feb 6, 2014, at 5:13 PM, Tristan Yantristan@oracle.com mailto:tristan@oracle.com wrote: Hi All Please help to review a simple fix for https://bugs.openjdk.java.net/browse/JDK-8030844 http://cr.openjdk.java.net/~tyan/JDK-8030844/webrev.00/. Description: Change replace a “/home/~user folder with an test source path. Folder “/home/~user” cause some problem whenever something wrong with the automount filesystem or an username lookup problem. Thank you Tristan
RFR (S): JDK-8034000 lack of /othervm option can cause some RMI tests to fail
Hi all, The RMI test directories were removed from TEST.ROOT's othervm.dirs by JDK-8031179 so that individual RMI tests could opt-in to othervm themselves. Unfortunately, some tests needed othervm but didn't get it, causing them to fail. This adds othervm to some failing tests, and adds a note to other tests that do not have othervm indicating specifically that they don't need othervm. Bug: https://bugs.openjdk.java.net/browse/jdk-8034000 Webrev: http://cr.openjdk.java.net/~smarks/reviews/8034000/webrev.0/ Thanks, s'marks
Re: RFR (S): JDK-8034000 lack of /othervm option can cause some RMI tests to fail
Look good Stuart, -Joe On 02/12/2014 11:08 PM, Stuart Marks wrote: Hi all, The RMI test directories were removed from TEST.ROOT's othervm.dirs by JDK-8031179 so that individual RMI tests could opt-in to othervm themselves. Unfortunately, some tests needed othervm but didn't get it, causing them to fail. This adds othervm to some failing tests, and adds a note to other tests that do not have othervm indicating specifically that they don't need othervm. Bug: https://bugs.openjdk.java.net/browse/jdk-8034000 Webrev: http://cr.openjdk.java.net/~smarks/reviews/8034000/webrev.0/ Thanks, s'marks
