Re: RFR(JDK12/NIO) 8209576: java.nio.file.Files.writeString writes garbled UTF-16 instead of UTF-8

[Xueming Shen]

+1

On 08/17/2018 02:39 PM, Joe Wang wrote:

Hi,

Please review a fix for a condition where a code conversion was skipped 
incorrectly resulting in a wrong byte array being written into the file.

JBS: https://bugs.openjdk.java.net/browse/JDK-8209576
webrevs: http://cr.openjdk.java.net/~joehw/jdk12/8209576/webrev/

Thanks,
Joe

RFR(JDK12/NIO) 8209576: java.nio.file.Files.writeString writes garbled UTF-16 instead of UTF-8

[Joe Wang]

Hi,

Please review a fix for a condition where a code conversion was skipped 
incorrectly resulting in a wrong byte array being written into the file.


JBS: https://bugs.openjdk.java.net/browse/JDK-8209576
webrevs: http://cr.openjdk.java.net/~joehw/jdk12/8209576/webrev/

Thanks,
Joe

Re: Moderator Responsibilities

[Jonathan Bluett-Duncan]

Hi Max (I only just realised that you gave your first name in your original
email!),

I'm still unclear on what you meant earlier by "no more kid games", and
also what you mean by "I am not here to adjust maturity in channel.  Agree
to get along here like adults is what". Did any earlier communications you
had on one of the OpenJDK mailing lists leave you disappointed or
something? 樂

Kind regards,
Jonathan

On Fri, 17 Aug 2018, 20:38 mr rupplin,  wrote:

> Great.  I'll check out the wiki.  I am not here to adjust maturity in
> channel.  Agree to get along here like adults is what.
>
> Ok.
>
>
>
> Get Outlook for Android 
>
> --
> *From:* Jonathan Bluett-Duncan 
> *Sent:* Friday, August 17, 2018 3:11:22 PM
> *To:* mea...@outlook.com
> *Cc:* core-libs-dev
> *Subject:* Re: Moderator Responsibilities
>
> Hi Mr Rupplin,
>
> I can't speak for anyone else, but I'm personally puzzled by your email.
> Could you clarify what you mean by "There will be no more kid games. If you
> are asked, please answer fully and promptly.  Debugging a long compile
> stack can be complicated."? I ask because it came across as antagonistic to
> me, but I'm sure didn't mean to!
>
> Do not the OpenJDK wiki  or the Adoption
> group's "New Contributor" page
>  give you
> the information you need to work with the OpenJDK sources? If not, is there
> any additional info that you think someone with relevant permissions could
> add to the wiki to make working with the sources easier?
>
> Cheers,
> Jonathan
>
> On Fri, 17 Aug 2018 at 18:41, mr rupplin  wrote:
>
>> Hello.
>>
>>
>> I have been contacted by someone who has stated that there are complaints
>> that we ask the channel about debug questions.  I have 12+ years in
>> Software and a Masters Degree in Computer Science.
>>
>>
>> There will be no more kid games. If you are asked, please answer fully
>> and promptly.  Debugging a long compile stack can be complicated.
>>
>>
>> Also, if recurring questions become irksome make a JDK wiki on github.
>> This is the least we can expect.
>>
>>
>> That is all.
>>
>>
>> Thank you for your understanding.
>>
>>
>> Max R.
>>
>>
>> (Sr. Software Fellow)
>>
>

Re: Moderator Responsibilities

[Jonathan Bluett-Duncan]

Hi Mr Rupplin,

I can't speak for anyone else, but I'm personally puzzled by your email.
Could you clarify what you mean by "There will be no more kid games. If you
are asked, please answer fully and promptly.  Debugging a long compile
stack can be complicated."? I ask because it came across as antagonistic to
me, but I'm sure didn't mean to!

Do not the OpenJDK wiki  or the Adoption
group's "New Contributor" page
 give you
the information you need to work with the OpenJDK sources? If not, is there
any additional info that you think someone with relevant permissions could
add to the wiki to make working with the sources easier?

Cheers,
Jonathan

On Fri, 17 Aug 2018 at 18:41, mr rupplin  wrote:

