Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
Doug Lea wrote:
On 01/14/13 17:58, Chris Hegarty wrote:
I'm not sure what we are saving here. Since new TLR is a singleton, we
save ~64 bytes per classloader. Worth messing with advanced
serialization mechanics? Probably not. I'll publish a revised webrev
soon.
serialPersistentFields is really quite simple, and will help avoid
any potential issues like this in the future. I can help implement
this on top of your patch if you like.
Thanks to Alan and Aleksey for noticing this and to Chris for
offering some serialPersistentFields incantations!
(The only way to serialize a TLR represents a strange abuse to
begin with. You'd need to save the result of
ThreadLocalRandom.current() in a field of a serialized object.
Which would be a terrible idea ...)
Right, which means we should also add a readResolve() method to return
ThreadLocalRandom.current(). All you need to do is add this to the
class and the format should be compatible with Java 1.7. This means
that if someone made the mistake of writing ThreadLocalRandom instances,
they would now still be able to read, but would instead get the correct
instance back for their thread:
/**
* We keep the same serial persistent format as in Java 1.7.
*/
private static final ObjectStreamField[] serialPersistentFields = {
new ObjectStreamField(rnd, long.class),
new ObjectStreamField(initialized, boolean.class),
new ObjectStreamField(pad0, long.class),
new ObjectStreamField(pad1, long.class),
new ObjectStreamField(pad2, long.class),
new ObjectStreamField(pad3, long.class),
new ObjectStreamField(pad4, long.class),
new ObjectStreamField(pad5, long.class),
new ObjectStreamField(pad6, long.class),
new ObjectStreamField(pad7, long.class),
};
/**
* Writes the ThreadLocalRandom object to the ObjectOutputStream
using the
* same format as in the past.
*/
private void writeObject(ObjectOutputStream out) throws IOException {
ObjectOutputStream.PutField fields = out.putFields();
fields.put(rnd, rnd);
fields.put(initialized, true);
fields.put(pad0, 0L);
fields.put(pad1, 0L);
fields.put(pad2, 0L);
fields.put(pad3, 0L);
fields.put(pad4, 0L);
fields.put(pad5, 0L);
fields.put(pad6, 0L);
fields.put(pad7, 0L);
out.writeFields();
}
/**
* Reads the ThreadLocalRandom object from the stream in order to keep
* the format compatible, but does not use any of the read data.
*/
private void readObject(ObjectInputStream in)
throws ClassNotFoundException, IOException {
in.readFields();
// we can ignore the values, since we will replace them in the
// readResolve() method anyway
}
/**
* Once the ThreadLocalRandom object has been read from the stream, we
* throw it away and instead return the correct instance for our thread.
*/
private Object readResolve() {
return current();
}
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
On 14/01/2013 23:55, Doug Lea wrote: Thanks to Alan and Aleksey for noticing this and to Chris for offering some serialPersistentFields incantations! (The only way to serialize a TLR represents a strange abuse to begin with. You'd need to save the result of ThreadLocalRandom.current() in a field of a serialized object. Which would be a terrible idea ...) It does seem nonsensical. Given that the padding isn't used then the simplest thing might be to do nothing, meaning treat this update as an API change that changes the serialized form. I don't see any compatibility issues as deserialization on an older release will just leave the padding fields with their default values (and as they are unused then it shouldn't matter). I think this would be simplest than adding serialPersistentFields and a writeObject to write these unused fields. -Alan
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
On 01/15/2013 09:55 AM, Alan Bateman wrote: On 14/01/2013 23:55, Doug Lea wrote: Thanks to Alan and Aleksey for noticing this and to Chris for offering some serialPersistentFields incantations! (The only way to serialize a TLR represents a strange abuse to begin with. You'd need to save the result of ThreadLocalRandom.current() in a field of a serialized object. Which would be a terrible idea ...) It does seem nonsensical. Given that the padding isn't used then the simplest thing might be to do nothing, meaning treat this update as an API change that changes the serialized form. I don't see any compatibility issues as deserialization on an older release will just leave the padding fields with their default values (and as they are unused then it shouldn't matter). I think this would be simplest than adding serialPersistentFields and a writeObject to write these unused fields. -Alan If this is an API change, then it might as well be a change in the serializability of the ThreadLocalRandom instance. Since TLR extends Random which is already Serializable, this could be done by throwing appropriate exception in writeObject(). I don't think anyone would really notice. Serializability of a java.util.Random is meant to transfer the state of the object over the wire (including current seed). This can't be done for ThreadLocalRandom since it's a singleton with thread-bound state. So serializability of TLR is dubious. Regards, Peter
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
On 01/15/2013 01:40 PM, Peter Levart wrote: If this is an API change, then it might as well be a change in the serializability of the ThreadLocalRandom instance. Since TLR extends Random which is already Serializable, this could be done by throwing appropriate exception in writeObject(). I don't think anyone would really notice. Serializability of a java.util.Random is meant to transfer the state of the object over the wire (including current seed). This can't be done for ThreadLocalRandom since it's a singleton with thread-bound state. So serializability of TLR is dubious. That's an interesting thought indeed. However, I would say already receiving TLR from the stream and throwing the don't want it exception would be surprising. We are at the curse of having these fields exposed in serialized form; the only thing we can do is to minimize further leakage. Chris, do you need help preparing the patch? -Aleksey.
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
On 01/15/2013 09:55 AM, Alan Bateman wrote: On 14/01/2013 23:55, Doug Lea wrote: Thanks to Alan and Aleksey for noticing this and to Chris for offering some serialPersistentFields incantations! (The only way to serialize a TLR represents a strange abuse to begin with. You'd need to save the result of ThreadLocalRandom.current() in a field of a serialized object. Which would be a terrible idea ...) It does seem nonsensical. Given that the padding isn't used then the simplest thing might be to do nothing, meaning treat this update as an API change that changes the serialized form. I don't see any compatibility issues as deserialization on an older release will just leave the padding fields with their default values (and as they are unused then it shouldn't matter). I think this would be simplest than adding serialPersistentFields and a writeObject to write these unused fields. And one more thing. With moving of fields off the TLR object, the semantics of de-serialization change. JDK7 TLR deserializes into an instance which is not thread-bound with the copy of the seed of original instance. So serializing/deserializing acts as a form of cloning the instance and detaching the copy from the thread. Proposed TLR can't keep this semantics (unless it is modeled as a private subclass of TLR for example which overrides existing logic and provides a writeReplace() method that replaces it with plain TLR instance before serializing). Regards, Peter
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
On 15/01/2013 09:40, Peter Levart wrote: If this is an API change, then it might as well be a change in the serializability of the ThreadLocalRandom instance. Since TLR extends Random which is already Serializable, this could be done by throwing appropriate exception in writeObject(). I don't think anyone would really notice. Dubious as it might be, I'm sure it would break somebody so I think it would be safer not to do this. -Alan.
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
On 01/15/2013 09:57 AM, Aleksey Shipilev wrote:
On 01/15/2013 01:40 PM, Peter Levart wrote:
If this is an API change, then it might as well be a change in the
serializability of the ThreadLocalRandom instance. Since TLR extends
Random which is already Serializable, this could be done by throwing
appropriate exception in writeObject().
I don't think anyone would really notice.
Serializability of a java.util.Random is meant to transfer the state of
the object over the wire (including current seed). This can't be done
for ThreadLocalRandom since it's a singleton with thread-bound state. So
serializability of TLR is dubious.
That's an interesting thought indeed. However, I would say already
receiving TLR from the stream and throwing the don't want it exception
would be surprising. We are at the curse of having these fields exposed
in serialized form; the only thing we can do is to minimize further leakage.
Chris, do you need help preparing the patch?
I'm still not sure that we have agreement here.
My preference, for what it's worth, is to forge ahead with Peter's
suggestion, and update the TLR specification to throw in the case of
writeObject. I believe this would be the best solution.
Add similar text to the class description:
* Note, that although ThreadLocalRandom inherits Serializable
* interface from Random, it is not intended to be Serializable.
* Appropriate serialization methods are implemented to throw
* NotSerializableException.
/**
* Throws NotSerializableException, since TLR
* objects are not intended to be serializable.
*/
private void writeObject(java.io.ObjectOutputStream out)
throws NotSerializableException
{
throw new NotSerializableException(Not serializable.);
}
private void readObject(java.io.ObjectInputStream in)
throws NotSerializableException
{
// swallow all field data.
}
This of course, is subject to spec change approval.
-Chris.
-Aleksey.
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
On 01/15/2013 02:12 PM, Chris Hegarty wrote:
On 01/15/2013 09:57 AM, Aleksey Shipilev wrote:
Chris, do you need help preparing the patch?
I'm still not sure that we have agreement here.
Sorry, I should not have replied in this thread.
My preference, for what it's worth, is to forge ahead with Peter's
suggestion, and update the TLR specification to throw in the case of
writeObject. I believe this would be the best solution.
Add similar text to the class description:
* Note, that although ThreadLocalRandom inherits Serializable
* interface from Random, it is not intended to be Serializable.
* Appropriate serialization methods are implemented to throw
* NotSerializableException.
/**
* Throws NotSerializableException, since TLR
* objects are not intended to be serializable.
*/
private void writeObject(java.io.ObjectOutputStream out)
throws NotSerializableException
{
throw new NotSerializableException(Not serializable.);
}
private void readObject(java.io.ObjectInputStream in)
throws NotSerializableException
{
// swallow all field data.
}
I'm technically OK with this, plus:
+ readResolve returning the actual TLR
+ SVUID fixup
This of course, is subject to spec change approval.
...but this leads me back to thinking if we should still persist the
leaked serialization form (serialPersistentFields code put together by
Heinz is fine) to make this functionality in JDK8 for Doug to rely on,
and then back off to suggestions for collapsing serialization forms.
-Aleksey.
hg: jdk8/tl/jdk: 8005250: Downgrade normative references to ${java.home}/lib folder from Java client code.
Changeset: a40052a54801
Author:uta
Date: 2013-01-15 14:26 +0400
URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a40052a54801
8005250: Downgrade normative references to ${java.home}/lib folder from Java
client code.
Summary: Javadoc was changed in accordance with CCC-8005250 request.
Reviewed-by: alanb, amenkov
! src/share/classes/java/awt/datatransfer/SystemFlavorMap.java
! src/share/classes/javax/imageio/spi/IIORegistry.java
! src/share/classes/javax/sound/midi/MidiSystem.java
! src/share/classes/javax/sound/sampled/AudioSystem.java
! src/share/classes/javax/swing/UIManager.java
hg: jdk8/tl/jdk: 8005406: HTTP server implementation should use Base64 API
Changeset: 4b012af44f24 Author:chegar Date: 2013-01-15 11:44 + URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/4b012af44f24 8005406: HTTP server implementation should use Base64 API Reviewed-by: khazra, alanb, chegar Contributed-by: Mark Sheppard mark.shepp...@oracle.com ! src/share/classes/com/sun/net/httpserver/BasicAuthenticator.java ! src/share/classes/sun/net/www/protocol/http/BasicAuthentication.java
Re: Request for review: 8005618 - TEST_BUG: java/lang/ProcessBuilder/Basic.java failing intermittently
On 15/01/2013 01:31, David Holmes wrote: On 15/01/2013 7:12 AM, Rob McKenna wrote: Simple enough fix but to be honest I'm not sure any value will *always* work for the dead process waitFor(). Our testing infrastructure seems to glide past whatever we consider to be acceptable tolerances. http://cr.openjdk.java.net/~robm/8005618/webrev.01/ http://cr.openjdk.java.net/%7Erobm/8005618/webrev.01/ Using the latch seems reasonable but the existing wait/sleep times do not. Why waitFor(1) if the main thread is going to interrupt you after a sleep(1000) ??? It's testing that Process.waitFor will be interrupted by Thread.interrupt so it requires a thread to block in waitFor. Using sleeps is always going to be problematic as the load on test machines is unpredictable but I think Rob's proposed change does make this test a bit more robust. -Alan.
Re: Request for review: 8005618 - TEST_BUG: java/lang/ProcessBuilder/Basic.java failing intermittently
On 15/01/2013 9:55 PM, Alan Bateman wrote: On 15/01/2013 01:31, David Holmes wrote: On 15/01/2013 7:12 AM, Rob McKenna wrote: Simple enough fix but to be honest I'm not sure any value will *always* work for the dead process waitFor(). Our testing infrastructure seems to glide past whatever we consider to be acceptable tolerances. http://cr.openjdk.java.net/~robm/8005618/webrev.01/ http://cr.openjdk.java.net/%7Erobm/8005618/webrev.01/ Using the latch seems reasonable but the existing wait/sleep times do not. Why waitFor(1) if the main thread is going to interrupt you after a sleep(1000) ??? It's testing that Process.waitFor will be interrupted by Thread.interrupt so it requires a thread to block in waitFor. Using sleeps is always going to be problematic as the load on test machines is unpredictable but I think Rob's proposed change does make this test a bit more robust. Ah I see. I'd missed that aspect of this. Thanks for clarifying. David -Alan.
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
(The only way to serialize a TLR represents a strange abuse to
begin with. You'd need to save the result of
ThreadLocalRandom.current() in a field of a serialized object.
Which would be a terrible idea ...)
And so, anything sensible that we do here will have the
(too nice?) effect of fixing the errors of anyone
crazy enough to do this -- rather than deserializing into
a non-thread-local ThreadLocalRandom, it will attach to
the common one.
Given all this, I think Heinz's suggestion below looks suitable.
Chris, do you agree about the serialization incantations?
-Doug
On 01/15/13 03:06, Dr Heinz M. Kabutz wrote:
Right, which means we should also add a readResolve() method to return
ThreadLocalRandom.current(). All you need to do is add this to the class and
the format should be compatible with Java 1.7. This means that if someone made
the mistake of writing ThreadLocalRandom instances, they would now still be able
to read, but would instead get the correct instance back for their thread:
/**
* We keep the same serial persistent format as in Java 1.7.
*/
private static final ObjectStreamField[] serialPersistentFields = {
new ObjectStreamField(rnd, long.class),
new ObjectStreamField(initialized, boolean.class),
new ObjectStreamField(pad0, long.class),
new ObjectStreamField(pad1, long.class),
new ObjectStreamField(pad2, long.class),
new ObjectStreamField(pad3, long.class),
new ObjectStreamField(pad4, long.class),
new ObjectStreamField(pad5, long.class),
new ObjectStreamField(pad6, long.class),
new ObjectStreamField(pad7, long.class),
};
/**
* Writes the ThreadLocalRandom object to the ObjectOutputStream using the
* same format as in the past.
*/
private void writeObject(ObjectOutputStream out) throws IOException {
ObjectOutputStream.PutField fields = out.putFields();
fields.put(rnd, rnd);
fields.put(initialized, true);
fields.put(pad0, 0L);
fields.put(pad1, 0L);
fields.put(pad2, 0L);
fields.put(pad3, 0L);
fields.put(pad4, 0L);
fields.put(pad5, 0L);
fields.put(pad6, 0L);
fields.put(pad7, 0L);
out.writeFields();
}
/**
* Reads the ThreadLocalRandom object from the stream in order to keep
* the format compatible, but does not use any of the read data.
*/
private void readObject(ObjectInputStream in)
throws ClassNotFoundException, IOException {
in.readFields();
// we can ignore the values, since we will replace them in the
// readResolve() method anyway
}
/**
* Once the ThreadLocalRandom object has been read from the stream, we
* throw it away and instead return the correct instance for our thread.
*/
private Object readResolve() {
return current();
}
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
On 15/01/2013 13:25, Doug Lea wrote: : And so, anything sensible that we do here will have the (too nice?) effect of fixing the errors of anyone crazy enough to do this -- rather than deserializing into a non-thread-local ThreadLocalRandom, it will attach to the common one. Given all this, I think Heinz's suggestion below looks suitable. Chris, do you agree about the serialization incantations? Heinz's proposal is fine but I think you also have the option of just dropping the pad fields from the serialization form (meaning no need for serialPersistentFields or writeObject). Technically it would be API change but one that shouldn't have any impact as the fields were never used and hence deserializing to their default value is okay. -Alan
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
On 01/15/2013 01:25 PM, Doug Lea wrote:
(The only way to serialize a TLR represents a strange abuse to
begin with. You'd need to save the result of
ThreadLocalRandom.current() in a field of a serialized object.
Which would be a terrible idea ...)
And so, anything sensible that we do here will have the
(too nice?) effect of fixing the errors of anyone
crazy enough to do this -- rather than deserializing into
a non-thread-local ThreadLocalRandom, it will attach to
the common one.
Given all this, I think Heinz's suggestion below looks suitable.
Chris, do you agree about the serialization incantations?
Yes, I am starting to come to this conclusion too (Heinz's suggestion).
There are a few @serialField tags to be added so we continue to generate
correct javadoc. Otherwise, I think we are good. I'll follow up on this
and send out a complete patch for review.
If the TLR spec is ever to be updated to specify that it is not ( any
more ) serializable (Peters suggestion), writeObect throw, we can do
this at a later stage.
-Chris.
-Doug
On 01/15/13 03:06, Dr Heinz M. Kabutz wrote:
Right, which means we should also add a readResolve() method to return
ThreadLocalRandom.current(). All you need to do is add this to the
class and
the format should be compatible with Java 1.7. This means that if
someone made
the mistake of writing ThreadLocalRandom instances, they would now
still be able
to read, but would instead get the correct instance back for their
thread:
/**
* We keep the same serial persistent format as in Java 1.7.
*/
private static final ObjectStreamField[] serialPersistentFields = {
new ObjectStreamField(rnd, long.class),
new ObjectStreamField(initialized, boolean.class),
new ObjectStreamField(pad0, long.class),
new ObjectStreamField(pad1, long.class),
new ObjectStreamField(pad2, long.class),
new ObjectStreamField(pad3, long.class),
new ObjectStreamField(pad4, long.class),
new ObjectStreamField(pad5, long.class),
new ObjectStreamField(pad6, long.class),
new ObjectStreamField(pad7, long.class),
};
/**
* Writes the ThreadLocalRandom object to the ObjectOutputStream
using the
* same format as in the past.
*/
private void writeObject(ObjectOutputStream out) throws IOException {
ObjectOutputStream.PutField fields = out.putFields();
fields.put(rnd, rnd);
fields.put(initialized, true);
fields.put(pad0, 0L);
fields.put(pad1, 0L);
fields.put(pad2, 0L);
fields.put(pad3, 0L);
fields.put(pad4, 0L);
fields.put(pad5, 0L);
fields.put(pad6, 0L);
fields.put(pad7, 0L);
out.writeFields();
}
/**
* Reads the ThreadLocalRandom object from the stream in order to
keep
* the format compatible, but does not use any of the read data.
*/
private void readObject(ObjectInputStream in)
throws ClassNotFoundException, IOException {
in.readFields();
// we can ignore the values, since we will replace them in the
// readResolve() method anyway
}
/**
* Once the ThreadLocalRandom object has been read from the
stream, we
* throw it away and instead return the correct instance for our
thread.
*/
private Object readResolve() {
return current();
}
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
On 01/15/2013 01:37 PM, Alan Bateman wrote: On 15/01/2013 13:25, Doug Lea wrote: : And so, anything sensible that we do here will have the (too nice?) effect of fixing the errors of anyone crazy enough to do this -- rather than deserializing into a non-thread-local ThreadLocalRandom, it will attach to the common one. Given all this, I think Heinz's suggestion below looks suitable. Chris, do you agree about the serialization incantations? Heinz's proposal is fine but I think you also have the option of just dropping the pad fields from the serialization form (meaning no need for serialPersistentFields or writeObject). Technically it would be API change but one that shouldn't have any impact as the fields were never used and hence deserializing to their default value is okay. But wouldn't there be an issue with JDK7 TLR streams being deserialized by JDK8, pad fields would be left in the Object input steam? If so, then we're down the road of having to have a readObject, etc... and possibly back to serialPersistentFields? -Chris. -Alan
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
On 15/01/2013 13:49, Chris Hegarty wrote: But wouldn't there be an issue with JDK7 TLR streams being deserialized by JDK8, pad fields would be left in the Object input steam? If so, then we're down the road of having to have a readObject, etc... and possibly back to serialPersistentFields? Technically deleting fields is an incompatible change as the stream will not contain the fields. If someone is crazy enough to serialize with jdk8 TLR and send it to a jdk7 release then the fields will have their default value (as the values aren't in the stream). As the fields aren't used then I don't think this should make a difference. Going the other way shouldn't be an issue either as the pad fields will be ignored. -Alan.
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
On 01/15/2013 02:57 PM, Alan Bateman wrote: On 15/01/2013 13:49, Chris Hegarty wrote: But wouldn't there be an issue with JDK7 TLR streams being deserialized by JDK8, pad fields would be left in the Object input steam? If so, then we're down the road of having to have a readObject, etc... and possibly back to serialPersistentFields? Technically deleting fields is an incompatible change as the stream will not contain the fields. If someone is crazy enough to serialize with jdk8 TLR and send it to a jdk7 release then the fields will have their default value (as the values aren't in the stream). As the fields aren't used then I don't think this should make a difference. Going the other way shouldn't be an issue either as the pad fields will be ignored. -Alan. Should there also be a change/patch to JDK7 TLR that would fix the strange behaviour (by adding readResolve)? This would align the semantics no matter which direction (7/7, 7/8, 8/7, 8/8) of serialization/deserialization is used... Regards, Peter
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
One more thing, not related to serialization: If a TLR reference is somehow passed from the thread that obtained it via TLR.current() to some other thread that did never call TLR.current() and this other thread calls methods on such instance (nextInt(), ...), it will start the random sequence from the zero seed, bypassing localInit() call... Is this ok? Regards, Peter
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
On 01/15/2013 06:33 PM, Peter Levart wrote: One more thing, not related to serialization: If a TLR reference is somehow passed from the thread that obtained it via TLR.current() to some other thread that did never call TLR.current() and this other thread calls methods on such instance (nextInt(), ...), it will start the random sequence from the zero seed, bypassing localInit() call... Is this ok? I think this counts as accidental sharing, and Javadoc recommends to always do TLR.current().* to take the appropriate TLR; hence I think this is the adopted failure scenario. The upside for *not* fixing this is skipping the initialized checks from each of next*() methods, which are needed in corner cases only, thus saving the hot-path performance. -Aleksey.
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
On 01/15/2013 03:38 PM, Aleksey Shipilev wrote: On 01/15/2013 06:33 PM, Peter Levart wrote: One more thing, not related to serialization: If a TLR reference is somehow passed from the thread that obtained it via TLR.current() to some other thread that did never call TLR.current() and this other thread calls methods on such instance (nextInt(), ...), it will start the random sequence from the zero seed, bypassing localInit() call... Is this ok? I think this counts as accidental sharing, and Javadoc recommends to always do TLR.current().* to take the appropriate TLR; hence I think this is the adopted failure scenario. The upside for *not* fixing this is skipping the initialized checks from each of next*() methods, which are needed in corner cases only, thus saving the hot-path performance. If javadoc recommends to always do TLR.current() then this method is also hot-path. So why not move the initialization check from current() to next(int bits)? Well, the following usage pattern can save some checks in the original case: TLR r = TLR.current(); r.next*(); r.next*(); Regards, Peter -Aleksey.
Re: RFR: javax.xml.xpath: Using ServiceLoader to load JAXP XPath factories (7169894: JAXP Plugability Layer: using service loader)
On 11/01/2013 14:27, Daniel Fuchs wrote: Hi, Here is a new webrev in the series that addresses using ServiceLoader in JAXP for JDK 8. 7169894: JAXP Plugability Layer: using service loader This changeset addresses modification in the javax.xml.xpath package. This changes are very similar to the changes proposed for the javax.xml.validation package, except that here we didn't need to add a new Error class. http://cr.openjdk.java.net/~dfuchs/JDK-7169894/javax.xml.xpath/webrev.00/ It's great to get the end of these updates. I looked at the xpath changes and it mostly looks good to me. As per the validation work, it looks like the isObjectModelSupported method will be called twice when assertions are enabled (it's a minor concern). One thing that I don't get is newXPathFactoryNoServiceLoader - maybe this is question for Joe but is this means to create the XPathFactory documented anywhere, I don't see it in the javadoc. A minor style comment on this is that method declaration looks like odd. Otherwise I think this is good. -Alan.
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
Updated webrev: http://cr.openjdk.java.net/~chegar/8005926/webrev.01/webrev/ * based on Aleksey initial webrev * applied serialPersistentFields, writeObject, readResolve ( as suggested by Heinz ) * did not apply readObject. It is unnecessary, defaultReadObject will read all the fields off the stream readResolve is necessary to give us good behavior ( as noted previously ). Once integrated, I will file a new bug to track the possible change of serialized form of TLR, and this can then remove serialPersistentFields and writeObject, if successful. -Chris. On 01/15/2013 01:57 PM, Alan Bateman wrote: On 15/01/2013 13:49, Chris Hegarty wrote: But wouldn't there be an issue with JDK7 TLR streams being deserialized by JDK8, pad fields would be left in the Object input steam? If so, then we're down the road of having to have a readObject, etc... and possibly back to serialPersistentFields? Technically deleting fields is an incompatible change as the stream will not contain the fields. If someone is crazy enough to serialize with jdk8 TLR and send it to a jdk7 release then the fields will have their default value (as the values aren't in the stream). As the fields aren't used then I don't think this should make a difference. Going the other way shouldn't be an issue either as the pad fields will be ignored. -Alan.
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
On 01/15/2013 08:33 PM, Chris Hegarty wrote: Updated webrev: http://cr.openjdk.java.net/~chegar/8005926/webrev.01/webrev/ Looks good to me; thanks for picking this up! -Aleksey. P.S. How many (Java) (concurrency) experts it takes to make a change to ThreadLocalRandom?
Re: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
Here is a revision http://cr.openjdk.java.net/~lancea/8006139/webrev.01 I still have to enter the JBS entry for the javadoc clarifications (and I also found another javadoc issue that was due to incorrect cut paste when the code was written) and ccc request As i mentioned in an earlier thread, these classes are hardly ever, if at all used and would only be used when UDTs are used and the majority of databases do not support this. Best lance On Jan 14, 2013, at 5:11 AM, Alan Bateman wrote: On 13/01/2013 23:51, Lance Andersen - Oracle wrote: : One other thing is that the CCE has a side-effect in that it consumes the next attribute. The methods could be changed to peek at the next attribute but that wouldn't work without synchronization or making it clear in the spec that the it is not a thread-safe implementation of SQLInput. I really want to keep the changes to the bare minimum here as this and the other serial classes are hardly, if ever used at all. I understand, but if you add a catch-all in the class description to cover the CCE case then this could be part of the same paragraph. -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: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
On 01/15/13 11:33, Chris Hegarty wrote: Updated webrev: http://cr.openjdk.java.net/~chegar/8005926/webrev.01/webrev/ Looks great; thanks!! -Doug * based on Aleksey initial webrev * applied serialPersistentFields, writeObject, readResolve ( as suggested by Heinz ) * did not apply readObject. It is unnecessary, defaultReadObject will read all the fields off the stream readResolve is necessary to give us good behavior ( as noted previously ). Once integrated, I will file a new bug to track the possible change of serialized form of TLR, and this can then remove serialPersistentFields and writeObject, if successful. -Chris. On 01/15/2013 01:57 PM, Alan Bateman wrote: On 15/01/2013 13:49, Chris Hegarty wrote: But wouldn't there be an issue with JDK7 TLR streams being deserialized by JDK8, pad fields would be left in the Object input steam? If so, then we're down the road of having to have a readObject, etc... and possibly back to serialPersistentFields? Technically deleting fields is an incompatible change as the stream will not contain the fields. If someone is crazy enough to serialize with jdk8 TLR and send it to a jdk7 release then the fields will have their default value (as the values aren't in the stream). As the fields aren't used then I don't think this should make a difference. Going the other way shouldn't be an issue either as the pad fields will be ignored. -Alan.
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
From user's point of view, there is nothing thread local about it - it is more like one global random generator, which is unseedable and unrepeatable. So why not call it a different name, like concurrent random? On Mon, Jan 14, 2013 at 4:17 PM, Alan Bateman alan.bate...@oracle.com wrote: On 14/01/2013 18:20, Chris Hegarty wrote: Ooh this change goes beyond my review capability!!! I thought we were just eliminating the indirection of ThreadLocal, anyway... As a basic review I don't see anything obviously wrong, and I don't have a problem with adding the fields to j.l.Thread, but I'm not in a position to review in detail the algorithm used. Alan, did mention that he may be in a better position to do a detailed review. I can also sponsor this change. I looked at the changes and they look good to me. The additional fields of java.lang.Thread shouldn't be an issue. The other thing I notice is that the serialized form of the original ThreadLocalRandom included the padding fields, I guess they should have been transient in the original implementation. -Alan.
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
On 15/01/2013 16:33, Chris Hegarty wrote: Updated webrev: http://cr.openjdk.java.net/~chegar/8005926/webrev.01/webrev/ * based on Aleksey initial webrev * applied serialPersistentFields, writeObject, readResolve ( as suggested by Heinz ) * did not apply readObject. It is unnecessary, defaultReadObject will read all the fields off the stream readResolve is necessary to give us good behavior ( as noted previously ). Once integrated, I will file a new bug to track the possible change of serialized form of TLR, and this can then remove serialPersistentFields and writeObject, if successful. The plan and the changes looks good to me too. -Alan
8006344: Broken javadoc link in javax.lang.model.element.Element
Minor oversight in the changes from 7193719: Support repeating
annotations in javax.lang.model.
~/repos/jdk8/tl/adder/build/solaris-i586/impsrc/javax/lang/model/element/Element.java:199:
warning - Tag @see: can't find getAnnotation() in
javax.lang.model.element.Element
I only noticed when building javadocs for another issue, thankfully we
don't have many warnings here ;-)
diff -r d54b4a091450 src/share/classes/javax/lang/model/element/Element.java
--- a/src/share/classes/javax/lang/model/element/Element.java Mon Jan
14 14:17:25 2013 -0800
+++ b/src/share/classes/javax/lang/model/element/Element.java Tue Jan
15 17:55:17 2013 +
@@ -188,7 +188,7 @@ public interface Element {
* type if present on this element, else an empty array
*
* @see #getAnnotationMirrors()
- * @see #getAnnotation()
+ * @see #getAnnotation(java.lang.Class)
* @see java.lang.reflect.AnnotatedElement#getAnnotations
* @see EnumConstantNotPresentException
* @see AnnotationTypeMismatchException
-Chris.
Re: 8006344: Broken javadoc link in javax.lang.model.element.Element
+1
On Jan 15, 2013, at 12:59 PM, Chris Hegarty wrote:
Minor oversight in the changes from 7193719: Support repeating annotations
in javax.lang.model.
~/repos/jdk8/tl/adder/build/solaris-i586/impsrc/javax/lang/model/element/Element.java:199:
warning - Tag @see: can't find getAnnotation() in
javax.lang.model.element.Element
I only noticed when building javadocs for another issue, thankfully we don't
have many warnings here ;-)
diff -r d54b4a091450 src/share/classes/javax/lang/model/element/Element.java
--- a/src/share/classes/javax/lang/model/element/Element.java Mon Jan 14
14:17:25 2013 -0800
+++ b/src/share/classes/javax/lang/model/element/Element.java Tue Jan 15
17:55:17 2013 +
@@ -188,7 +188,7 @@ public interface Element {
* type if present on this element, else an empty array
*
* @see #getAnnotationMirrors()
- * @see #getAnnotation()
+ * @see #getAnnotation(java.lang.Class)
* @see java.lang.reflect.AnnotatedElement#getAnnotations
* @see EnumConstantNotPresentException
* @see AnnotationTypeMismatchException
-Chris.
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
hg: jdk8/tl/jdk: 8005618: TEST_BUG: java/lang/ProcessBuilder/Basic.java failing intermittently
Changeset: 44d6cabc9a3f Author:robm Date: 2013-01-15 19:58 + URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/44d6cabc9a3f 8005618: TEST_BUG: java/lang/ProcessBuilder/Basic.java failing intermittently Reviewed-by: alanb, martin, dholmes ! test/java/lang/ProcessBuilder/Basic.java
Re: 8006344: Broken javadoc link in javax.lang.model.element.Element
On 15/01/2013 17:59, Chris Hegarty wrote:
Minor oversight in the changes from 7193719: Support repeating
annotations in javax.lang.model.
~/repos/jdk8/tl/adder/build/solaris-i586/impsrc/javax/lang/model/element/Element.java:199:
warning - Tag @see: can't find getAnnotation() in
javax.lang.model.element.Element
I only noticed when building javadocs for another issue, thankfully we
don't have many warnings here ;-)
diff -r d54b4a091450
src/share/classes/javax/lang/model/element/Element.java
--- a/src/share/classes/javax/lang/model/element/Element.java Mon
Jan 14 14:17:25 2013 -0800
+++ b/src/share/classes/javax/lang/model/element/Element.java Tue
Jan 15 17:55:17 2013 +
@@ -188,7 +188,7 @@ public interface Element {
* type if present on this element, else an empty array
*
* @see #getAnnotationMirrors()
- * @see #getAnnotation()
+ * @see #getAnnotation(java.lang.Class)
* @see java.lang.reflect.AnnotatedElement#getAnnotations
* @see EnumConstantNotPresentException
* @see AnnotationTypeMismatchException
-Chris.
Looks good to me too.
-Alan
Re: 8006344: Broken javadoc link in javax.lang.model.element.Element
Thanks for catching this,
Looks good.
cheers
/Joel
On 15 jan 2013, at 18:59, Chris Hegarty chris.hega...@oracle.com wrote:
Minor oversight in the changes from 7193719: Support repeating annotations
in javax.lang.model.
~/repos/jdk8/tl/adder/build/solaris-i586/impsrc/javax/lang/model/element/Element.java:199:
warning - Tag @see: can't find getAnnotation() in
javax.lang.model.element.Element
I only noticed when building javadocs for another issue, thankfully we don't
have many warnings here ;-)
diff -r d54b4a091450 src/share/classes/javax/lang/model/element/Element.java
--- a/src/share/classes/javax/lang/model/element/Element.java Mon Jan 14
14:17:25 2013 -0800
+++ b/src/share/classes/javax/lang/model/element/Element.java Tue Jan 15
17:55:17 2013 +
@@ -188,7 +188,7 @@ public interface Element {
* type if present on this element, else an empty array
*
* @see #getAnnotationMirrors()
- * @see #getAnnotation()
+ * @see #getAnnotation(java.lang.Class)
* @see java.lang.reflect.AnnotatedElement#getAnnotations
* @see EnumConstantNotPresentException
* @see AnnotationTypeMismatchException
-Chris.
hg: jdk8/tl/langtools: 8006344: Broken javadoc link in javax.lang.model.element.Element
Changeset: f805b5e3c9d1 Author:chegar Date: 2013-01-15 20:38 + URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/f805b5e3c9d1 8006344: Broken javadoc link in javax.lang.model.element.Element Reviewed-by: lancea, alanb, jfranck ! src/share/classes/javax/lang/model/element/Element.java
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
On 15/01/2013 16:33, Chris Hegarty wrote: Updated webrev: http://cr.openjdk.java.net/~chegar/8005926/webrev.01/webrev/ Any chance of using SharedSecrets instead of UNSAFE? You know, with Unsafe being like unsafe and everything. Tom
Re: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
Looks great! Little nit: SQLOutputImpl.java line 579 could be dropped. -Ulf Am 15.01.2013 17:48, schrieb Lance Andersen - Oracle: Here is a revision http://cr.openjdk.java.net/~lancea/8006139/webrev.01 I still have to enter the JBS entry for the javadoc clarifications (and I also found another javadoc issue that was due to incorrect cut paste when the code was written) and ccc request As i mentioned in an earlier thread, these classes are hardly ever, if at all used and would only be used when UDTs are used and the majority of databases do not support this. Best lance On Jan 14, 2013, at 5:11 AM, Alan Bateman wrote: On 13/01/2013 23:51, Lance Andersen - Oracle wrote: : One other thing is that the CCE has a side-effect in that it consumes the next attribute. The methods could be changed to peek at the next attribute but that wouldn't work without synchronization or making it clear in the spec that the it is not a thread-safe implementation of SQLInput. I really want to keep the changes to the bare minimum here as this and the other serial classes are hardly, if ever used at all. I understand, but if you add a catch-all in the class description to cover the CCE case then this could be part of the same paragraph. -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: review request 8006139, add missing methods to javax.sql.rowset.serial.SQLInput/OutputImpl
Thank you Ulf. I deleted the extra line on 579 Best Lance On Jan 15, 2013, at 3:45 PM, Ulf Zibis wrote: Looks great! Little nit: SQLOutputImpl.java line 579 could be dropped. -Ulf Am 15.01.2013 17:48, schrieb Lance Andersen - Oracle: Here is a revision http://cr.openjdk.java.net/~lancea/8006139/webrev.01 I still have to enter the JBS entry for the javadoc clarifications (and I also found another javadoc issue that was due to incorrect cut paste when the code was written) and ccc request As i mentioned in an earlier thread, these classes are hardly ever, if at all used and would only be used when UDTs are used and the majority of databases do not support this. Best lance On Jan 14, 2013, at 5:11 AM, Alan Bateman wrote: On 13/01/2013 23:51, Lance Andersen - Oracle wrote: : One other thing is that the CCE has a side-effect in that it consumes the next attribute. The methods could be changed to peek at the next attribute but that wouldn't work without synchronization or making it clear in the spec that the it is not a thread-safe implementation of SQLInput. I really want to keep the changes to the bare minimum here as this and the other serial classes are hardly, if ever used at all. I understand, but if you add a catch-all in the class description to cover the CCE case then this could be part of the same paragraph. -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 Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
hg: jdk8/tl/langtools: 8006224: Doclint NPE for attribute with no value
Changeset: bc1023e0e533 Author:jjg Date: 2013-01-15 13:03 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/bc1023e0e533 8006224: Doclint NPE for attribute with no value Reviewed-by: darcy ! src/share/classes/com/sun/tools/doclint/Checker.java ! src/share/classes/com/sun/tools/doclint/resources/doclint.properties + test/tools/doclint/AnchorTest.java + test/tools/doclint/AnchorTest.out
zip64 compatibility problems
Hi zip64 team! It's always a problem creating zip files using new zip spec features, since older zip implementations will fail to read those files. I see the jdk8 fix to allow the launcher to read zip64 files changeset: 5812:f1838d040cc7 user:ksrini date:2012-09-05 11:38 -0700 7194005: (launcher) needs to be enhanced for 64-bit jar file handling Reviewed-by: darcy, sherman but that has not been backported to jdk7, which seems like a serious problem to me. More generally, most zip implementations, including in the jdk, worked just fine with 64k entries, since the entry count field was generally just treated as a hint. The change to use zip64 when there are 64k entries makes zip files *less* portable in practice for the next few years. It would have been better to default to writing non-zip64 zip files in such cases. Note that zip 3.0 does write zip64 files for 64k entries, but also provides a flag -fz- to change the default. I'd like to see a backport of 7194005 and am also aware of some other issues with our ugly friend parse_manifest.c. You changed calls to open to do this: if ((fd = open(jarfile, O_RDONLY #ifdef O_LARGEFILE | O_LARGEFILE /* large file mode on solaris */ #endif #ifdef O_BINARY | O_BINARY /* use binary mode on windows */ #endif )) == -1) But this is not done consistently - there are 2 other calls to open in the same file that didn't get the LARGEFILE treatment. Why isn't there a JLI_Open? Comments?
Codereview request for 8003680: JSR 310: Date/Time API
Hi, The Threeten project [1] is planned to be integrated into OpenJDK8 M6 milestone. Here is the webrev http://cr.openjdk.java.net/~sherman/8003680/webrev and the latest javadoc http://cr.openjdk.java.net/~sherman/8003680/javadoc Review comments can be sent to the threeten-dev email list [2] and/or core-libs-dev email list[3]. Thanks, Sherman [1] http://openjdk.java.net/projects/threeten [2] threeten-dev @ openjdk.java.net [3] core-libs-dev @ openjdk.java.net
Re: Review Request : Java exe doesn't process args ending Back slash
Hello Jayashree, I see the issue, I have been busy with other things, I will revisit your patch in the coming week. Kumar On 20-12-2012 12:07 AM, Kumar Srinivasan wrote: Hello Jayashree, a. you are referencing a bug which has already been fixed, is there a new one for this ? b. with regards to the fix, I don't quite understand the issue, could you please provide a use case ? c. your regression test does not seem to be accurate it behaves the same with or without your fix, also you will need to provide a C++ test case in cmdtoargs.c as well see the bottom of that file. Thanks Kumar Hi All, Java.exe doesn't seems to process arguments ending with back slashes well , in windows only . I have added test scenario and changeset in the below webrev . http://cr.openjdk.java.net/~jviswana/7188114/webrev.01/ This seems to be introduced after the bug fix for 7188114 has be made into jdk8 . Thanks and Regards, Jayashree Viswanathan Hi Kumar , a. I am referencing an old bug because , that bug fix has caused this regression . b. The use case is when there are backslashes at the end args for a java command using say -Dtest.argEndingInBackslash=a JavaVM args: version 0x00010002, ignoreUnrecognized is JNI_FALSE, nOptions is 5 option[ 0] = '-Dsun.java.launcher.diag=true' option[ 1] = '-Djava.class.path=.' option[ 2] = '-Dtest.argEndingInBackslash=a' option[ 3] = '-Dsun.java.command=TestCmdLineParsing' option[ 4] = '-Dsun.java.launcher=SUN_STANDARD' 74439 micro seconds to InitializeJVM Main class is 'TestCmdLineParsing' App's argc is 0 9182 micro seconds to load main class _JAVA_LAUNCHER_DEBUG value of test.argEndingInBackslash = a c. Sorry , I seem to have missed something , the above test case should help you exhibit the problem . Can you please let me know where to find or add such C++ test cases , as In the test cases bucket I know off is jtreg or JCKs only at the moment . Thanks and Regards, Jayashree Viswanathan
Re: zip64 compatibility problems
Hi Martin, I would assume you are not asking for a similar flag for jar to disable the zip64 support (or even something at API level) to generate old style jar/zip file for 64k entries, but the backport of this changeset to 7u, right? -Sherman On 1/15/13 1:46 PM, Martin Buchholz wrote: Hi zip64 team! It's always a problem creating zip files using new zip spec features, since older zip implementations will fail to read those files. I see the jdk8 fix to allow the launcher to read zip64 files changeset: 5812:f1838d040cc7 user:ksrini date:2012-09-05 11:38 -0700 7194005: (launcher) needs to be enhanced for 64-bit jar file handling Reviewed-by: darcy, sherman but that has not been backported to jdk7, which seems like a serious problem to me. More generally, most zip implementations, including in the jdk, worked just fine with 64k entries, since the entry count field was generally just treated as a hint. The change to use zip64 when there are 64k entries makes zip files *less* portable in practice for the next few years. It would have been better to default to writing non-zip64 zip files in such cases. Note that zip 3.0 does write zip64 files for 64k entries, but also provides a flag -fz- to change the default. I'd like to see a backport of 7194005 and am also aware of some other issues with our ugly friend parse_manifest.c. You changed calls to open to do this: if ((fd = open(jarfile, O_RDONLY #ifdef O_LARGEFILE | O_LARGEFILE /* large file mode on solaris */ #endif #ifdef O_BINARY | O_BINARY /* use binary mode on windows */ #endif )) == -1) But this is not done consistently - there are 2 other calls to open in the same file that didn't get the LARGEFILE treatment. Why isn't there a JLI_Open? Comments?
Re: zip64 compatibility problems
On Tue, Jan 15, 2013 at 5:23 PM, Xueming Shen xueming.s...@oracle.com wrote: Hi Martin, I would assume you are not asking for a similar flag for jar to disable the zip64 support (or even something at API level) to generate old style jar/zip file for 64k entries, but the backport of this changeset to 7u, right? Actually, I think y'all should do 3 things: - backport Kumar's bug fix to jdk7 - introduce a system property to turn off the zip64 support. I am working on a patch to introduce such a system property. We will probably default to disabling zip64 by default, but you will probably find that unacceptable. But you can default the other way. - fix up some remaining largefile issues in parse_manifest I might contribute a bit of code here as well.
Re: zip64 compatibility problems
On 1/15/2013 1:46 PM, Martin Buchholz wrote: Hi zip64 team! It's always a problem creating zip files using new zip spec features, since older zip implementations will fail to read those files. I see the jdk8 fix to allow the launcher to read zip64 files changeset: 5812:f1838d040cc7 user:ksrini date:2012-09-05 11:38 -0700 7194005: (launcher) needs to be enhanced for 64-bit jar file handling Reviewed-by: darcy, sherman but that has not been backported to jdk7, which seems like a serious problem to me. More generally, most zip implementations, including in the jdk, worked just fine with 64k entries, since the entry count field was generally just treated as a hint. The change to use zip64 when there are 64k entries makes zip files *less* portable in practice for the next few years. It would have been better to default to writing non-zip64 zip files in such cases. Note that zip 3.0 does write zip64 files for 64k entries, but also provides a flag -fz- to change the default. I'd like to see a backport of 7194005 and am also aware of some other issues with our ugly friend parse_manifest.c. Joe and I contemplated about this, in the end we decided not to, in order to allow for some soak time in jdk8. You changed calls to open to do this: if ((fd = open(jarfile, O_RDONLY #ifdef O_LARGEFILE | O_LARGEFILE /* large file mode on solaris */ #endif #ifdef O_BINARY | O_BINARY /* use binary mode on windows */ #endif )) == -1) But this is not done consistently - there are 2 other calls to open in the same file that didn't get the LARGEFILE treatment. Why isn't there a JLI_Open? Maybe if you had reviewed my code changes, you would've caught this. :) I will look into it, maybe time for a JLI_Open as you suggested. Kumar Comments?
Re: zip64 compatibility problems
You want code? I got code.
@@ -75,6 +75,18 @@
}
/**
+ * Whether to use ZIP64 for zip files with more than 64k entries.
+ * When ZIP64 support in zip implementations is ubiquitous, this
+ * should default to true, but before then, defaulting to false is
+ * more portable, since most zip implementations tolerate
+ * incorrect total entry count fields.
+ */
+private static final boolean useZip64For64kEntries =
+java.security.AccessController.doPrivileged(
+new sun.security.action.GetBooleanAction(
+java.util.zip.useZip64For64kEntries)).booleanValue();
+
+/**
* Checks to make sure that this stream has not been closed.
*/
private void ensureOpen() throws IOException {
@@ -534,8 +546,10 @@
}
int count = xentries.size();
if (count = ZIP64_MAGICCOUNT) {
-count = ZIP64_MAGICCOUNT;
-hasZip64 = true;
+hasZip64 |= useZip64For64kEntries;
+if (hasZip64) {
+count = ZIP64_MAGICCOUNT;
+}
}
if (hasZip64) {
long off64 = written;
On Tue, Jan 15, 2013 at 5:33 PM, Martin Buchholz marti...@google.com wrote:
On Tue, Jan 15, 2013 at 5:23 PM, Xueming Shen xueming.s...@oracle.com wrote:
Hi Martin,
I would assume you are not asking for a similar flag for jar to disable the
zip64 support (or even something at API level) to generate old style
jar/zip
file for 64k entries, but the backport of this changeset to 7u, right?
Actually, I think y'all should do 3 things:
- backport Kumar's bug fix to jdk7
- introduce a system property to turn off the zip64 support.
I am working on a patch to introduce such a system property.
We will probably default to disabling zip64 by default, but you will
probably find that unacceptable. But you can default the other way.
- fix up some remaining largefile issues in parse_manifest
I might contribute a bit of code here as well.
Re: zip64 compatibility problems
zip3.0 has a flag to do the similar thing may be a compelling reason to add such a system property, but make disabled default turns yourself into the dark side:-) have to explicitly turn the flag on will force the people to think if this is something they really want to do. Given it has been in the major releases for years, I don't think it's possible to disable it in openjdk. -Sherman On 1/15/13 5:33 PM, Martin Buchholz wrote: On Tue, Jan 15, 2013 at 5:23 PM, Xueming Shen xueming.s...@oracle.com wrote: Hi Martin, I would assume you are not asking for a similar flag for jar to disable the zip64 support (or even something at API level) to generate old style jar/zip file for 64k entries, but the backport of this changeset to 7u, right? Actually, I think y'all should do 3 things: - backport Kumar's bug fix to jdk7 - introduce a system property to turn off the zip64 support. I am working on a patch to introduce such a system property. We will probably default to disabling zip64 by default, but you will probably find that unacceptable. But you can default the other way. - fix up some remaining largefile issues in parse_manifest I might contribute a bit of code here as well.
Re: zip64 compatibility problems
On 1/15/13 6:21 PM, Xueming Shen wrote: zip3.0 has a flag to do the similar thing may be a compelling reason to add such a system property, but make disabled default turns yourself into the dark side:-) have to explicitly turn the flag on will force the people to think if this is something they really want to do. Given it has been in the major releases for years, I don't think it's possible to disable it in openjdk. I meant it's impossible to disable it by default...
Re: zip64 compatibility problems
On Tue, Jan 15, 2013 at 6:24 PM, Xueming Shen xueming.s...@oracle.com wrote: On 1/15/13 6:21 PM, Xueming Shen wrote: zip3.0 has a flag to do the similar thing may be a compelling reason to add such a system property, but make disabled default turns yourself into the dark side:-) have to explicitly turn the flag on will force the people to think if this is something they really want to do. Given it has been in the major releases for years, I don't think it's possible to disable it in openjdk. I meant it's impossible to disable it by default... I agree with you that it's the right decision for OpenJDK proper. You are consistent with zip 3.0 and also are obeying the zip spec 4.4.1.4 If one of the fields in the end of central directory record is too small to hold required data, the field should be set to -1 (0x or 0x) and the ZIP64 format record should be created. It's a different story for the groups of users just now adopting jdk7. The situation is a bit grim for those using jar files with 64k entries. jdk7 is not only creating jar files that are unreadable by jdk6 - they are also unreadable by the jdk7 launcher! With no visible benefit for the user! So temporarily turning off zip64 seems a better choice for us.
Re: Review Request : Java exe doesn't process args ending Back slash
On 16-01-2013 6:26 AM, Kumar Srinivasan wrote: Hello Jayashree, I see the issue, I have been busy with other things, I will revisit your patch in the coming week. Kumar On 20-12-2012 12:07 AM, Kumar Srinivasan wrote: Hello Jayashree, a. you are referencing a bug which has already been fixed, is there a new one for this ? b. with regards to the fix, I don't quite understand the issue, could you please provide a use case ? c. your regression test does not seem to be accurate it behaves the same with or without your fix, also you will need to provide a C++ test case in cmdtoargs.c as well see the bottom of that file. Thanks Kumar Hi All, Java.exe doesn't seems to process arguments ending with back slashes well , in windows only . I have added test scenario and changeset in the below webrev . http://cr.openjdk.java.net/~jviswana/7188114/webrev.01/ This seems to be introduced after the bug fix for 7188114 has be made into jdk8 . Thanks and Regards, Jayashree Viswanathan Hi Kumar , a. I am referencing an old bug because , that bug fix has caused this regression . b. The use case is when there are backslashes at the end args for a java command using say -Dtest.argEndingInBackslash=a JavaVM args: version 0x00010002, ignoreUnrecognized is JNI_FALSE, nOptions is 5 option[ 0] = '-Dsun.java.launcher.diag=true' option[ 1] = '-Djava.class.path=.' option[ 2] = '-Dtest.argEndingInBackslash=a' option[ 3] = '-Dsun.java.command=TestCmdLineParsing' option[ 4] = '-Dsun.java.launcher=SUN_STANDARD' 74439 micro seconds to InitializeJVM Main class is 'TestCmdLineParsing' App's argc is 0 9182 micro seconds to load main class _JAVA_LAUNCHER_DEBUG value of test.argEndingInBackslash = a c. Sorry , I seem to have missed something , the above test case should help you exhibit the problem . Can you please let me know where to find or add such C++ test cases , as In the test cases bucket I know off is jtreg or JCKs only at the moment . Thanks and Regards, Jayashree Viswanathan Thanks for the response ! Regards, Jayashree Viswanathan
Re: JDK 8 code review request for 8005298 Add FunctionalInterface type to the core libraries
Hi Florian, On 1/10/2013 1:23 AM, Florian Weimer wrote: On 01/08/2013 10:24 PM, Joe Darcy wrote: As discussed over on one of the Project Lambda lists [1], we're adding an interface type to the platform to explicitly mark interface types as being functional interfaces suitable for use in lambda expressions. Please review the addition of this new type: http://cr.openjdk.java.net/~darcy/8005298.0/ Shouldn't this annotation have @Retention(RetentionPolicy.SOURCE), similar to the @Override annotation? No; we intentionally made this annotation have runtime retention to allow it to also be queried to various tools at runtime, etc. Cheers, -Joe
Re: RFR (S): CR 8005926: (thread) Merge ThreadLocalRandom state into java.lang.Thread
Just a minor note: In writeObject(), the code writes constant 0L for field rnd. If we're trying to emulate the stream of JDK7 TLR then writeObject() could do better by writing the value of Thread.currentThread().threadLocalRandomSeed field... Regards, Peter On 01/15/2013 05:33 PM, Chris Hegarty wrote: Updated webrev: http://cr.openjdk.java.net/~chegar/8005926/webrev.01/webrev/ * based on Aleksey initial webrev * applied serialPersistentFields, writeObject, readResolve ( as suggested by Heinz ) * did not apply readObject. It is unnecessary, defaultReadObject will read all the fields off the stream readResolve is necessary to give us good behavior ( as noted previously ). Once integrated, I will file a new bug to track the possible change of serialized form of TLR, and this can then remove serialPersistentFields and writeObject, if successful. -Chris. On 01/15/2013 01:57 PM, Alan Bateman wrote: On 15/01/2013 13:49, Chris Hegarty wrote: But wouldn't there be an issue with JDK7 TLR streams being deserialized by JDK8, pad fields would be left in the Object input steam? If so, then we're down the road of having to have a readObject, etc... and possibly back to serialPersistentFields? Technically deleting fields is an incompatible change as the stream will not contain the fields. If someone is crazy enough to serialize with jdk8 TLR and send it to a jdk7 release then the fields will have their default value (as the values aren't in the stream). As the fields aren't used then I don't think this should make a difference. Going the other way shouldn't be an issue either as the pad fields will be ignored. -Alan.
