Re: RFR (JAXP): 8033980 : Xerces Update: datatype XMLGregorianCalendarImpl and DurationImpl
Hi Alan, Lance, and Daniel, The Xerces serialization revision meant to create a serialization form that would help maintain future serialization compatibility. But in reality it itself is causing significant incompatibility as Alan pointed out below and we discussed previously. I've removed the revision from the patch as a result. Please see the new webrev here: http://cr.openjdk.java.net/~joehw/jdk9/8033980/webrev/ Thanks, Joe On 2/12/2014 11:45 AM, Alan Bateman wrote: 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: JDK-8032050: TEST_BUG: java/rmi/activation/Activatable/shutdownGracefully/ShutdownGracefully.java fails intermittently
Thank you Stuart I have fixed comment in JavaVM.java. Dealing with different cases in ShutdownGracefully.java, two variables were added. One is a flag indicate test passed or not. Other variable keeps the error message when test failed. I put TestLibrary.bomb in the bottom of the main method which only shows test fail message. Could you review it again http://cr.openjdk.java.net/~tyan/JDK-8032050/webrev.04/ Tristan On 02/13/2014 05:29 AM, Stuart Marks wrote: 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
Re: RFR (S): JDK-8034000 lack of /othervm option can cause some RMI tests to fail
On 13/02/2014 07:08, Stuart Marks wrote: Webrev: http://cr.openjdk.java.net/~smarks/reviews/8034000/webrev.0/ Looks good to me too. -Alan
Fwd: Re: Fwd: Improve large array allocation / gc intrinsics
Comments opinions from core-libs, please ? Laurent -- Message transféré -- De : Thomas Schatzl thomas.scha...@oracle.com Date : 11 févr. 2014 11:28 Objet : Re: Fwd: Improve large array allocation / gc intrinsics À : Laurent Bourgès bourges.laur...@gmail.com Cc : hotspot-...@openjdk.java.net Hi, just a few comments... De : Laurent Bourgès bourges.laur...@gmail.com Date : 10 févr. 2014 10:24 Objet : Improve large array allocation / gc intrinsics À : core-libs-dev core-libs-dev@openjdk.java.net, discuss disc...@openjdk.java.net Cc : Dear all, I would like to propose a JDK9 RFE to improve JVM efficiency when dealing with large arrays (allocation + gc). In several scientific applications (and my patched java2d pisces), many large arrays are needed, created on the fly and it becomes very painful to recycle them using an efficient array cache (concurrency, cache size tuning, clear + cache eviction issues). Why do you think that a one-size fits all approach that any library in the JDK will not have the same issues to deal with? How do you know that a generic library in the JDK (as in your proposal) or hacking things into the VM can deal with this problem better than you who knows the application allocation patterns? Do you have a prototype (library) for that? In this case, the GC overhead leads to a big performance penalty (several hundred megabytes per seconds). I do not understand what the problem is. Typically I would not specify performance (throughput?) penalty in megabytes per seconds. Do the GCs take too long, or do you feel there too much memory wastage somewhere? As such array cache are very efficient in an application context, I am wondering if that approach could be promoted at the JDK level itself: - provide a new array allocator for large arrays that can return larger arrays than expected (size = 4 or 8 multiples) using array caches (per thread ?) stored in a dedicated JVM large memory area The behavior you propose seems very particular to your application. Others may have completely different requirements. The mentioned per-thread caches do not seem to be problematic to do in a library either. (GC aware) and providing efficient cache eviction policies Did you every try one of the other available garbage collectors with your application? Both CMS and G1 never move large objects around, i.e. there is almost no direct GC overhead associated with them. Reclaiming them is almost zero cost in these collectors. Keeping them alive seems to be best handled by logic in a library. Can you give examples where the VM has significant advantages over a dedicated library, or a particular use case with measurements that show this could be the case? - may support for both clean arrays (zero filled) or dirty arrays because some algorithms does not need zero-filled arrays. - improve JVM intrinsics (array clear, fill) to achieve maximum performance ie take into account the data alignment (4 or 8 multiples) I think the compiler already uses specialized methods for that, using the best instructions that are available. It should also already be able to detect fill loops, and vectorize them. Objects are always 8 byte aligned - I think you can force higher alignments by setting ObjectAlignmentInBytes or so. Otherwise these changes could be simply added, i.e. seems to not need any RFE. - upgrade GC to recycle such 'cached' arrays (clean), update usage statistics and manage cache eviction policy (avoid wasting memory) The GCs already automatically recycle the freed space. Everything else seems to be more complicated to implement at VM level than at library level, with the added drawback that you add a VM dependency. Please give me your feedback opinion and evaluate if this feature seems possible to implement into the JDK (hotspot, gc, core-libs)... Thanks, Thomas
Re: RFR (JAXP): 8033980 : Xerces Update: datatype XMLGregorianCalendarImpl and DurationImpl
Hi Joe, Couldn't all the new orig_* fields in XMLGregorianCalendarImpl be made transient? It looks as if they shouldn't be serialized anyway. Should they? Not making them transient would change the serialization form, and I'm not sure where that would take us... best regards, -- daniel On 2/13/14 9:18 AM, huizhe wang wrote: Hi Alan, Lance, and Daniel, The Xerces serialization revision meant to create a serialization form that would help maintain future serialization compatibility. But in reality it itself is causing significant incompatibility as Alan pointed out below and we discussed previously. I've removed the revision from the patch as a result. Please see the new webrev here: http://cr.openjdk.java.net/~joehw/jdk9/8033980/webrev/ Thanks, Joe On 2/12/2014 11:45 AM, Alan Bateman wrote: 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: JEP 187: Serialization 2.0
On 01/23/2014 03:30 PM, Chris Hegarty wrote:
On 22 Jan 2014, at 15:14, Florian Weimer fwei...@redhat.com wrote:
On 01/22/2014 03:47 PM, Chris Hegarty wrote:
On 22/01/14 13:57, Florian Weimer wrote:
On 01/14/2014 01:26 AM, mark.reinh...@oracle.com wrote:
Posted: http://openjdk.java.net/jeps/187
There's another aspect of the current approach to serialization that is
not mentioned: the type information does not come from the calling
context, but exclusively from the input stream.
Have you overlooked resolveClass [1], or are you looking for additional
context?
I mean something slightly different, so here's a concrete example: Assume we
are deserializing an instance of a class with a String field. The current
framework deserializes whatever is in the input stream, and will bail out with
a ClassCastException if the deserialized object turns out unusable. Contrast
this with an alternative approach that uses the knowledge that the field String
and will refuse to read anything else from the input stream.
Sorry, but I may still be missing your point. From my reading of the code, CCE
will only be thrown if the object being deserialized contains a field with a
type that does not match the type of the similarly named field in the class
loaded by the deserializing context.
This is from the method
ObjectStreamClass.FieldReflector::setObjFieldValues(Object, Object[]),
called indirectly from ObjectInputStream::readObject0(boolean):
Object val = vals[offsets[i]];
if (val != null
!types[i - numPrimFields].isInstance(val))
{
Field f = fields[i].getField();
throw new ClassCastException(
cannot assign instance of +
val.getClass().getName() + to field +
f.getDeclaringClass().getName() + . +
f.getName() + of type +
f.getType().getName() + in instance of +
obj.getClass().getName());
}
So the exception is thrown after the object val has been read from the
stream, which involves its construction and deserialization. This means
that you always expose the full serialization functionality, no matter
with which types you start.
Are you thinking specifically about corrupt/malicious streams, or evolving
serializable types? Or a performance optimisation?
Mostly the former. Sorry, I should have said so.
There are serialization frameworks which restrict the types of
deserializable objects based on the type of the top-level object being
requested. Or in Java serialization terms, they won't load and
instantiate arbitrary classes even if the programmer did not provide a
customer, restrictive resolveClass() implementation. I get that
type-erased generics make it complicated to achieve this in current
Java. (But so is writing a correct resolveClass() because it breaks
encapsulation.)
Knowing in advance which types to expect in the stream would allow for a
performance optimization, but something like that could be added to the
current framework as well.
--
Florian Weimer / Red Hat Product Security Team
Re: RFR for JDK-8031563: TEST_BUG: java/nio/channels/Selector/ChangingInterests.java failed once
On 12/02/2014 13:34, michael cui wrote: : 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? I plan to look at this soon but I have a concern that it is changing the test so that the change from OP_READ to OP_READ|OP_WRITE no longer tests that the key is both readable and writable after one select. -Alan
Re: RFR (JAXP): 8033980 : Xerces Update: datatype XMLGregorianCalendarImpl and DurationImpl
On 13/02/2014 08:18, huizhe wang wrote: Hi Alan, Lance, and Daniel, The Xerces serialization revision meant to create a serialization form that would help maintain future serialization compatibility. But in reality it itself is causing significant incompatibility as Alan pointed out below and we discussed previously. I've removed the revision from the patch as a result. Please see the new webrev here: http://cr.openjdk.java.net/~joehw/jdk9/8033980/webrev/ Thanks for dropping the serialization change as it was just not going to work the way you had intended. I agree with Daniel's comment about all the new fields added to XMLGregorianCalendarImpl as it's not clear why they aren't transient. I have not studied the rest of the changes but I think Daniel and Lance are. -Alan
Re: A hole in the serialization spec
On 12 Feb 2014, at 15:24, David M. Lloyd david.ll...@redhat.com wrote:
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).
Yes, possibly. And who knows what fields/values may be read and mistaken for
the wrong object in the hierarchy. So ‘undefined' behaviour seems right to me.
A simple example throws StreamCorruptedException with Oracles JDK:
public class NoFields {
public static void main(String[] args) throws Exception {
ByteArrayOutputStream baos = new ByteArrayOutputStream();
try (ObjectOutputStream oos = new ObjectOutputStream(baos)) {
oos.writeObject(new B(5, 10));
}
ByteArrayInputStream bais = new
ByteArrayInputStream(baos.toByteArray());
ObjectInputStream ois = new ObjectInputStream(bais);
B b = (B)ois.readObject();
System.out.println(aValue = + b.aValue);
System.out.println(bValue = + b.bValue);
}
static class A implements Serializable {
final int aValue;
A(int value) { this.aValue = value; }
private void writeObject(ObjectOutputStream oos) throws IOException {
oos.defaultWriteObject(); // comment out
}
}
static class B extends A implements Serializable {
final int bValue;
B(int aValue, int bValue) { super(aValue); this.bValue = bValue; }
}
-Chris.
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
Re: A hole in the serialization spec
On 02/13/2014 10:29 AM, Chris Hegarty wrote: On 12 Feb 2014, at 15:24, David M. Lloyd david.ll...@redhat.com wrote: 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). Yes, possibly. And who knows what fields/values may be read and mistaken for the wrong object in the hierarchy. So ‘undefined' behaviour seems right to me. I think the behavior is well-defined, just bad, which is my point. If the exact current is spec'd out as-is then at least we can be assured of the same bad behavior across implementations. If the behavior is changed such that fields are read/written but discarded, without updating the spec, then the undefined behavior at least becomes safer. If the behavior is changed, *and* the spec is updated, then we get both benefits, but at the cost that all previous implementations will not be compliant with the spec. All options seem to have a cost though. -- - DML
Re: RFR: 7152892: some jtreg tests fail with permission denied
Sorry Mandy, I totally missed your reply. I'll get this resolved separately. -Rob On 10/02/14 15:43, Mandy Chung wrote: On 2/7/2014 10:44 AM, Rob McKenna wrote: Hi folks, When files are copied by our test harness the permissions are left unchanged. This can cause trouble for a few tests when the test folder is read only. http://cr.openjdk.java.net/~robm/7152892/webrev.01/ AIX is missing in your change in Assert.sh. Assert.sh doesn't seem to be necessary. I think it can replace the shell test with @build to compile the classes. It may also need to change java Assert with the right classpath (I haven't checked in details) in Assert.java (which execs the java command). Do you mind looking into the possibility to remove Assert.sh? $ hg diff Assert.java diff --git a/test/java/lang/ClassLoader/Assert.java b/test/java/lang/ClassLoader/Assert.java --- a/test/java/lang/ClassLoader/Assert.java +++ b/test/java/lang/ClassLoader/Assert.java @@ -24,7 +24,8 @@ /* * @test * @bug 4290640 4785473 - * @run shell/timeout=300 Assert.sh + * @build package1.Class1 package2.Class2 package1.package3.Class3 Assert + * @run main/othervm Assert * @summary Test the assertion facility * @author Mike McCloskey */ Mandy
Re: RFR: 7152892: some jtreg tests fail with permission denied
On 2/13/14 9:54 AM, Rob McKenna wrote: Sorry Mandy, I totally missed your reply. I'll get this resolved separately. No worries. It's fine to do it separately. Mandy -Rob On 10/02/14 15:43, Mandy Chung wrote: On 2/7/2014 10:44 AM, Rob McKenna wrote: Hi folks, When files are copied by our test harness the permissions are left unchanged. This can cause trouble for a few tests when the test folder is read only. http://cr.openjdk.java.net/~robm/7152892/webrev.01/ AIX is missing in your change in Assert.sh. Assert.sh doesn't seem to be necessary. I think it can replace the shell test with @build to compile the classes. It may also need to change java Assert with the right classpath (I haven't checked in details) in Assert.java (which execs the java command). Do you mind looking into the possibility to remove Assert.sh? $ hg diff Assert.java diff --git a/test/java/lang/ClassLoader/Assert.java b/test/java/lang/ClassLoader/Assert.java --- a/test/java/lang/ClassLoader/Assert.java +++ b/test/java/lang/ClassLoader/Assert.java @@ -24,7 +24,8 @@ /* * @test * @bug 4290640 4785473 - * @run shell/timeout=300 Assert.sh + * @build package1.Class1 package2.Class2 package1.package3.Class3 Assert + * @run main/othervm Assert * @summary Test the assertion facility * @author Mike McCloskey */ Mandy
RFR 8034896, clean up typo in Clob.free
Hi all, Need a reviewer for this trivial change to remove the redundant the resources from Clob.free. I will be going through all of the java[x]/sql classes at a later time to clean up the use of code/code so I did not touch for now. Best Lance hg diff diff -r 4711a64b6a13 src/share/classes/java/sql/Clob.java --- a/src/share/classes/java/sql/Clob.java Thu Feb 13 17:40:43 2014 +0100 +++ b/src/share/classes/java/sql/Clob.java Thu Feb 13 14:55:33 2014 -0500 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1998, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1998, 2014, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -309,8 +309,8 @@ void truncate(long len) throws SQLException; /** - * This method frees the codeClob/code object and releases the resources the resources - * that it holds. The object is invalid once the codefree/code method + * This method releases the resources that the codeClob/code object + * holds. The object is invalid once the codefree/code method * is called. * p * After codefree/code has been called, any attempt to invoke a Best Lance
Re: RFR 8034896, clean up typo in Clob.free
Hi Lance, Looks good, -- daniel On 2/13/14 8:59 PM, Lance Andersen wrote: Hi all, Need a reviewer for this trivial change to remove the redundant the resources from Clob.free. I will be going through all of the java[x]/sql classes at a later time to clean up the use of code/code so I did not touch for now. Best Lance hg diff diff -r 4711a64b6a13 src/share/classes/java/sql/Clob.java --- a/src/share/classes/java/sql/Clob.java Thu Feb 13 17:40:43 2014 +0100 +++ b/src/share/classes/java/sql/Clob.java Thu Feb 13 14:55:33 2014 -0500 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1998, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1998, 2014, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -309,8 +309,8 @@ void truncate(long len) throws SQLException; /** - * This method frees the codeClob/code object and releases the resources the resources - * that it holds. The object is invalid once the codefree/code method + * This method releases the resources that the codeClob/code object + * holds. The object is invalid once the codefree/code method * is called. * p * After codefree/code has been called, any attempt to invoke a Best Lance
RFR 9: 8034903: Add Logging of Process.start arguments and resulting pid
Hi, Having folks stumbling over process creation and problems of quoting, especially on windows, it seems useful to log the native commands and arguments. They are proposed to be logged using the PlatformLogger at Level.FINE which will not be logged by default. The environment is useful in some cases, but verbose, that it should be Logged at FINER. The pid of the spawned process logged as well for traceability. Please take a look at this first draft and comment on whether it is useful, a good idea and any improvements needed. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-log-processcreate/ Thanks, Roger
Re: RFR 9: 8034903: Add Logging of Process.start arguments and resulting pid
Roger, I only have two suggested improvements: 1) solaris/../ProcessImpl.java should use Objects.toString() rather than the tenary operator for a string choice. You did this alright in /windows/../ProcessImpl.java 2) /windows/../ProcessImpl.java doesn't need to specify null for Objects.toString() since that is its default output for null input. Cheers, Paul On Thu, Feb 13, 2014 at 3:26 PM, roger riggs roger.ri...@oracle.com wrote: Hi, Having folks stumbling over process creation and problems of quoting, especially on windows, it seems useful to log the native commands and arguments. They are proposed to be logged using the PlatformLogger at Level.FINE which will not be logged by default. The environment is useful in some cases, but verbose, that it should be Logged at FINER. The pid of the spawned process logged as well for traceability. Please take a look at this first draft and comment on whether it is useful, a good idea and any improvements needed. Webrev: http://cr.openjdk.java.net/~rriggs/webrev-log-processcreate/ Thanks, Roger -- Cheers, Paul
Re: RFR: (8031737) CHECK_NULL and CHECK_EXCEPTION macros cleanup
That worked on Mac but I just found it doesn't build on Linux because a
macro-redefinition
warning is treated as an error there.
https://bugs.openjdk.java.net/browse/JDK-8034912
The early/original fix had removed the duplicate definition in
src/share/native/java/net/net_util.h
So the fix is to do the same in 8u :-
~/jdk8u-dev/jdk$ hg diff src/share/native/java/net/net_util.h
diff --git a/src/share/native/java/net/net_util.h
b/src/share/native/java/net/net_util.h
--- a/src/share/native/java/net/net_util.h
+++ b/src/share/native/java/net/net_util.h
@@ -42,9 +42,6 @@
#define NET_ERROR(env, ex, msg) \
{ if (!(*env)-ExceptionOccurred(env)) JNU_ThrowByName(env, ex, msg) }
-#define CHECK_NULL(x) if ((x) == NULL) return;
-#define CHECK_NULL_RETURN(x, y) if ((x) == NULL) return y;
-
/
* Cached field IDs
*
A little surprised no else else found this already (did they?)
Anyway I need a review and Ok to push 8034912 to JDk 8 u
-Phil.
On 02/12/2014 10:21 AM, Mike Duigou wrote:
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.
Re: RFR (S): JDK-8034000 lack of /othervm option can cause some RMI tests to fail
Thanks guys. And Joe, you know that I *always* look good! :-) s'marks On 2/12/14 11:31 PM, Joe Darcy wrote: 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
Re: RFR for JDK-8030844:sun/rmi/rmic/classpath/RMICClassPathTest.java timeout in same binaries run
Great, pushed: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/2cecdf7fbc90 Having the full changeset in the webrev was quite convenient. I was able to hg import it directly. s'marks On 2/12/14 7:21 PM, Tristan Yan wrote: 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
Re: RFR: (8031737) CHECK_NULL and CHECK_EXCEPTION macros cleanup
Hi Phil,
There was an earlier commit that updated java.net
8030875: Macros for checking and returning on exceptions
Maybe it will apply as a backport too?
Roger
On 2/13/14 4:50 PM, Phil Race wrote:
That worked on Mac but I just found it doesn't build on Linux because
a macro-redefinition
warning is treated as an error there.
https://bugs.openjdk.java.net/browse/JDK-8034912
The early/original fix had removed the duplicate definition in
src/share/native/java/net/net_util.h
So the fix is to do the same in 8u :-
~/jdk8u-dev/jdk$ hg diff src/share/native/java/net/net_util.h
diff --git a/src/share/native/java/net/net_util.h
b/src/share/native/java/net/net_util.h
--- a/src/share/native/java/net/net_util.h
+++ b/src/share/native/java/net/net_util.h
@@ -42,9 +42,6 @@
#define NET_ERROR(env, ex, msg) \
{ if (!(*env)-ExceptionOccurred(env)) JNU_ThrowByName(env, ex, msg) }
-#define CHECK_NULL(x) if ((x) == NULL) return;
-#define CHECK_NULL_RETURN(x, y) if ((x) == NULL) return y;
-
/
* Cached field IDs
*
A little surprised no else else found this already (did they?)
Anyway I need a review and Ok to push 8034912 to JDk 8 u
-Phil.
On 02/12/2014 10:21 AM, Mike Duigou wrote:
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.
Re: RFR: (8031737) CHECK_NULL and CHECK_EXCEPTION macros cleanup
Looks good. Sorry I didn't catch this earlier and I was counting on
the test build :)
Mandy
[1] http://hg.openjdk.java.net/jdk9/dev/jdk/rev/c58c6b0fbe34
On 2/13/2014 1:50 PM, Phil Race wrote:
That worked on Mac but I just found it doesn't build on Linux because
a macro-redefinition
warning is treated as an error there.
https://bugs.openjdk.java.net/browse/JDK-8034912
The early/original fix had removed the duplicate definition in
src/share/native/java/net/net_util.h
So the fix is to do the same in 8u :-
~/jdk8u-dev/jdk$ hg diff src/share/native/java/net/net_util.h
diff --git a/src/share/native/java/net/net_util.h
b/src/share/native/java/net/net_util.h
--- a/src/share/native/java/net/net_util.h
+++ b/src/share/native/java/net/net_util.h
@@ -42,9 +42,6 @@
#define NET_ERROR(env, ex, msg) \
{ if (!(*env)-ExceptionOccurred(env)) JNU_ThrowByName(env, ex, msg) }
-#define CHECK_NULL(x) if ((x) == NULL) return;
-#define CHECK_NULL_RETURN(x, y) if ((x) == NULL) return y;
-
/
* Cached field IDs
*
A little surprised no else else found this already (did they?)
Anyway I need a review and Ok to push 8034912 to JDk 8 u
-Phil.
On 02/12/2014 10:21 AM, Mike Duigou wrote:
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.
Re: RFR: (8031737) CHECK_NULL and CHECK_EXCEPTION macros cleanup
I did look closely at that today. It has 3 parts
- removing the ones in net_util
- moving these macros to jni_util - however these new versions are
already superseded by the
ones we have, so we can' t apply that.
- updating some 'pack' code to use the new macros instead of its own
ones (no clash here)
So except for the last part I essentially have that fix here (and more)
and at this point I just
wanted to fix the build problem ...
-phil.
On 2/13/14 5:27 PM, Roger Riggs wrote:
Hi Phil,
There was an earlier commit that updated java.net
8030875: Macros for checking and returning on exceptions
Maybe it will apply as a backport too?
Roger
On 2/13/14 4:50 PM, Phil Race wrote:
That worked on Mac but I just found it doesn't build on Linux because
a macro-redefinition
warning is treated as an error there.
https://bugs.openjdk.java.net/browse/JDK-8034912
The early/original fix had removed the duplicate definition in
src/share/native/java/net/net_util.h
So the fix is to do the same in 8u :-
~/jdk8u-dev/jdk$ hg diff src/share/native/java/net/net_util.h
diff --git a/src/share/native/java/net/net_util.h
b/src/share/native/java/net/net_util.h
--- a/src/share/native/java/net/net_util.h
+++ b/src/share/native/java/net/net_util.h
@@ -42,9 +42,6 @@
#define NET_ERROR(env, ex, msg) \
{ if (!(*env)-ExceptionOccurred(env)) JNU_ThrowByName(env, ex, msg) }
-#define CHECK_NULL(x) if ((x) == NULL) return;
-#define CHECK_NULL_RETURN(x, y) if ((x) == NULL) return y;
-
/
* Cached field IDs
*
A little surprised no else else found this already (did they?)
Anyway I need a review and Ok to push 8034912 to JDk 8 u
-Phil.
On 02/12/2014 10:21 AM, Mike Duigou wrote:
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.
Re: RFR: (8031737) CHECK_NULL and CHECK_EXCEPTION macros cleanup
Yeah sorry about that. It built on Windows and Mac but not Linux as i
found out today.
-phil.
On 2/13/14 5:36 PM, Mandy Chung wrote:
Looks good. Sorry I didn't catch this earlier and I was counting on
the test build :)
Mandy
[1] http://hg.openjdk.java.net/jdk9/dev/jdk/rev/c58c6b0fbe34
On 2/13/2014 1:50 PM, Phil Race wrote:
That worked on Mac but I just found it doesn't build on Linux because
a macro-redefinition
warning is treated as an error there.
https://bugs.openjdk.java.net/browse/JDK-8034912
The early/original fix had removed the duplicate definition in
src/share/native/java/net/net_util.h
So the fix is to do the same in 8u :-
~/jdk8u-dev/jdk$ hg diff src/share/native/java/net/net_util.h
diff --git a/src/share/native/java/net/net_util.h
b/src/share/native/java/net/net_util.h
--- a/src/share/native/java/net/net_util.h
+++ b/src/share/native/java/net/net_util.h
@@ -42,9 +42,6 @@
#define NET_ERROR(env, ex, msg) \
{ if (!(*env)-ExceptionOccurred(env)) JNU_ThrowByName(env, ex, msg) }
-#define CHECK_NULL(x) if ((x) == NULL) return;
-#define CHECK_NULL_RETURN(x, y) if ((x) == NULL) return y;
-
/
* Cached field IDs
*
A little surprised no else else found this already (did they?)
Anyway I need a review and Ok to push 8034912 to JDk 8 u
-Phil.
On 02/12/2014 10:21 AM, Mike Duigou wrote:
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.