> Hello.
>
>
> I have been contacted by someone who has stated that there are complaints
> that we ask the channel about debug questions.  I have 12+ years in
> Software and a Masters Degree in Computer Science.
>
>
> There will be no more kid games. If you are asked, please answer fully and
> promptly.  Debugging a long compile stack can be complicated.
>
>
> Also, if recurring questions become irksome make a JDK wiki on github.
> This is the least we can expect.
>
>
> That is all.
>
>
> Thank you for your understanding.
>
>
> Max R.
>
>
> (Sr. Software Fellow)
>

Re: Moderator Responsibilities

[joe darcy]

This list is for discussions related to the development of the JDK's 
core libraries.


It is not a no-cost support or engineering services offering and should 
be treated accordingly.


-Joe

Moderator Responsibilities

[mr rupplin]

Hello.


I have been contacted by someone who has stated that there are complaints that 
we ask the channel about debug questions.  I have 12+ years in Software and a 
Masters Degree in Computer Science.


There will be no more kid games. If you are asked, please answer fully and 
promptly.  Debugging a long compile stack can be complicated.


Also, if recurring questions become irksome make a JDK wiki on github.  This is 
the least we can expect.


That is all.


Thank you for your understanding.


Max R.


(Sr. Software Fellow)

Re: Is returning a value != '0' or '1' as jboolean from a JNI function legal?

[Xueming Shen]

On 08/17/2018 10:25 AM, Aleksey Shipilev wrote:

On 08/17/2018 05:12 PM, Volker Simonis wrote:

The offending code in Console_md.c looks as follows:

#define ECHO8

JNIEXPORT jboolean JNICALL
Java_java_io_Console_echo(...) {

 jboolean old;
 ...
 old = (tio.c_lflag&  ECHO);
 ...
 return old;
}

The intention of this code is to return "true" if the ECHO flag was
set but it really returns the value of ECHO (which is defined as '8'
in a system header).

The question now is, if a Java SE compatible VM guarantees that any
arbitrary, non-zero valued jboolean will be interpreted as "JNI_TRUE"
and only a zero valued jboolean will be interpreted as "JNI_FALSE"?

Or, the other way round, is the normalization performed by the HotSpot
result handlers necessary (i.e. enforced by the specification) or just
a convenience to fix broken code like the above from Console.echo()?

I think this is intentional aftermath of boolean value normalization:
   https://bugs.openjdk.java.net/browse/JDK-8161720

So, Java_java_io_Console_echo looks broken and needs to be fixed, by e.g.:

-   old = (tio.c_lflag&  ECHO);
+   old = (tio.c_lflag&  ECHO) != 0;

Thanks,
-Aleksey



Yes, the code in Console_md.c need/will be updated to fix this issue. We planed 
and
tried in early circle of jdk11 when working on 8194750, at least for the newly 
added
code.  But change had been backed out for other reason. Will fix this specific 
one in
12. But the existing code is about decade old, I would assume you probably 
still need
to fix the s390x part as a "regression".

Thanks!
Sherman

Re: Is returning a value != '0' or '1' as jboolean from a JNI function legal?

[Aleksey Shipilev]

On 08/17/2018 05:12 PM, Volker Simonis wrote:
> The offending code in Console_md.c looks as follows:
> 
> #define ECHO8
> 
> JNIEXPORT jboolean JNICALL
> Java_java_io_Console_echo(...) {
> 
> jboolean old;
> ...
> old = (tio.c_lflag & ECHO);
> ...
> return old;
> }
> 
> The intention of this code is to return "true" if the ECHO flag was
> set but it really returns the value of ECHO (which is defined as '8'
> in a system header).
> 
> The question now is, if a Java SE compatible VM guarantees that any
> arbitrary, non-zero valued jboolean will be interpreted as "JNI_TRUE"
> and only a zero valued jboolean will be interpreted as "JNI_FALSE"?
> 
> Or, the other way round, is the normalization performed by the HotSpot
> result handlers necessary (i.e. enforced by the specification) or just
> a convenience to fix broken code like the above from Console.echo()?

I think this is intentional aftermath of boolean value normalization:
  https://bugs.openjdk.java.net/browse/JDK-8161720

So, Java_java_io_Console_echo looks broken and needs to be fixed, by e.g.:

-   old = (tio.c_lflag & ECHO);
+   old = (tio.c_lflag & ECHO) != 0;

Thanks,
-Aleksey

Re: Is returning a value != '0' or '1' as jboolean from a JNI function legal?

[Roger Riggs]

Hi Volker,

This may be more of a hotspot question than core-libs.

This seems like a practical feature of the implementation and a convenience
to match the conventions of C/C++ with JNI.

It might be worth adding a test to verify the assumptions between Java 
and Native.


$.02, Roger


On 8/17/18 11:12 AM, Volker Simonis wrote:

Hi,

we've recently discovered a problem with Console.readPassword() which
failed to restore the correct echo mode on Linux/s390x [1]. We could
finally track this down to a "bug" [2] in the s390x template
interpreter which failed to call the "result handler" after a native
call. For jboolean return types, the "result handler" merges all
values > 0 into 1 (i.e. JNI_TRUE). Not calling the result handler
after returning from native calls resulted in cases where non-zero
jboolean values returned from native functions where interpreted as
zero (i.e. JNI_FALSE).

The offending code in Console_md.c looks as follows:

#define ECHO8

JNIEXPORT jboolean JNICALL
Java_java_io_Console_echo(...) {

 jboolean old;
 ...
 old = (tio.c_lflag & ECHO);
 ...
 return old;
}

The intention of this code is to return "true" if the ECHO flag was
set but it really returns the value of ECHO (which is defined as '8'
in a system header).

The question now is, if a Java SE compatible VM guarantees that any
arbitrary, non-zero valued jboolean will be interpreted as "JNI_TRUE"
and only a zero valued jboolean will be interpreted as "JNI_FALSE"?

Or, the other way round, is the normalization performed by the HotSpot
result handlers necessary (i.e. enforced by the specification) or just
a convenience to fix broken code like the above from Console.echo()?

All I could find from the Specification so far is the following, which
seems to suggest that only JNI_TRUE/JNI_FASLE are valid values for a
jboolean, but I'm not completely sure...

From: JNI Types and Data Structures [3]

Java "boolean" == JNI "jboolean" == unsigned 8 bits

The following definition is provided for convenience.

#define JNI_FALSE  0
#define JNI_TRUE   1

From: JVMLS, § 2.3.4. The boolean Type [4]

Although the Java Virtual Machine defines a boolean type, it only
provides very limited support for it. There are no Java Virtual
Machine instructions solely dedicated to operations on boolean values.
Instead, expressions in the Java programming language that operate on
boolean values are compiled to use values of the Java Virtual Machine
int data type.

The Java Virtual Machine encodes boolean array components using 1 to
represent true and 0 to represent false. Where Java programming
language boolean values are mapped by compilers to values of Java
Virtual Machine type int, the compilers must use the same encoding.

Thank you and best regards,
Volker

[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-July/thread.html#54479
[2] https://bugs.openjdk.java.net/browse/JDK-8209637
[3] https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/types.html
[4] https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-2.html#jvms-2.3.4

Re: RFR: 8209633: Avoid creating WeakEntry wrappers when looking up cached MethodType

[mandy chung]

On 8/17/18 7:38 AM, Peter Levart wrote:



On 08/17/2018 04:32 PM, Claes Redestad wrote:

Hi Peter,

On 2018-08-17 16:04, Peter Levart wrote:

Hi Claes,

Nice trick.


+1

You made MethodType(s) and WeakEntry(s) holding them equal, 
respecting the equals()/hashCode() contract. When WeakEntry looses 
the referent it is left equal to itself only, which is enough for 
expunging it from map.


Good summary. Do you think we need a few comments to spell out these 
intricacies?


Yes, perhaps just pointers from the two equals() methods to each other, 
explaining that they are actually one method which is split into two 
unrelated classes.


It'd be worth to document this as javadoc of these two equals methods
(that's probably what you are thinking).

Mandy

Re: RFR JDK-8209553: ExceptionInInitializerError can have a default detail message if the cause is given

[mandy chung]

Hi Peter, Jason, Joe,

Thanks for the pointers.  I have missed the use of
serialPersistentFields (thanks to Peter for calling this out).  I read
the javadoc and serialization spec but that didn't come clearly to me.
It'd be good to include an example in the javadoc (will file a JBS issue).

W.r.t. the package-private API to set the cause in EIIE [1], yes it's
needed and it's done in webrev.01.  Let me redo the
readObject/writeObject part and send a new webrev.

Mandy
http://cr.openjdk.java.net/~mchung/jdk12/webrevs/8209553/webrev.01/src/java.base/share/classes/java/lang/Throwable.java.sdiff.html

On 8/17/18 8:05 AM, Jason Mehrens wrote:

Hi Mandy,

This what we were doing in the past and has examples of removing
fields with expected tests: 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051339.html

http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/017594.html >
However, this case is a little different because it actually
disallows subsequent initCause.  I would assume that will get tricky
if you deserialize the old binary form in a newer JDK because we
would have to jump through some hoops to update null to the cause
exception.

Jason

Is returning a value != '0' or '1' as jboolean from a JNI function legal?

[Volker Simonis]

Hi,

we've recently discovered a problem with Console.readPassword() which
failed to restore the correct echo mode on Linux/s390x [1]. We could
finally track this down to a "bug" [2] in the s390x template
interpreter which failed to call the "result handler" after a native
call. For jboolean return types, the "result handler" merges all
values > 0 into 1 (i.e. JNI_TRUE). Not calling the result handler
after returning from native calls resulted in cases where non-zero
jboolean values returned from native functions where interpreted as
zero (i.e. JNI_FALSE).

The offending code in Console_md.c looks as follows:

#define ECHO8

JNIEXPORT jboolean JNICALL
Java_java_io_Console_echo(...) {

jboolean old;
...
old = (tio.c_lflag & ECHO);
...
return old;
}

The intention of this code is to return "true" if the ECHO flag was
set but it really returns the value of ECHO (which is defined as '8'
in a system header).

The question now is, if a Java SE compatible VM guarantees that any
arbitrary, non-zero valued jboolean will be interpreted as "JNI_TRUE"
and only a zero valued jboolean will be interpreted as "JNI_FALSE"?

Or, the other way round, is the normalization performed by the HotSpot
result handlers necessary (i.e. enforced by the specification) or just
a convenience to fix broken code like the above from Console.echo()?

All I could find from the Specification so far is the following, which
seems to suggest that only JNI_TRUE/JNI_FASLE are valid values for a
jboolean, but I'm not completely sure...

From: JNI Types and Data Structures [3]

Java "boolean" == JNI "jboolean" == unsigned 8 bits

The following definition is provided for convenience.

#define JNI_FALSE  0
#define JNI_TRUE   1

From: JVMLS, § 2.3.4. The boolean Type [4]

Although the Java Virtual Machine defines a boolean type, it only
provides very limited support for it. There are no Java Virtual
Machine instructions solely dedicated to operations on boolean values.
Instead, expressions in the Java programming language that operate on
boolean values are compiled to use values of the Java Virtual Machine
int data type.

The Java Virtual Machine encodes boolean array components using 1 to
represent true and 0 to represent false. Where Java programming
language boolean values are mapped by compilers to values of Java
Virtual Machine type int, the compilers must use the same encoding.

Thank you and best regards,
Volker

[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-July/thread.html#54479
[2] https://bugs.openjdk.java.net/browse/JDK-8209637
[3] https://docs.oracle.com/javase/7/docs/technotes/guides/jni/spec/types.html
[4] https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-2.html#jvms-2.3.4

Re: Arrays.asList returned List - Its modifying/write-through semantics

[Jaikiran Pai]

Thank you Roger and Stuart for the detailed reply. Comments inline.


On 17/08/18 7:27 AM, Stuart Marks wrote:
>  
>
> Note that while this is changing javadoc and doc comments the task is
> quite a bit more rigorous than merely updating documentation. The
> javadoc for public Java SE APIs is a *specification* and requires very
> precise language. It's not unusual for what are apparently small
> changes to go through several rounds of review. (Sorry if you already
> know this, but I wanted to make sure.)
I understand :) I decided to pick this up since it's been around for a
few years now and I don't think anyone is in a hurry to have this done.
So this will give me enough time to understand the contribution process
as well as go through as many review rounds as necessary to get it right.

>
> Whether this requires a CSR depends on whether any normative text of
> the specification is changed. A simple clarification ("API note") can
> be added without a CSR. As the wording stands now, however, it seems
> like there is a mixture of normative and informative statements in the
> text; these should be teased apart and the informative statements
> placed into an API note. In addition, the normative text could
> probably be reworded to be more clear. (See my comments in the bug.)
> Such changes would probably need a CSR, even though they wouldn't
> actually change the intent of the specification.
>
> If you're still with me, then dive right in! It'll probably be easiest
> to exchange drafts on the email list. 
I'll come up with an initial version of the change (based on your inputs
that I see in the JIRA) and send it out to this mailing list for review,
in the coming days.

Thanks again for the detailed reply.

-Jaikiran

Re: RFR JDK-8209553: ExceptionInInitializerError can have a default detail message if the cause is given

[Jason Mehrens]

Hi Mandy,

This what we were doing in the past and has examples of removing fields with 
expected tests:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-February/051339.html
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/017594.html

However, this case is a little different because it actually disallows 
subsequent initCause.  I would assume that will get tricky if you deserialize 
the old binary form in a newer JDK because we would have to jump through some 
hoops to update null to the cause exception.

Jason


From: core-libs-dev  on behalf of mandy 
chung 
Sent: Thursday, August 16, 2018 6:54 PM
To: joe darcy; Peter Levart
Cc: core-libs-dev
Subject: Re: RFR JDK-8209553: ExceptionInInitializerError can have a default 
detail message if the cause is given



On 8/16/18 4:48 PM, joe darcy wrote:
>>> Just a question. Why does "private Throwable exception" field in
>>> ExceptionInInitializerError exist? Was it there before there was a
>>> "cause" in Throwable and later still remained there because of
>>> serialization format? Would it be possible to "simulate" its effect
>>> for serialization using "serialPersistentFields" and
>>> ObjectOutputStream.PutField?
>>
>> Thanks for asking.  I meant to mention this and it'd be nice to
>> follow up this in a separate issue.
>>
>> The private exception field exists since 1.1 and kept there for
>> serialization.  getException in existing releases returns the
>> exception field.  I can't think of any way to remove the exception
>> field in JDK n to deserialize it with older JDK x unless JDK x was
>> changed to write the exception field with the cause or getException
>> to return cause.
>
> Just a quick comment, it is possible, although a bit tedious, to remove
> a field and retain serial compatibility if readObject/writeObject
> methods are added to the new version of the class. There are a few
> examples of doing this kind of conversion in the JDK, such as for
> BigInteger.


Thanks Joe.  In EIIE case, ideally we should remove the private
exception field and then write that to the serialize stream.
However, PutField::put requires the field to be present in
the current class; otherwise it throws IAE.

ObjectOutputStream.PutField fields = s.putFields();
fields.put("exception", getCause());

I haven't digged the history but I assume a field in BigInteger
was not renamed/removed.

Any other suggestion would be appreciated.

Mandy

Re: RFR: 8209633: Avoid creating WeakEntry wrappers when looking up cached MethodType

[Peter Levart]

On 08/17/2018 04:32 PM, Claes Redestad wrote:

Hi Peter,

On 2018-08-17 16:04, Peter Levart wrote:

Hi Claes,

Nice trick.


thanks!

You made MethodType(s) and WeakEntry(s) holding them equal, 
respecting the equals()/hashCode() contract. When WeakEntry looses 
the referent it is left equal to itself only, which is enough for 
expunging it from map.


Good summary. Do you think we need a few comments to spell out these 
intricacies?


Yes, perhaps just pointers from the two equals() methods to each other, 
explaining that they are actually one method which is split into two 
unrelated classes.


Peter

Re: RFR: 8209633: Avoid creating WeakEntry wrappers when looking up cached MethodType

[Claes Redestad]

Hi Peter,

On 2018-08-17 16:04, Peter Levart wrote:

Hi Claes,

Nice trick.


thanks!

You made MethodType(s) and WeakEntry(s) holding them equal, respecting 
the equals()/hashCode() contract. When WeakEntry looses the referent 
it is left equal to itself only, which is enough for expunging it from 
map.


Good summary. Do you think we need a few comments to spell out these 
intricacies?


/Claes

Re: RFR: 8209633: Avoid creating WeakEntry wrappers when looking up cached MethodType

[Peter Levart]

Hi Claes,

Nice trick. You made MethodType(s) and WeakEntry(s) holding them equal, 
respecting the equals()/hashCode() contract. When WeakEntry looses the 
referent it is left equal to itself only, which is enough for expunging 
it from map.


Regards, Peter

On 08/17/2018 12:56 PM, Claes Redestad wrote:

Hi,

a small improvement can be seen on startup tests that exercise lambdas 
and ISC if we change the interning mechanism of MethodTypes slightly 
so that lookup can be done without wrapping the lookup key in a 
WeakEntry.


Bug: https://bugs.openjdk.java.net/browse/JDK-8209633
Webrev: http://cr.openjdk.java.net/~redestad/8209633/open.00/

Testing: jdk-tier1+jdk-tier2

Thanks!

/Claes

Re: RFR JDK-8209553: ExceptionInInitializerError can have a default detail message if the cause is given

[Roger Riggs]

Hi David,

Exceptions should have a detail message that is useful.
For EIIE, all of the salient information is in the message of the cause and
is much more useful that something generic that just echos the EIIE's type.
Without a stacktrace EIIE, with a generic message is nearly useless.

Generally, where an exception is caught just to be wrapped and 
propagated with a
different type, the message from the underlying cause is more useful to 
report.


$.02, Roger

On 8/16/18 6:22 PM, David Holmes wrote:

On 17/08/2018 1:18 AM, mandy chung wrote:



On 8/15/18 11:00 PM, David Holmes wrote:

Hi Mandy,

Not a review - sorry :)

On 16/08/2018 7:46 AM, mandy chung wrote:
ExceptionInInitializerError(Throwable cause) sets the detail 
message to null.  It'd be helpful to include a detail message 
rather than null

as in Throwable(Throwable cause) that constructs a new throwable
with a default message cause==null ? null : cause.toString().

This would help troubleshooting of writing new bootstrap methods
where VM currently asserts non-null linkage error message [1] where
there are other solutions such as constructing the message in
the VM or removing the assert.


I think that's just a bug in the VM code making inappropriate 
assumptions! There shouldn't have to be a detail message, especially 
in a wrapping exception like ExceptionInInitializerError! It only 
gets thrown because some other exception was thrown, there's really 
no more detail for the EIIE itself. Promoting the cause's detail 
message to be the detail message of the EIIE just seems wrong to me.


I think even VM removes the assert, I don't see how EIIE includes
the detail message is wrong?  It's consistent with the
Throwable(Throwable cause) constructor.


I think it is wrong because the message is not that of the EIIE but 
that of the original exception.


If EIIE must have a detail message then that should just be something 
explicit like:


Caused by  : 

Unlike general Throwables that may or may not have a cause, a true 
EIIE must always have a cause when thrown by the JVM.


David
-

Did this come about through the new code that saves the original 
cause of the EIIE so that it can be reported when the subsequent 
NoClassDefFoundError is thrown due to the class being in the 
erroneous state? (I don't recall if that actually got pushed.)


I am not sure but I don't think so.  I ran into this when I develop
the bootstrap methods.  I think this is a good EIIE clean up.

Mandy

RFR: 8209633: Avoid creating WeakEntry wrappers when looking up cached MethodType

[Claes Redestad]

Hi,

a small improvement can be seen on startup tests that exercise lambdas 
and ISC if we change the interning mechanism of MethodTypes slightly so 
that lookup can be done without wrapping the lookup key in a WeakEntry.


Bug: https://bugs.openjdk.java.net/browse/JDK-8209633
Webrev: http://cr.openjdk.java.net/~redestad/8209633/open.00/

Testing: jdk-tier1+jdk-tier2

Thanks!

/Claes

Re: RFR 8209184: JDK8 ResourceBundle vulnerable to GC (fix included)

[Adam Farley8]

Hi Mandy,

The extra space is fine.

In hindsight, you could probably rename "loaderone" to just "loader" too.

Thanks for helping with this. :)

Best Regards

Adam Farley 
OpenJDK Team 
Runtimes 
IBM 


mandy chung  wrote on 16/08/2018 18:57:01:

> From: mandy chung 
> To: Adam Farley8 
> Cc: core-libs-dev , Hans Boehm 
> , i18n-...@openjdk.java.net
> Date: 16/08/2018 18:57
> Subject: Re: RFR 8209184: JDK8 ResourceBundle vulnerable to GC (fix 
included)
> 
> 
> 
> On 8/16/18 2:16 AM, Adam Farley8 wrote:
> > Hi Mandy,
> > 
> > I request that you review this for 8u.
> > 
> > --
> > @@ -1398,10 +1398,18 @@
> >   bundle = baseBundle;
> >   }
> > 
> > +keepAlive(loader);
> >   return bundle;
> >   }
> > 
> >   /**
> > + * Keeps the argument ClassLoader alive.
> > + */
> > +private static void keepAlive(ClassLoader loaderone){
> > +//Do nothing.
> > +}
> > +
> > +/**
> >* Checks if the given List is not null, not empty,
> >* not having null in its elements.
> >*/
> > --
> 
> Looks fine. Nit: a space before "Do nothing" comment would be good.
> No need for a new diff.
> 
> Mandy
> 

Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

Re: RFR JDK-8209553: ExceptionInInitializerError can have a default detail message if the cause is given

[Peter Levart]

On 08/17/2018 01:54 AM, mandy chung wrote:



On 8/16/18 4:48 PM, joe darcy wrote:
Just a question. Why does "private Throwable exception" field in 
ExceptionInInitializerError exist? Was it there before there was a 
"cause" in Throwable and later still remained there because of 
serialization format? Would it be possible to "simulate" its effect 
for serialization using "serialPersistentFields" and 
ObjectOutputStream.PutField?


Thanks for asking.  I meant to mention this and it'd be nice to
follow up this in a separate issue.

The private exception field exists since 1.1 and kept there for
serialization.  getException in existing releases returns the
exception field.  I can't think of any way to remove the exception
field in JDK n to deserialize it with older JDK x unless JDK x was
changed to write the exception field with the cause or getException
to return cause.


Just a quick comment, it is possible, although a bit tedious, to 
remove a field and retain serial compatibility if 
readObject/writeObject methods are added to the new version of the 
class. There are a few examples of doing this kind of conversion in 
the JDK, such as for BigInteger.



Thanks Joe.  In EIIE case, ideally we should remove the private
exception field and then write that to the serialize stream.
However, PutField::put requires the field to be present in
the current class; otherwise it throws IAE.

   ObjectOutputStream.PutField fields = s.putFields();
   fields.put("exception", getCause());


Not necessarily. Check ConcurrentHashMap for example. It contains 
several "pseudo" fields, declared with the "serialPersistentFields" 
static final field.


The only problem with this approach is that while deserializing the 
Throwable part of the ExceptionInInitializerError from the serial stream 
produced by an old version of EIIE, the cause field will already be 
deserialized as null and later in ExceptionInInitializerError.readObject 
when "exception" pseudo field is read-in, its value would have to be 
written over Throwable.cause field, but public API will not allow that 
(re-initialization of cause is not permitted), so some package-private 
API would have to be used to overwrite "cause".


But this same problem is present even if you keep the physical 
"exception" field in the ExceptionInInitializerError. You have to be 
prepared to deserialize an old version of EIIE where the "cause" is null 
and the "exception" is non-null.


Regards, Peter



I haven't digged the history but I assume a field in BigInteger
was not renamed/removed.

Any other suggestion would be appreciated.

Mandy