Re: [11] RFR JDK-8198249: Remove deprecated Runtime::runFinalizersOnExit and System::runFinalizersOnExit

[David Holmes]

Hi Mandy,

On 22/02/2018 12:28 PM, mandy chung wrote:

On 2/21/18 6:08 PM, David Holmes wrote:

Hi Mandy,

tl;dr: I think this is now good to go.

On 22/02/2018 5:58 AM, mandy chung wrote:

Here is the updated webrev:
http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198249/webrev.04/

I added some comments to clarify the implementation.


src/java.base/share/classes/java/lang/Shutdown.java

 96 if (VM.isShutdown()|| slot <= currentRunningHook)

Nit: need space after ()




Fixed.


 113 private static void runHooks() {
 114 synchronized (lock) {
 115 /* Guard against the possibility of a daemon thread 
invoking exit

 116  * after DestroyJavaVM initiates the shutdown sequence
 117  */
 118 if (VM.isShutdown()) return;
 119 }

I think this is actually impossible to hit, but that's a separate 
cleanup.


When T1 is calling runHooks, T2 calls exit and blocks on Shutdown.class, 
once T1 releases the lock if VM does not halt yet, T2 will enter this 
method.


Ah! T1 is executing destroyJavaVM - and of course releases the lock 
eventually. Got it.




 139 // set shutdown state
 140 VM.shutdown();

Just an observation that we consider "shutdown" to be after all system 
shutdown hooks are run. So this (contrary to what I wrote in the CSR) 
narrows the window in which a concurrent exit(-1) would trigger an 
immediate halt rather than a hang. Which in turn strengthens the 
argument for just dropping that behaviour.


If an application hook calls exit(0), it will hang (not the thread 
holding the Shutdown class lock).  Do you want to file an issue to track 
that?


If an application hook calls exit(0) it will block on the Shutdown.class 
lock which then hangs the whole shutdown process as the thread holding 
that lock is doing a join() on the hook thread - logical deadlock.


But that wasn't my point at all. I just think this whole "calling exit 
with non-zero causes immediate halt" logic is meaningless now there are 
no finalizers running (and potentially calling exit).


Cheers,
David
---



---

src/java.base/share/classes/jdk/internal/misc/VM.java

 103 public static void shutdown() {
 104 initLevel(SYSTEM_SHUTDOWN);
 105 }

Doing this is exactly what I meant by my previous comments.



Good.

Thanks for the thorough review.
Mandy

Re: [11] RFR JDK-8198249: Remove deprecated Runtime::runFinalizersOnExit and System::runFinalizersOnExit

[mandy chung]

On 2/21/18 6:08 PM, David Holmes wrote:

Hi Mandy,

tl;dr: I think this is now good to go.

On 22/02/2018 5:58 AM, mandy chung wrote:

Here is the updated webrev:
http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198249/webrev.04/

I added some comments to clarify the implementation.


src/java.base/share/classes/java/lang/Shutdown.java

 96 if (VM.isShutdown()|| slot <= currentRunningHook)

Nit: need space after ()




Fixed.


 113 private static void runHooks() {
 114 synchronized (lock) {
 115 /* Guard against the possibility of a daemon thread 
invoking exit

 116  * after DestroyJavaVM initiates the shutdown sequence
 117  */
 118 if (VM.isShutdown()) return;
 119 }

I think this is actually impossible to hit, but that's a separate 
cleanup.


When T1 is calling runHooks, T2 calls exit and blocks on Shutdown.class, 
once T1 releases the lock if VM does not halt yet, T2 will enter this 
method.


 139 // set shutdown state
 140 VM.shutdown();

Just an observation that we consider "shutdown" to be after all system 
shutdown hooks are run. So this (contrary to what I wrote in the CSR) 
narrows the window in which a concurrent exit(-1) would trigger an 
immediate halt rather than a hang. Which in turn strengthens the 
argument for just dropping that behaviour.


If an application hook calls exit(0), it will hang (not the thread 
holding the Shutdown class lock).  Do you want to file an issue to track 
that?




---

src/java.base/share/classes/jdk/internal/misc/VM.java

 103 public static void shutdown() {
 104 initLevel(SYSTEM_SHUTDOWN);
 105 }

Doing this is exactly what I meant by my previous comments.



Good.

Thanks for the thorough review.
Mandy

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native Memory Usage for Direct Byte Buffers

[Paul Sandoz]

Hi Adam,

While the burden is minimal there is a principle here that i think we should 
adhere to regarding additions to the code base: additions should have value 
within OpenJDK itself otherwise it can become a thin end of the wedge to more 
stuff (“well you added these things, why not just add these too?”).

So i would still be reluctant to add such methods without understanding the 
larger picture and what you have in mind.

Can you send a pointer to your email referring in more detail to the larger 
change sets?

This use-case might also apply in other related areas too with regards to 
logging/monitoring. I would be interested to understand what Java Flight 
Recorder (JFR) does in this regard (it being open sourced soon i believe) and 
how JFR might relate to what you are doing. Should we be adding JFR events to 
unsafe memory allocation? Can JFR efficiently access part of the Java call 
stack to determine the origin?

Thanks,
Paul.

> On Feb 19, 2018, at 5:08 AM, Adam Farley8  wrote:
> 
> Hi Paul, 
> 
> > Hi Adam, 
> > 
> > From reading the thread i cannot tell if this is part of a wider solution 
> > including some yet to be proposed HotSpot changes. 
> 
> The wider solution would need to include some Hotspot changes, yes. 
> I'm proposing raising a bug, committing the code we have here to 
> "set the stage", and then we can invest more time&energy later 
> if the concept goes down well and the community agrees to pursue 
> the full solution. 
> 
> As an aside, I tried submitting a big code set (including hotspot 
> changes) months ago, and I'm *still* struggling to find someone to 
> commit the thing, so I figured I'd try a more gradual, staged approach 
> this time. 
> 
> > 
> > As is i would be resistant to adding such standalone internal wrapper 
> > methods to Unsafe that have no apparent benefit within the OpenJDK itself 
> > since it's a maintenance burden. 
> 
> I'm hoping the fact that the methods are a single line (sans 
> comments, descriptors and curly braces) will minimise this burden. 
> 
> > 
> > Can you determine if the calls to UNSAFE.freeMemory/allocateMemory come 
> > from a DBB by looking at the call stack frame above the unsafe call? 
> > 
> > Thanks, 
> > Paul. 
> 
> Yes that is possible, though I would advise against this because: 
> 
>  A) Checking the call stack is expensive, and doing this every time we 
> allocate native memory is an easy way to slow down a program, 
> or rack up mips. 
> and 
>  B) deciding which code path we're using based on the stack 
> means the DBB class+method (and anything the parsing code 
> mistakes for that class+method) can only ever allocate native 
> memory for DBBs. 
> 
> What do you think? 
> 
> Best Regards 
> 
> Adam Farley 
> 
> > 
> >> On Feb 14, 2018, at 3:32 AM, Adam Farley8  wrote: 
> >> 
> >> Hi All, 
> >> 
> >> Currently, diagnostic core files generated from OpenJDK seem to lump all 
> >> of the 
> >> native memory usages together, making it near-impossible for someone to 
> >> figure 
> >> out *what* is using all that memory in the event of a memory leak. 
> >> 
> >> The OpenJ9 VM has a feature which allows it to track the allocation of 
> >> native 
> >> memory for Direct Byte Buffers (DBBs), and to supply that information into 
> >> the 
> >> cores when they are generated. This makes it a *lot* easier to find out 
> >> what is using 
> >> all that native memory, making memory leak resolution less like some dark 
> >> art, and 
> >> more like logical debugging. 
> >> 
> >> To use this feature, there is a native method referenced in Unsafe.java. 
> >> To open 
> >> up this feature so that any VM can make use of it, the java code below 
> >> sets the 
> >> stage for it. This change starts letting people call DBB-specific methods 
> >> when 
> >> allocating native memory, and getting into the habit of using it. 
> >> 
> >> Thoughts? 
> >> 
> >> Best Regards 
> >> 
> >> Adam Farley 
> >> 
> >> P.S. Code: 
> >> 
> >> diff --git 
> >> a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template 
> >> b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template 
> >> --- a/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template 
> >> +++ b/src/java.base/share/classes/java/nio/Direct-X-Buffer.java.template 
> >> @@ -85,7 +85,7 @@ 
> >> // Paranoia 
> >> return; 
> >> } 
> >> -UNSAFE.freeMemory(address); 
> >> +UNSAFE.freeDBBMemory(address); 
> >> address = 0; 
> >> Bits.unreserveMemory(size, capacity); 
> >> } 
> >> @@ -118,7 +118,7 @@ 
> >> 
> >> long base = 0; 
> >> try { 
> >> -base = UNSAFE.allocateMemory(size); 
> >> +base = UNSAFE.allocateDBBMemory(size); 
> >> } catch (OutOfMemoryError x) { 
> >> Bits.unreserveMemory(size, cap); 
> >> throw x; 
> >> diff --git a/src/java.base/share/classes/jdk/internal/misc/Unsafe.

Re: [11] RFR JDK-8198249: Remove deprecated Runtime::runFinalizersOnExit and System::runFinalizersOnExit

[David Holmes]

Hi Mandy,

tl;dr: I think this is now good to go.

On 22/02/2018 5:58 AM, mandy chung wrote:

Here is the updated webrev:
http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198249/webrev.04/

I added some comments to clarify the implementation.


src/java.base/share/classes/java/lang/Shutdown.java

 96 if (VM.isShutdown()|| slot <= currentRunningHook)

Nit: need space after ()


 113 private static void runHooks() {
 114 synchronized (lock) {
 115 /* Guard against the possibility of a daemon thread 
invoking exit

 116  * after DestroyJavaVM initiates the shutdown sequence
 117  */
 118 if (VM.isShutdown()) return;
 119 }

I think this is actually impossible to hit, but that's a separate cleanup.

 139 // set shutdown state
 140 VM.shutdown();

Just an observation that we consider "shutdown" to be after all system 
shutdown hooks are run. So this (contrary to what I wrote in the CSR) 
narrows the window in which a concurrent exit(-1) would trigger an 
immediate halt rather than a hang. Which in turn strengthens the 
argument for just dropping that behaviour.


---

src/java.base/share/classes/jdk/internal/misc/VM.java

 103 public static void shutdown() {
 104 initLevel(SYSTEM_SHUTDOWN);
 105 }

Doing this is exactly what I meant by my previous comments.

Thanks,
David



Mandy

On 2/21/18 11:34 AM, mandy chung wrote:

Hi David,

I think I'm clear on the implementation now (my mistake that I 
neglected ApplicationShutdownHooks).  Shutdown class keeps the "system 
shutdown hooks".  ApplicationShutdownHooks is one system hook that 
starts all application shutdown hooks and waits until they finish.   
java.io.Console and DeleteOnExitHook are two other system shutdown 
hooks to restore console echo state or support deleteOnExit.


The system shutdown hooks are all run in the thread initiating the 
shutdown that holds the Shutdown.class.


On 2/20/18 10:27 PM, David Holmes wrote:

Hi Mandy,

On 21/02/2018 5:57 AM, mandy chung wrote:

Hi David,

I reworked the change in Shutdown class and uses 
jdk.internal.misc.VM to maintain the shutdown state, either in 
progress or shutdown (i.e. all shutdown hooks have been started).


What do you think this revised version:
http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198249/webrev.03/


It confuses me a bit. On the one hand I wasn't sure why we needed to 
bring the VM class into this, then on the other hand it perhaps made 
sense to have VM track shutdown states as well as initialization states.




The latter is my motivation and I think it's a good cleanup to have VM 
state to indicate it's shutdown.


But I'm not sure about the two-phase "shutdown" state - I think I'd 
probably prefer a "shutdown initiated" state and a "shutdown 
complete" state. Then no need to duplicate the constant values for 
IN_PROGRESS and SHUTDOWN (which now need to be kept in sync with 
changes to the VM class). It would also simplify the shutdown() logic.




Shutdown::add method can be used to register a system hook while 
shutdown is in progress.  Actually currentRunningHook can be used to 
guard if Shutdown::add should reject to add a new hook.  I updated the 
webrev and no longer needs the two phases.


Also shutdown(level) should be using initLevel(value) so that it fits 
in with the existing awaitInitLevel() logic and locking strategy! 
Someone may want to wait for shutdown in the future. That also deals 
with the locking issue in the Shutdown class because you don't need 
to use "synchronized(lock)" because runHooks is only called within 
"synchronized(Shutdown.class)". [To be fair the existing locking 
strategy seems confused to me as well - or at least it confuses me.]


I agree but I tried not to touch the existing locking strategy as it 
should be a separate changeset.  I prefer to add VM::shutdown method


I also now realize that the changes I suggested for the Runtime.exit 
docs was incorrect. The existing docs state that we only halt if 
hooks have already been run - which corresponds to the old and new 
code. I, for some reason that escapes me, claimed we only cared if 
the hooks had been started, not completed - but that is inconsistent 
with old spec and the implementation.




I confused myself too.  A small change will fix it:

87 *  If this method is invoked after all shutdown hooks have already
88 * been run and the status is nonzero then this method halts the
89 * virtual machine with the given status code. Otherwise, this method



Mandy

So apologies but what started out as a seemingly simple code removal, 
has become a lot more complicated, and confusing to me.


Thanks,
David
-


On 2/15/18 9:14 PM, David Holmes wrote:


All other updates seem okay. I did have one further thought - in 
Runtime does this change:


  public void runFinalization() {
! SharedSecrets.getJavaLangRefAccess().runFinalization();
  }

affect the classloading/initi

Re: [PATCH] Reduce Chance Of Mistakenly Early Backing Memory Cleanup

[Paul Sandoz]

Hi Ben,

Much better :-) thanks for doing this.

Before preceding with replacing @DontInline with @ForceInline i would like to 
appeal to Vladimir to confirm this is ok.

If you send me an attached patch directly in email i can upload it as a webrev 
for you, it should be easier to review. (When you have two sponsored 
contributions you can request the author role which gives you an account on 
JIRA and to the cr system so you can upload webrevs [2] yourself). 

Paul.

[1] http://openjdk.java.net/projects/#project-author 

[2] http://openjdk.java.net/guide/webrevHelp.html 


> On Feb 19, 2018, at 8:37 AM, Ben Walsh  wrote:
> 
> As requested, here are the results with modifications to the annotations 
> on Reference.reachabilityFence. Much more promising …

Re: RFR: 8198523: Refactor BootstrapMethodInvoker to further avoid runtime type checks

[Paul Sandoz]

+1

Paul.

> On Feb 21, 2018, at 1:00 PM, Claes Redestad  wrote:
> 
> Hi,
> 
> BootstrapMethodInvoker can be improved a bit further by providing
> static information about the type argument, and additionally
> teaching it about StringConcatFactory BSM:
> 
> Patch: http://cr.openjdk.java.net/~redestad/8198523/jdk.00/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8198523
> 
> This slightly improves startup on string concat tests.
> 
> Thanks!
> 
> /Claes

Reactive Streams utility API

[James Roper]

Hi all,

This is an email to give people a heads up that we'd like to look at
creating an API, in the same vein as the JDK8 Streams API, for building
reactive streams (a la JDK9 juc.Flow). Our goals for this are:

* To fill a gap in the JDK where if a developer wants to do even the
simplest of things with a JDK9 juc.Flow, such as map or filter, they need
to bring in a third party library that implements that.
* To produce an API that can build Publishers, Subscribers, Processors, and
complete graphs, for the purposes of consuming APIs that use reactive
streams (for example, JDK9 Http Client).
* To produce an API that aligns closely with ju.stream.Stream, using it for
inspiration for naming, scope, general API shape, and other aspects. The
purpose of this goal is to ensure familiarity of Java developers with the
new API, and to limit the number of concepts Java developers need to
understand to do the different types of streaming offered by the JDK.
* To produce an API that can be implemented by multiple providers
(including an RI in the JDK itself), using the ServiceLoader mechanism to
provide and load a default implementation (while allowing custom
implementations to be manually provided). There are a lot of concerns that
each different streams implementation provides and implements, beyond
streaming, for example monitoring/tracing, concurrency modelling, buffering
strategies, performance aspects of the streams handling including fusing,
and context (eg thread local) propagation. This will allow libraries to use
and provide contracts based on this API without depending on a particular
implementation, and allows developers to select the implementation that
meets their needs.

Non goals:

* To produce a kitchen sink of utilities for working with reactive streams.
There already exist a number of reactive streams implementations that seek
to meet this goal (eg, Akka Streams, Reactor, RxJava), and once you go past
the basics (map, filter, collect), and start dealing with things like fan
in/out, cycles, restarting, etc, the different approaches to solving this
start to vary greatly. The JDK should provide enough to be useful for
typical every day streaming use cases, with developers being able to select
a third party library for anything more advanced.

We will update this list when we have something ready for public review.
This probably won't be far off. Our hope is that we can propose this as a
JEP.

Regards,

James

-- 
*James Roper*
*Senior Octonaut*

Lightbend  – Build reactive apps!
Twitter: @jroper 

Re: RFR 8189330 Cleanup FileDescriptor implementation

[Brian Burkhalter]

Hi Roger,

I think that this looks fine. I assume the issue should be labelled 
“noreg-cleanup."

Brian

On Feb 16, 2018, at 2:28 PM, Roger Riggs  wrote:

> Please review a cleanup and refactoring of FileDescriptor.java.
> The Unix and Windows versions are merged to ease maintenance remove 
> replicated code
> The FileCleanable class is extracted and SocketCleanable is updated.
> 
> Webrev:
>   http://cr.openjdk.java.net/~rriggs/webrev-fd-cleanup-8189330/
> 
> Issue:
>   https://bugs.openjdk.java.net/browse/JDK-8189330

Re: [11] RFR: JDK-8198385: Remove property sun.locale.formatasdefault

[Brian Burkhalter]

Hi Naoto,

+1

Brian

On Feb 21, 2018, at 1:13 PM, naoto.s...@oracle.com wrote:

> Please review the fix to the following issue:
> 
> https://bugs.openjdk.java.net/browse/JDK-8198385
> 
> The proposed changeset is located at:
> 
> http://cr.openjdk.java.net/~naoto/8198385/webrev.00/
> 
> The property was introduced in JDK7 for the backward compatibility, which at 
> this point is no longer needed. Corresponding CSR is already approved.

Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

[yumin qi]

Roger,

 Thanks. Pushed.

On Wed, Feb 21, 2018 at 2:00 PM, Roger Riggs  wrote:

> Hi Yumin,
>
> Use your OpenJDK id  "minqi" for the Author instead of your email.
>
> Roger
>
>
>
> On 2/21/2018 4:54 PM, yumin qi wrote:
>
>> Alan,
>>
>>Somehow there is a problem for me to push the changeset:
>> pushing to ssh://mi...@hg.openjdk.java.net/jdk/jdk
>> searching for changes
>> remote: adding changesets
>> remote: adding manifests
>> remote: adding file changes
>> remote: added 1 changesets with 3 changes to 3 files
>> remote: [jcheck fc6df5c6d4d2 2018-01-23 14:21:17 -0800]
>> remote:
>> remote: > Changeset: 48927:0736b1e12c70
>> remote: > Author:Yumin Qi 
>> remote: > Date:  2018-02-21 12:01
>> remote: >
>> remote: > 8194154: System property user.dir should not be changed
>> remote: > Summary: Cached user.dir so getCanonicalPath uses the cached
>> value.
>> remote: > Reviewed-by: alanb, bpb, rriggs
>> remote:
>> remote: Invalid changeset author: Yumin Qi 
>> remote:
>> remote: transaction abort!
>> remote: rollback completed
>> remote: abort: pretxnchangegroup.0.jcheck hook failed
>>
>> What is the real reason for the failure?
>> I am in following projects:
>>
>> Projects
>> hsx  HotSpot Express Project – Author
>> jdk  JDK Project – Committer
>> jdk-updates  JDK Updates
>> Project – Committer
>> jdk10  JDK 10 Project – Committer
>>
>> Thanks
>> Yumin
>>
>> On Mon, Feb 19, 2018 at 10:11 AM, yumin qi  wrote:
>>
>> Yes, I am committer.
>>>
>>> Brian, do you okay with the version? If no objection, I will push it into
>>> jdk.
>>>
>>> Thanks
>>> Yumin
>>>
>>> On Mon, Feb 19, 2018 at 2:28 AM, Alan Bateman 
>>> wrote:
>>>
>>> On 19/02/2018 05:09, yumin qi wrote:

 Thanks!
>
> I need a sponsor for pushing it to jdk. Can you or someone else help to
> push it?
>
> minqi is a jdk committer. If this is you then you should be able to
 push
 it yourself.

 -Alan.


>>>
>

Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

[Roger Riggs]

Hi Yumin,

Use your OpenJDK id  "minqi" for the Author instead of your email.

Roger


On 2/21/2018 4:54 PM, yumin qi wrote:

Alan,

   Somehow there is a problem for me to push the changeset:
pushing to ssh://mi...@hg.openjdk.java.net/jdk/jdk
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 3 changes to 3 files
remote: [jcheck fc6df5c6d4d2 2018-01-23 14:21:17 -0800]
remote:
remote: > Changeset: 48927:0736b1e12c70
remote: > Author:Yumin Qi 
remote: > Date:  2018-02-21 12:01
remote: >
remote: > 8194154: System property user.dir should not be changed
remote: > Summary: Cached user.dir so getCanonicalPath uses the cached
value.
remote: > Reviewed-by: alanb, bpb, rriggs
remote:
remote: Invalid changeset author: Yumin Qi 
remote:
remote: transaction abort!
remote: rollback completed
remote: abort: pretxnchangegroup.0.jcheck hook failed

What is the real reason for the failure?
I am in following projects:

Projects
hsx  HotSpot Express Project – Author
jdk  JDK Project – Committer
jdk-updates  JDK Updates
Project – Committer
jdk10  JDK 10 Project – Committer

Thanks
Yumin

On Mon, Feb 19, 2018 at 10:11 AM, yumin qi  wrote:


Yes, I am committer.

Brian, do you okay with the version? If no objection, I will push it into
jdk.

Thanks
Yumin

On Mon, Feb 19, 2018 at 2:28 AM, Alan Bateman 
wrote:


On 19/02/2018 05:09, yumin qi wrote:


Thanks!

I need a sponsor for pushing it to jdk. Can you or someone else help to
push it?


minqi is a jdk committer. If this is you then you should be able to push
it yourself.

-Alan.

Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

[yumin qi]

Alan,

  Somehow there is a problem for me to push the changeset:
pushing to ssh://mi...@hg.openjdk.java.net/jdk/jdk
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 3 changes to 3 files
remote: [jcheck fc6df5c6d4d2 2018-01-23 14:21:17 -0800]
remote:
remote: > Changeset: 48927:0736b1e12c70
remote: > Author:Yumin Qi 
remote: > Date:  2018-02-21 12:01
remote: >
remote: > 8194154: System property user.dir should not be changed
remote: > Summary: Cached user.dir so getCanonicalPath uses the cached
value.
remote: > Reviewed-by: alanb, bpb, rriggs
remote:
remote: Invalid changeset author: Yumin Qi 
remote:
remote: transaction abort!
remote: rollback completed
remote: abort: pretxnchangegroup.0.jcheck hook failed

What is the real reason for the failure?
I am in following projects:

Projects
hsx  HotSpot Express Project – Author
jdk  JDK Project – Committer
jdk-updates  JDK Updates
Project – Committer
jdk10  JDK 10 Project – Committer

Thanks
Yumin

On Mon, Feb 19, 2018 at 10:11 AM, yumin qi  wrote:

> Yes, I am committer.
>
> Brian, do you okay with the version? If no objection, I will push it into
> jdk.
>
> Thanks
> Yumin
>
> On Mon, Feb 19, 2018 at 2:28 AM, Alan Bateman 
> wrote:
>
>> On 19/02/2018 05:09, yumin qi wrote:
>>
>>> Thanks!
>>>
>>> I need a sponsor for pushing it to jdk. Can you or someone else help to
>>> push it?
>>>
>> minqi is a jdk committer. If this is you then you should be able to push
>> it yourself.
>>
>> -Alan.
>>
>
>

[11] RFR: JDK-8198385: Remove property sun.locale.formatasdefault

[naoto . sato]

Hello,

Please review the fix to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8198385

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/8198385/webrev.00/

The property was introduced in JDK7 for the backward compatibility, 
which at this point is no longer needed. Corresponding CSR is already 
approved.


Naoto

RFR: 8198523: Refactor BootstrapMethodInvoker to further avoid runtime type checks

[Claes Redestad]

Hi,

BootstrapMethodInvoker can be improved a bit further by providing
static information about the type argument, and additionally
teaching it about StringConcatFactory BSM:

Patch: http://cr.openjdk.java.net/~redestad/8198523/jdk.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8198523

This slightly improves startup on string concat tests.

Thanks!

/Claes

Re: Oracle Java 8u161 regression in XML Schema Factory

[Joe Wang]

@Joe: Is there some documentation for this change in default behavior that came 
with JDK8u161? I assume it is for higher security and cannot be reverted (e.g. 
by setting the feature default for Djdk.xml.overrideDefaultParser to true).


It is indeed. It was a customer's request. Customers' complaints were 
that a factory created through the official API could in many cases, 
unknowingly to the customers, load 3rd party parsers that didn't 
necessarily implement the security features added since JDK7u40 and 8. 
For customers, this behavior was a security concern. It was particularly 
inconvenient to users who might have some 3rd party libraries that just 
happen to be in their environment.


This change was not mentioned in the release notes. I'll check whether 
we could find a right place for a note of this change. The 8u161 release 
notes would have been a good place to do so.


Best,
Joe



Best regards
Christoph


-Original Message-
From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
Behalf Of Bernd
Sent: Dienstag, 13. Februar 2018 22:55
To: OpenJDK Dev list
Subject: Re: Oracle Java 8u161 regression in XML Schema Factory

Hello,


2018-01-25 17:41 GMT+01:00 Seán Coffey:


Classes nearer to those below were touched via JDK-8186080: Transform

XML

interfaces
http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/rev/cb84156d54b2
http://hg.openjdk.java.net/jdk8u/jdk8u/jaxp/rev/08a44c164993

This may be connected with some tools trying to redefine the classes
perhaps. Needs more investigating. Perhaps the XMLSchemaLoader

changes are

a factor ?


I have ben able to extract a minimal reproducer. It is not related to
XMLUnit, only to powermock. If it instruments com.sun but not javax.xml
(and other combinations) then it fails.

For details see the readme in this maven project:

https://github.com/ecki/reproduce-schemaex

I also found a way to make it work with both versions, so its no longer an
issue for me, but there is definitely some changes (which might also be
triggered in AppServers or OSGi containers with partially reconfigured
implementations. Not sure if you want to investigate deeper).

Gruss
Bernd


Here is the stacktrace anyway:

com.sun.org.apache.xerces.internal.impl.dv.DVFactoryException: Schema
factory class
com.sun.org.apache.xerces.internal.impl.dv.xs.SchemaDVFactoryImpl

does

not
extend from SchemaDVFactory.
  at
com.sun.org.apache.xerces.internal.impl.dv.SchemaDVFactory.
getInstance(SchemaDVFactory.java:75)
  at
com.sun.org.apache.xerces.internal.impl.dv.SchemaDVFactory.
getInstance(SchemaDVFactory.java:57)
  at
com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaLoader.
reset(XMLSchemaLoader.java:1024)
  at
com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaLoader.
loadGrammar(XMLSchemaLoader.java:556)
  at
com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaLoader.
loadGrammar(XMLSchemaLoader.java:535)
  at
com.sun.org.apache.xerces.internal.jaxp.validation.XMLSchema
Factory.newSchema(XMLSchemaFactory.java:254)
  at javax.xml.validation.SchemaFactory.newSchema(SchemaFactory.
java:638)
  at javax.xml.validation.SchemaFactory.newSchema(SchemaFactory.
java:654)
  at
com.seeburger.api.test.helpers.BuilderTestHelper.getCRSchema
Validator(BuilderTestHelper.java:57)
  at
com.seeburger.api.test.helpers.BuilderTestHelper.validateAnd
Compare(BuilderTestHelper.java:73)
  at
com.seeburger.api.test.KSMBuilderTest.testDeletePGP(KSMBuild
erTest.java:196)
  at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
  at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAcce
ssorImpl.java:62)
  at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMe
thodAccessorImpl.java:43)
  at java.lang.reflect.Method.invoke(Method.java:498)
  at org.junit.internal.runners.TestMethod.invoke(TestMethod.java:68)
  at
org.powermock.modules.junit4.internal.impl.PowerMockJUnit44R
unnerDelegateImpl$PowerMockJUnit44MethodRunner.runTestMethod
(PowerMockJUnit44RunnerDelegateImpl.java:310)
  at org.junit.internal.runners.MethodRoadie$2.run(MethodRoadie.
java:89)
  at
org.junit.internal.runners.MethodRoadie.runBeforesThenTestTh
enAfters(MethodRoadie.java:97)
  at
org.powermock.modules.junit4.internal.impl.PowerMockJUnit44R
unnerDelegateImpl$PowerMockJUnit44MethodRunner.executeTest(P
owerMockJUnit44RunnerDelegateImpl.java:294)
  at
org.powermock.modules.junit4.internal.impl.PowerMockJUnit47R
unnerDelegateImpl$PowerMockJUnit47MethodRunner.executeTestIn
Super(PowerMockJUnit47RunnerDelegateImpl.java:127)
  at
org.powermock.modules.junit4.internal.impl.PowerMockJUnit47R
unnerDelegateImpl$PowerMockJUnit47MethodRunner.executeTest(P
owerMockJUnit47RunnerDelegateImpl.java:82)
  at
org.powermock.modules.junit4.internal.impl.PowerMockJUnit44R
unnerDelegateImpl$PowerMockJUnit44MethodRunner.runBeforesThe
nTestThenAfters(PowerMockJUnit44RunnerDelegateImpl.java:282)
  at org.junit.internal.runners.MethodRoa

RE: Draft JEP: To use UTF-8 as the default charset for the Java virtual machine.

[Uwe Schindler]

Hi Alan,

> > The Java 7+ methods in java.nio.file.Files already ignore the default 
> > charset
> and always use UTF-8. How to proceed with those? Should they be changed
> to behave to the new mechanisms? I'd suggest to not do this, as its part of
> the spec (to use UTF-8) and should not rely on external forces, but I wanted
> to bring this in.
> >
> There is no proposal to change these methods.

Thanks for clarifying! I just wanted to mention this, because those methods are 
different, so you should at least think about it 😊

Uwe

-
Uwe Schindler
uschind...@apache.org 
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
http://lucene.apache.org/

Re: Draft JEP: To use UTF-8 as the default charset for the Java virtual machine.

[Robert Muir]

On Wed, Feb 21, 2018 at 1:16 PM, Xueming Shen  wrote:

>
> Hi Robert,
>
> Understood a silent replacement might not be the desired behavior in
> some use scenarios. Anymore details regarding what "most apps want"
> when there is/are malformed/unmappable? It appears the best the
> underneath de/encoder can do here is to throw an IOException. Given
> the caller of the Reader/Writer does not have the access to the bytes of
> the underlying stream src (reader)/dst(writer), there is in theory
> impossible
> to do anything to recover and continue without risking data loss. The
> assumption here is if you want to have a fine-grained control of the de/
> encoding, you might want to work with the Input/OutStream/Channel +
> CharsetDe/Encoder instead of Reader/Writer.
>
> No, I'm not saying we can't do
> Reader(CharsetDecoder)/Writer(CharsetEncoder),
> just wanted to know what's the real use scenario and what's the better/
> best choice here.
>

I think the exception is the best default? This is the default
behavior of python for example, unless you specifically ask for
"replace" or "ignore".

>>> b'\xFFabc'.decode("utf-8")
Traceback (most recent call last):
  File "", line 1, in 
UnicodeDecodeError: 'utf-8' codec can't decode byte 0xff in position
0: invalid start byte

Its also the default behavior of 'iconv' command-line tool used for
converting charsets, unless you pass additional options.

$ iconv -f utf-8 -t utf-8 test2.mp4
 ftypisomisomiso2avc1mp41e
iconv: test2.mp4:1:26: cannot convert

Unfortunately in java, when using Charset or String parameters, it
gives silently replacement with \uFFFD, etc. Its necessary to pass a
CharsetDecoder to get an exception that something went wrong.

The current situation is especially confusing as there is nothing in
the javadocs to indicate that the behavior of InputStreamReader(x,
Charset) and InputStreamReader(x, String) differ substantially from
InputStreamReader(x, CharsetDecoder) ! I think the Charset and String
parameters should default to REPORT, so the behavior of all
constructors are consistent. If you want to replace, you should have
to ask for it. I think replacement has use-cases but they are more
"expert", e.g. web-crawling and so on. In general, wrong bytes
indicate a problem and it can be very difficult to debug these issues
when java hides these problems by default...

Re: RFR: Here are some URLClassPath patches

[Martin Buchholz]

On Wed, Feb 21, 2018 at 7:48 AM, Roger Riggs  wrote:

>
> On the style changes in URLClassPath-ArrayDeque, about declarations like:
>
> ArrayList path = new ArrayList<>();
>
> It had been a recommended style to use the abstract type (List) on the
> left hand side
> and only use the concrete type where necessary.
>

There's a debate about whether using concrete types in the implementation
is clearer.  I happen to think so.   But in performance critical classes
the concrete class is more important to the maintainer (I'm thinking
"ArrayDeque" not "Deque" when I maintain this code), and is also likely to
help the JVM.

Re: RFR: Here are some URLClassPath patches

[Martin Buchholz]

OK, we have a reworked set of patches.

(In my corner of openjdk we generally use real javadoc on private elements.)

I reverted private doc comment style to the current maddening
inconsistency, except I couldn't restrain myself from fixing

-// ACC used when loading classes and resources */
+// ACC used when loading classes and resources

I changed ArrayDeque.push to addFirst, as Peter suggested.


8198480: Improve ClassLoaders static init block
http://cr.openjdk.java.net/~martin/webrevs/jdk/ClassLoaders-static/
https://bugs.openjdk.java.net/browse/JDK-8198480

8198481: Coding style cleanups for
src/java.base/share/classes/jdk/internal/loader
http://cr.openjdk.java.net/~martin/webrevs/jdk/loader-style/
https://bugs.openjdk.java.net/browse/JDK-8198481

8198482: The URLClassPath field "urls" should be renamed to "unopenedUrls"
http://cr.openjdk.java.net/~martin/webrevs/jdk/URLClassPath-rename-urls/
https://bugs.openjdk.java.net/browse/JDK-8198482

8198484: URLClassPath should use an ArrayDeque instead of a Stack
http://cr.openjdk.java.net/~martin/webrevs/jdk/URLClassPath-ArrayDeque/
https://bugs.openjdk.java.net/browse/JDK-8198484

8198485: Simplify a URLClassPath constructor
http://cr.openjdk.java.net/~martin/webrevs/jdk/URLClassPath-simplify-constructor/
https://bugs.openjdk.java.net/browse/JDK-8198485

Re: RFR (JDK11) 8137326: Methods for comparing CharSequence, StringBuilder, and StringBuffer

[Joe Wang]

Hi Stuart,

Thanks for the apiNote! Joe re-approved the CSR with the added apiNote.

For a test that compares Latin1 vs UTF16 coders, I added tests to the 
compact string's CompareTo test[1], basically running the original test 
for String with StringBuilder/Buffer. A build with jdk_core tests passed.


Here's the updated webrev:
JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

[1] 
http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/test/jdk/java/lang/String/CompactString/CompareTo.java.sdiff.html


Best,
Joe

On 2/14/18, 5:09 PM, Stuart Marks wrote:

Hi Joe,

Overall, looks good.

Are there any tests of AbstractStringBuilder.compareTo() that exercise 
comparisons of the Latin1 vs UTF16 coders?


A couple people have raised the issue of the SB's implementing 
Comparable but not overriding equals(). This is unusual but 
well-defined. I do think it deserves the "inconsistent with equals" 
treatment. Something like the following should be added to both SB's:


==
@apiNote
{@code StringBuilder} implements {@code Comparable} but does not 
override {@link Object#equals equals}. Thus, the natural ordering of 
{@code StringBuilder} is inconsistent with equals. Care should be 
exercised if {@code StringBuilder} objects are used as keys in a 
{@code SortedMap} or elements in a {@code SortedSet}. See {@link 
Comparable}, {@link java.util.SortedMap SortedMap}, or {@link 
java.util.SortedSet SortedSet} for more information.

==

Respectively for StringBuffer. (Adjust markup to taste.)

Thanks,

s'marks




On 2/8/18 4:47 PM, Joe Wang wrote:

Hi all,

The CSR for the enhancement is now approved. Thanks Joe!

The webrev has been updated accordingly. Please let me know if you 
have any further comment on the implementation.

JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

Thanks,
Joe

On 2/2/2018 12:46 PM, Joe Wang wrote:

Thanks Jason. Will update that accordingly.

Best,
Joe

On 2/2/2018 11:22 AM, Jason Mehrens wrote:

Joe,

The identity check in CS.compare makes sense.  However, it won't be 
null hostile if we call CS.compare(null, null) and that doesn't 
seem right.

Usually when writing comparator classes I end up with:
===
if (Objects.requireNonNull(o1) == Objects.requireNonNull(o2)) {
 return 0;
}
===

Jason

From: core-libs-dev  on 
behalf of Joe Wang 

Sent: Friday, February 2, 2018 1:01 PM
To: core-libs-dev
Subject: Re: RFR (JDK11) 8137326: Methods for comparing 
CharSequence, StringBuilder, and StringBuffer


Hi,

Thanks all for comments and suggestions. I've updated the webrev. 
Please

review.

JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

Thanks,
Joe

On 1/31/2018 9:31 PM, Joe Wang wrote:

Hi Tagir,

Thanks for the comment. I will consider adding that to the javadoc
emphasizing that the comparison is performed from 0 to length() - 
1 of

the two sequences.

Best,
Joe

On 1/29/18, 8:07 PM, Tagir Valeev wrote:

Hello!

An AbstractStringBuilder#compareTo implementation is wrong. You 
cannot

simply compare the whole byte array. Here's the test-case:

public class Test {
public static void main(String[] args) {
  StringBuilder sb1 = new StringBuilder("test1");
  StringBuilder sb2 = new StringBuilder("test2");
  sb1.setLength(4);
  sb2.setLength(4);
  System.out.println(sb1.compareTo(sb2));
System.out.println(sb1.toString().compareTo(sb2.toString()));
}
}

We truncated the stringbuilders making their content equal, so
sb1.toString().compareTo(sb2.toString()) is 0, but compareTo 
compares
the original content (before the truncation) as truncation, of 
course,

does not zero the truncated bytes, neither does it reallocate the
array (unless explicitly asked via trimToSize).

With best regards,
Tagir Valeev.


On Fri, Jan 26, 2018 at 10:00 AM, Joe Wang
wrote:

Hi,

Adding methods for comparing CharSequence, StringBuilder, and
StringBuffer.

The Comparable implementations for StringBuilder/Buffer are similar
to that
of String, allowing comparison operations between two
StringBuilders/Buffers, e.g.
aStringBuilder.compareTo(anotherStringBuilder).
For CharSequence however, refer to the comments in JIRA, a static
method
'compare' is added instead of implementing the Comparable 
interface.

This
'compare' method may take CharSequence implementations such as 
String,

StringBuilder and StringBuffer, making it possible to perform
comparison
among them. The previous example for example is equivalent to
CharSequence.compare(aStringBuilder, anotherStringBuilder).

Tests for java.base have been independent from each other. The new
tests are
therefore created to have no dependency on each other or sharing 
any

code.

JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/we

Re: [11] RFR JDK-8198249: Remove deprecated Runtime::runFinalizersOnExit and System::runFinalizersOnExit

[mandy chung]

Here is the updated webrev:
http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198249/webrev.04/

I added some comments to clarify the implementation.

Mandy

On 2/21/18 11:34 AM, mandy chung wrote:

Hi David,

I think I'm clear on the implementation now (my mistake that I 
neglected ApplicationShutdownHooks).  Shutdown class keeps the "system 
shutdown hooks".  ApplicationShutdownHooks is one system hook that 
starts all application shutdown hooks and waits until they finish.   
java.io.Console and DeleteOnExitHook are two other system shutdown 
hooks to restore console echo state or support deleteOnExit.


The system shutdown hooks are all run in the thread initiating the 
shutdown that holds the Shutdown.class.


On 2/20/18 10:27 PM, David Holmes wrote:

Hi Mandy,

On 21/02/2018 5:57 AM, mandy chung wrote:

Hi David,

I reworked the change in Shutdown class and uses 
jdk.internal.misc.VM to maintain the shutdown state, either in 
progress or shutdown (i.e. all shutdown hooks have been started).


What do you think this revised version:
http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198249/webrev.03/


It confuses me a bit. On the one hand I wasn't sure why we needed to 
bring the VM class into this, then on the other hand it perhaps made 
sense to have VM track shutdown states as well as initialization states.




The latter is my motivation and I think it's a good cleanup to have VM 
state to indicate it's shutdown.


But I'm not sure about the two-phase "shutdown" state - I think I'd 
probably prefer a "shutdown initiated" state and a "shutdown 
complete" state. Then no need to duplicate the constant values for 
IN_PROGRESS and SHUTDOWN (which now need to be kept in sync with 
changes to the VM class). It would also simplify the shutdown() logic.




Shutdown::add method can be used to register a system hook while 
shutdown is in progress.  Actually currentRunningHook can be used to 
guard if Shutdown::add should reject to add a new hook.  I updated the 
webrev and no longer needs the two phases.


Also shutdown(level) should be using initLevel(value) so that it fits 
in with the existing awaitInitLevel() logic and locking strategy! 
Someone may want to wait for shutdown in the future. That also deals 
with the locking issue in the Shutdown class because you don't need 
to use "synchronized(lock)" because runHooks is only called within 
"synchronized(Shutdown.class)". [To be fair the existing locking 
strategy seems confused to me as well - or at least it confuses me.]


I agree but I tried not to touch the existing locking strategy as it 
should be a separate changeset.  I prefer to add VM::shutdown method


I also now realize that the changes I suggested for the Runtime.exit 
docs was incorrect. The existing docs state that we only halt if 
hooks have already been run - which corresponds to the old and new 
code. I, for some reason that escapes me, claimed we only cared if 
the hooks had been started, not completed - but that is inconsistent 
with old spec and the implementation.




I confused myself too.  A small change will fix it:

87 *  If this method is invoked after all shutdown hooks have already
88 * been run and the status is nonzero then this method halts the
89 * virtual machine with the given status code. Otherwise, this method



Mandy

So apologies but what started out as a seemingly simple code removal, 
has become a lot more complicated, and confusing to me.


Thanks,
David
-


On 2/15/18 9:14 PM, David Holmes wrote:


All other updates seem okay. I did have one further thought - in 
Runtime does this change:


  public void runFinalization() {
! SharedSecrets.getJavaLangRefAccess().runFinalization();
  }

affect the classloading/initialization order at all?


Runtime::runFinalization is not called by the system code.  So it 
won't be invoked during startup and hence won't change the 
classloading order during startup.


Mandy

Re: RFR (JDK11) 8137326: Methods for comparing CharSequence, StringBuilder, and StringBuffer

[Xueming Shen]

Joe,

AbstractStringBuilder is a mutable class. You might need to take a local 
copy

and do the explicit boundary check before going into the StringLatin1/UTF16,
especially StringUTF16. Some intrinsics, UTF16.getChar(byte[], int) for 
example,

assume the caller performs bounds check (for performance reason).

-Sherman

On 2/21/18, 11:25 AM, Joe Wang wrote:

Hi Stuart,

Thanks for the apiNote! Joe re-approved the CSR with the added apiNote.

For a test that compares Latin1 vs UTF16 coders, I added tests to the 
compact string's CompareTo test[1], basically running the original 
test for String with StringBuilder/Buffer. A build with jdk_core tests 
passed.


Here's the updated webrev:
JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

[1] 
http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/test/jdk/java/lang/String/CompactString/CompareTo.java.sdiff.html


Best,
Joe

On 2/14/18, 5:09 PM, Stuart Marks wrote:

Hi Joe,

Overall, looks good.

Are there any tests of AbstractStringBuilder.compareTo() that 
exercise comparisons of the Latin1 vs UTF16 coders?


A couple people have raised the issue of the SB's implementing 
Comparable but not overriding equals(). This is unusual but 
well-defined. I do think it deserves the "inconsistent with equals" 
treatment. Something like the following should be added to both SB's:


==
@apiNote
{@code StringBuilder} implements {@code Comparable} but does not 
override {@link Object#equals equals}. Thus, the natural ordering of 
{@code StringBuilder} is inconsistent with equals. Care should be 
exercised if {@code StringBuilder} objects are used as keys in a 
{@code SortedMap} or elements in a {@code SortedSet}. See {@link 
Comparable}, {@link java.util.SortedMap SortedMap}, or {@link 
java.util.SortedSet SortedSet} for more information.

==

Respectively for StringBuffer. (Adjust markup to taste.)

Thanks,

s'marks




On 2/8/18 4:47 PM, Joe Wang wrote:

Hi all,

The CSR for the enhancement is now approved. Thanks Joe!

The webrev has been updated accordingly. Please let me know if you 
have any further comment on the implementation.

JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

Thanks,
Joe

On 2/2/2018 12:46 PM, Joe Wang wrote:

Thanks Jason. Will update that accordingly.

Best,
Joe

On 2/2/2018 11:22 AM, Jason Mehrens wrote:

Joe,

The identity check in CS.compare makes sense.  However, it won't 
be null hostile if we call CS.compare(null, null) and that doesn't 
seem right.

Usually when writing comparator classes I end up with:
===
if (Objects.requireNonNull(o1) == Objects.requireNonNull(o2)) {
 return 0;
}
===

Jason

From: core-libs-dev  on 
behalf of Joe Wang 

Sent: Friday, February 2, 2018 1:01 PM
To: core-libs-dev
Subject: Re: RFR (JDK11) 8137326: Methods for comparing 
CharSequence, StringBuilder, and StringBuffer


Hi,

Thanks all for comments and suggestions. I've updated the webrev. 
Please

review.

JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

Thanks,
Joe

On 1/31/2018 9:31 PM, Joe Wang wrote:

Hi Tagir,

Thanks for the comment. I will consider adding that to the javadoc
emphasizing that the comparison is performed from 0 to length() - 
1 of

the two sequences.

Best,
Joe

On 1/29/18, 8:07 PM, Tagir Valeev wrote:

Hello!

An AbstractStringBuilder#compareTo implementation is wrong. You 
cannot

simply compare the whole byte array. Here's the test-case:

public class Test {
public static void main(String[] args) {
  StringBuilder sb1 = new StringBuilder("test1");
  StringBuilder sb2 = new StringBuilder("test2");
  sb1.setLength(4);
  sb2.setLength(4);
  System.out.println(sb1.compareTo(sb2));
System.out.println(sb1.toString().compareTo(sb2.toString()));
}
}

We truncated the stringbuilders making their content equal, so
sb1.toString().compareTo(sb2.toString()) is 0, but compareTo 
compares
the original content (before the truncation) as truncation, of 
course,

does not zero the truncated bytes, neither does it reallocate the
array (unless explicitly asked via trimToSize).

With best regards,
Tagir Valeev.


On Fri, Jan 26, 2018 at 10:00 AM, Joe Wang
wrote:

Hi,

Adding methods for comparing CharSequence, StringBuilder, and
StringBuffer.

The Comparable implementations for StringBuilder/Buffer are 
similar

to that
of String, allowing comparison operations between two
StringBuilders/Buffers, e.g.
aStringBuilder.compareTo(anotherStringBuilder).
For CharSequence however, refer to the comments in JIRA, a static
method
'compare' is added instead of implementing the Comparable 
interface.

This
'compare' method may take CharSequence implementations such as 
String,

StringBuilder and StringBuffer, making it possible to perform
comparison
among them. The previous 

Re: [11] RFR JDK-8198249: Remove deprecated Runtime::runFinalizersOnExit and System::runFinalizersOnExit

[mandy chung]

Hi David,

I think I'm clear on the implementation now (my mistake that I neglected 
ApplicationShutdownHooks).  Shutdown class keeps the "system shutdown 
hooks".  ApplicationShutdownHooks is one system hook that starts all 
application shutdown hooks and waits until they finish.   
java.io.Console and DeleteOnExitHook are two other system shutdown hooks 
to restore console echo state or support deleteOnExit.


The system shutdown hooks are all run in the thread initiating the 
shutdown that holds the Shutdown.class.


On 2/20/18 10:27 PM, David Holmes wrote:

Hi Mandy,

On 21/02/2018 5:57 AM, mandy chung wrote:

Hi David,

I reworked the change in Shutdown class and uses jdk.internal.misc.VM 
to maintain the shutdown state, either in progress or shutdown (i.e. 
all shutdown hooks have been started).


What do you think this revised version:
http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198249/webrev.03/


It confuses me a bit. On the one hand I wasn't sure why we needed to 
bring the VM class into this, then on the other hand it perhaps made 
sense to have VM track shutdown states as well as initialization states.




The latter is my motivation and I think it's a good cleanup to have VM 
state to indicate it's shutdown.


But I'm not sure about the two-phase "shutdown" state - I think I'd 
probably prefer a "shutdown initiated" state and a "shutdown complete" 
state. Then no need to duplicate the constant values for IN_PROGRESS 
and SHUTDOWN (which now need to be kept in sync with changes to the VM 
class). It would also simplify the shutdown() logic.




Shutdown::add method can be used to register a system hook while 
shutdown is in progress.  Actually currentRunningHook can be used to 
guard if Shutdown::add should reject to add a new hook.  I updated the 
webrev and no longer needs the two phases.


Also shutdown(level) should be using initLevel(value) so that it fits 
in with the existing awaitInitLevel() logic and locking strategy! 
Someone may want to wait for shutdown in the future. That also deals 
with the locking issue in the Shutdown class because you don't need to 
use "synchronized(lock)" because runHooks is only called within 
"synchronized(Shutdown.class)". [To be fair the existing locking 
strategy seems confused to me as well - or at least it confuses me.]


I agree but I tried not to touch the existing locking strategy as it 
should be a separate changeset.  I prefer to add VM::shutdown method


I also now realize that the changes I suggested for the Runtime.exit 
docs was incorrect. The existing docs state that we only halt if hooks 
have already been run - which corresponds to the old and new code. I, 
for some reason that escapes me, claimed we only cared if the hooks 
had been started, not completed - but that is inconsistent with old 
spec and the implementation.




I confused myself too.  A small change will fix it:

87 *  If this method is invoked after all shutdown hooks have already
88 * been run and the status is nonzero then this method halts the
89 * virtual machine with the given status code. Otherwise, this method



Mandy

So apologies but what started out as a seemingly simple code removal, 
has become a lot more complicated, and confusing to me.


Thanks,
David
-


On 2/15/18 9:14 PM, David Holmes wrote:


All other updates seem okay. I did have one further thought - in 
Runtime does this change:


  public void runFinalization() {
! SharedSecrets.getJavaLangRefAccess().runFinalization();
  }

affect the classloading/initialization order at all?


Runtime::runFinalization is not called by the system code.  So it 
won't be invoked during startup and hence won't change the 
classloading order during startup.


Mandy

Re: Oracle Java 8u161 regression in XML Schema Factory

[Bernd Eckenfels]

Hallo Christoph,

Yes the Tests fail with -Djdk.xml.overrideDefaultParser=true (and false) as 
well.

Gruss
Bernd
-- 
http://bernd.eckenfels.net

Von: Langer, Christoph
Gesendet: Mittwoch, 21. Februar 2018 16:16
An: Bernd; OpenJDK Dev list; huizhe.w...@oracle.com
Betreff: RE: Oracle Java 8u161 regression in XML Schema Factory

Hi Bernd,

would your test still fail with system property 
"-Djdk.xml.overrideDefaultParser=true"?

I debugged a similar issue (if not the same) today and found that with change 
http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jaxp/rev/08a44c164993 the default 
behavior for Xalan changed a bit.

Before that change, the services mechanism was enabled to load the SAX and DOM 
implementations for the use of Xalan. E.g. when 
TransformerFactory.newInstance() was called, the services mechanism was used 
and in the TransformerFactory (at least if it was the default JDK 
implementation), the property to use the services mechanism was set as well - 
wich was used to retrieve SAX and DOM parser implementations. 

And what hit me even more was that SAX2DOM had its private field 
"DocumentBuilderFactory _factory" automatically initialized by a call to 
DocumentBuilderFactory.newInstance() which in turn would resolve any custom DBF 
implementation if it were on classpath. Now, with the new implementation, the 
_factory in SAX2DOM will be initialized with the "overrideDefaultParser " 
setting coming from the Transformer Factory. So, by setting 
-Djdk.xml.overrideDefaultParser=true, you will restore the old behavior.

@Joe: Is there some documentation for this change in default behavior that came 
with JDK8u161? I assume it is for higher security and cannot be reverted (e.g. 
by setting the feature default for Djdk.xml.overrideDefaultParser to true).

Best regards
Christoph

> -Original Message-
> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
> Behalf Of Bernd
> Sent: Dienstag, 13. Februar 2018 22:55
> To: OpenJDK Dev list 
> Subject: Re: Oracle Java 8u161 regression in XML Schema Factory
> 
> Hello,
> 
> 
> 2018-01-25 17:41 GMT+01:00 Seán Coffey :
> 
> >
> > Classes nearer to those below were touched via JDK-8186080: Transform
> XML
> > interfaces
> > http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/rev/cb84156d54b2
> > http://hg.openjdk.java.net/jdk8u/jdk8u/jaxp/rev/08a44c164993
> >
> > This may be connected with some tools trying to redefine the classes
> > perhaps. Needs more investigating. Perhaps the XMLSchemaLoader
> changes are
> > a factor ?
> >
> 
> I have ben able to extract a minimal reproducer. It is not related to
> XMLUnit, only to powermock. If it instruments com.sun but not javax.xml
> (and other combinations) then it fails.
> 
> For details see the readme in this maven project:
> 
> https://github.com/ecki/reproduce-schemaex
> 
> I also found a way to make it work with both versions, so its no longer an
> issue for me, but there is definitely some changes (which might also be
> triggered in AppServers or OSGi containers with partially reconfigured
> implementations. Not sure if you want to investigate deeper).
> 
> Gruss
> Bernd
> 
> 
> Here is the stacktrace anyway:
> >>
> >> com.sun.org.apache.xerces.internal.impl.dv.DVFactoryException: Schema
> >> factory class
> >> com.sun.org.apache.xerces.internal.impl.dv.xs.SchemaDVFactoryImpl
> does
> >> not
> >> extend from SchemaDVFactory.
> >>  at
> >> com.sun.org.apache.xerces.internal.impl.dv.SchemaDVFactory.
> >> getInstance(SchemaDVFactory.java:75)
> >>  at
> >> com.sun.org.apache.xerces.internal.impl.dv.SchemaDVFactory.
> >> getInstance(SchemaDVFactory.java:57)
> >>  at
> >> com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaLoader.
> >> reset(XMLSchemaLoader.java:1024)
> >>  at
> >> com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaLoader.
> >> loadGrammar(XMLSchemaLoader.java:556)
> >>  at
> >> com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaLoader.
> >> loadGrammar(XMLSchemaLoader.java:535)
> >>  at
> >> com.sun.org.apache.xerces.internal.jaxp.validation.XMLSchema
> >> Factory.newSchema(XMLSchemaFactory.java:254)
> >>  at javax.xml.validation.SchemaFactory.newSchema(SchemaFactory.
> >> java:638)
> >>  at javax.xml.validation.SchemaFactory.newSchema(SchemaFactory.
> >> java:654)
> >>  at
> >> com.seeburger.api.test.helpers.BuilderTestHelper.getCRSchema
> >> Validator(BuilderTestHelper.java:57)
> >>  at
> >> com.seeburger.api.test.helpers.BuilderTestHelper.validateAnd
> >> Compare(BuilderTestHelper.java:73)
> >>  at
> >> com.seeburger.api.test.KSMBuilderTest.testDeletePGP(KSMBuild
> >> erTest.java:196)
> >>  at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> >>  at
> >> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAcce
> >> ssorImpl.java:62)
> >>  at
> >> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMe
> >> thodAccessorImpl.java:43)
> >>  at java.lang.reflect.Method.invoke(Method.java:498)
> >>  at o

Re: Draft JEP: To use UTF-8 as the default charset for the Java virtual machine.

[Xueming Shen]

On 2/21/18, 6:26 AM, Robert Muir wrote:

On Wed, Feb 21, 2018 at 8:55 AM, Alan Bateman  wrote:


Good progress was made via JDK-8183743 [1] in Java SE 10 to add constructors
and methods that take a Charset and eliminate the historical
inconsistencies. The issue of legacy FileReader/FileWriter is linked from
that JIRA issue.


Can we ensure we have CharsetDecoder/Encoder params too? There is
unfortunately a huge difference between InputStreamReader(x,
StandardCharsets.UTF_8) and InputStreamReader(x,
StandardCharsets.UTF_8.newDecoder()). And the silent replacement of
the "easier" one is probably not what most apps want.

Hi Robert,

Understood a silent replacement might not be the desired behavior in
some use scenarios. Anymore details regarding what "most apps want"
when there is/are malformed/unmappable? It appears the best the
underneath de/encoder can do here is to throw an IOException. Given
the caller of the Reader/Writer does not have the access to the bytes of
the underlying stream src (reader)/dst(writer), there is in theory 
impossible

to do anything to recover and continue without risking data loss. The
assumption here is if you want to have a fine-grained control of the de/
encoding, you might want to work with the Input/OutStream/Channel +
CharsetDe/Encoder instead of Reader/Writer.

No, I'm not saying we can't do 
Reader(CharsetDecoder)/Writer(CharsetEncoder),

just wanted to know what's the real use scenario and what's the better/
best choice here.

-Sherman

Re: RFR: Here are some URLClassPath patches

[mark . reinhold]

2018/2/21 2:28:19 -0500, alan.bate...@oracle.com:
> On 21/02/2018 06:08, Martin Buchholz wrote:
>> :
>> 
>> 8198481: Coding style cleanups for 
>> src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java
>> http://cr.openjdk.java.net/~martin/webrevs/jdk/loader-style/ 
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8198481
> 
> This changes comments on private fields from // to /**. Are you 
> generating javadoc on private members of internal classes or why are you 
> changing all these fields?

The use of // rather than /** on private members is intentional,
to make it clear to the reader that these are internal elements
rather than API.

You might not like it, but please don't change it.

- Mark

Re: Draft JEP: To use UTF-8 as the default charset for the Java virtual machine.

[Xueming Shen]

Hi Volker,

Yes, the handing of sun.jnu.encoding will not be changed. It will remain as
a read-only/informative system property.

sun.jnu.encoding is really an implementation details (as well as 
file.encoding,
though in this JEP file.encoding might be used to provide a mechanism to 
fallback
to the current/old/existing behavior, so might become a public/official 
interface/
system property). From API perspective the Charset.defaultCharset() is 
the only

place to obtain the "Java virtual machine's default charset".

As Alan said in previous comment, clarifications will be included in the 
final

version based on feedback/suggestion

-Sherman

On 2/21/18, 8:11 AM, Volker Simonis wrote:

Hi Sherman,

the tricky part is really "sun.jnu.encoding" and how the VM interacts
with the underlying OS. You may remember that we had an interesting
discussion about this topic some time ago [1].

As far as I understand, the JEP doesn't plan to change the handling of
"sun.jnu.encoding". So does this mean that the VM will still correctly
start and work on system with a platform encoding different from
UTF-8? I.e. will starting the VM from a path which contains characters
in that special platform encoding or classpath/argument settings with
characters in that special character encoding still work? If the
answer will be yes (which I expect) maybe you could explain that a
little more detailed in the JEP. I.e. the JEP should say that it
changes the default encoding for the Java API classes and not the
default encoding for natively accessing system resources.

Maybe the JEP should also mention that "sun.jnu.encoding" is more or
less a "read-only" property which can not be reliable set by the user
on the command line (it's a chicken-egg problem: for the parsing of
the command line we need the correct encoding, so it can not be
reliably set on the command line).

For these reasons the Summary "Use UTF-8 as the Java virtual machine's
default charset ..." is a little misleading. Maybe you could rephrase
to something like "Use UTF-8 as the default charset so that Java APIs
that depend on the default charset behave consistently across all
platforms."

Thank you and best regards,
Volker

[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/thread.html#37516

On Wed, Feb 21, 2018 at 7:31 AM, Xueming Shen  wrote:

This draft JEP contains a proposal to use UTF-8 as the default charset for
the JVM, so that
APIs that depend on the default charset behave consistently cross all
platforms.

For more details, please see:
https://bugs.openjdk.java.net/browse/JDK-8187041

Sherman

Re: Draft JEP: To use UTF-8 as the default charset for the Java virtual machine.

[Volker Simonis]

Hi Sherman,

the tricky part is really "sun.jnu.encoding" and how the VM interacts
with the underlying OS. You may remember that we had an interesting
discussion about this topic some time ago [1].

As far as I understand, the JEP doesn't plan to change the handling of
"sun.jnu.encoding". So does this mean that the VM will still correctly
start and work on system with a platform encoding different from
UTF-8? I.e. will starting the VM from a path which contains characters
in that special platform encoding or classpath/argument settings with
characters in that special character encoding still work? If the
answer will be yes (which I expect) maybe you could explain that a
little more detailed in the JEP. I.e. the JEP should say that it
changes the default encoding for the Java API classes and not the
default encoding for natively accessing system resources.

Maybe the JEP should also mention that "sun.jnu.encoding" is more or
less a "read-only" property which can not be reliable set by the user
on the command line (it's a chicken-egg problem: for the parsing of
the command line we need the correct encoding, so it can not be
reliably set on the command line).

For these reasons the Summary "Use UTF-8 as the Java virtual machine's
default charset ..." is a little misleading. Maybe you could rephrase
to something like "Use UTF-8 as the default charset so that Java APIs
that depend on the default charset behave consistently across all
platforms."

Thank you and best regards,
Volker

[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/thread.html#37516

On Wed, Feb 21, 2018 at 7:31 AM, Xueming Shen  wrote:
> This draft JEP contains a proposal to use UTF-8 as the default charset for
> the JVM, so that
> APIs that depend on the default charset behave consistently cross all
> platforms.
>
> For more details, please see:
> https://bugs.openjdk.java.net/browse/JDK-8187041
>
> Sherman

Re: RFR: Here are some URLClassPath patches

[Roger Riggs]

Hi Martin,

Generally, I would leave code alone that does not have a good reason for 
changing.

That goes for commenting conventions (// vs /*) too.

On the style changes in URLClassPath-ArrayDeque, about declarations like:

ArrayList path = new ArrayList<>();

It had been a recommended style to use the abstract type (List) on the 
left hand side

and only use the concrete type where necessary.

But with the addition of 'var' local type inference, perhaps that would 
an option here.


(Not trying to start a style war).

$.02, Roger


On 2/21/2018 1:08 AM, Martin Buchholz wrote:

At Google I spend a lot of time staring unproductively at classloader code,
and it's always hard for me to resist the urge to clean it up.  So here are
some patches that should be relatively uncontroversial, and may prepare for
more radical changes later.


8198484: URLClassPath should use an ArrayDeque instead of a Stack
http://cr.openjdk.java.net/~martin/webrevs/jdk/URLClassPath-ArrayDeque/
https://bugs.openjdk.java.net/browse/JDK-8198484

RE: Oracle Java 8u161 regression in XML Schema Factory

[Langer, Christoph]

Hi Bernd,

would your test still fail with system property 
"-Djdk.xml.overrideDefaultParser=true"?

I debugged a similar issue (if not the same) today and found that with change 
http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jaxp/rev/08a44c164993 the default 
behavior for Xalan changed a bit.

Before that change, the services mechanism was enabled to load the SAX and DOM 
implementations for the use of Xalan. E.g. when 
TransformerFactory.newInstance() was called, the services mechanism was used 
and in the TransformerFactory (at least if it was the default JDK 
implementation), the property to use the services mechanism was set as well - 
wich was used to retrieve SAX and DOM parser implementations. 

And what hit me even more was that SAX2DOM had its private field 
"DocumentBuilderFactory _factory" automatically initialized by a call to 
DocumentBuilderFactory.newInstance() which in turn would resolve any custom DBF 
implementation if it were on classpath. Now, with the new implementation, the 
_factory in SAX2DOM will be initialized with the "overrideDefaultParser " 
setting coming from the Transformer Factory. So, by setting 
-Djdk.xml.overrideDefaultParser=true, you will restore the old behavior.

@Joe: Is there some documentation for this change in default behavior that came 
with JDK8u161? I assume it is for higher security and cannot be reverted (e.g. 
by setting the feature default for Djdk.xml.overrideDefaultParser to true).

Best regards
Christoph

> -Original Message-
> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
> Behalf Of Bernd
> Sent: Dienstag, 13. Februar 2018 22:55
> To: OpenJDK Dev list 
> Subject: Re: Oracle Java 8u161 regression in XML Schema Factory
> 
> Hello,
> 
> 
> 2018-01-25 17:41 GMT+01:00 Seán Coffey :
> 
> >
> > Classes nearer to those below were touched via JDK-8186080: Transform
> XML
> > interfaces
> > http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/rev/cb84156d54b2
> > http://hg.openjdk.java.net/jdk8u/jdk8u/jaxp/rev/08a44c164993
> >
> > This may be connected with some tools trying to redefine the classes
> > perhaps. Needs more investigating. Perhaps the XMLSchemaLoader
> changes are
> > a factor ?
> >
> 
> I have ben able to extract a minimal reproducer. It is not related to
> XMLUnit, only to powermock. If it instruments com.sun but not javax.xml
> (and other combinations) then it fails.
> 
> For details see the readme in this maven project:
> 
> https://github.com/ecki/reproduce-schemaex
> 
> I also found a way to make it work with both versions, so its no longer an
> issue for me, but there is definitely some changes (which might also be
> triggered in AppServers or OSGi containers with partially reconfigured
> implementations. Not sure if you want to investigate deeper).
> 
> Gruss
> Bernd
> 
> 
> Here is the stacktrace anyway:
> >>
> >> com.sun.org.apache.xerces.internal.impl.dv.DVFactoryException: Schema
> >> factory class
> >> com.sun.org.apache.xerces.internal.impl.dv.xs.SchemaDVFactoryImpl
> does
> >> not
> >> extend from SchemaDVFactory.
> >>  at
> >> com.sun.org.apache.xerces.internal.impl.dv.SchemaDVFactory.
> >> getInstance(SchemaDVFactory.java:75)
> >>  at
> >> com.sun.org.apache.xerces.internal.impl.dv.SchemaDVFactory.
> >> getInstance(SchemaDVFactory.java:57)
> >>  at
> >> com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaLoader.
> >> reset(XMLSchemaLoader.java:1024)
> >>  at
> >> com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaLoader.
> >> loadGrammar(XMLSchemaLoader.java:556)
> >>  at
> >> com.sun.org.apache.xerces.internal.impl.xs.XMLSchemaLoader.
> >> loadGrammar(XMLSchemaLoader.java:535)
> >>  at
> >> com.sun.org.apache.xerces.internal.jaxp.validation.XMLSchema
> >> Factory.newSchema(XMLSchemaFactory.java:254)
> >>  at javax.xml.validation.SchemaFactory.newSchema(SchemaFactory.
> >> java:638)
> >>  at javax.xml.validation.SchemaFactory.newSchema(SchemaFactory.
> >> java:654)
> >>  at
> >> com.seeburger.api.test.helpers.BuilderTestHelper.getCRSchema
> >> Validator(BuilderTestHelper.java:57)
> >>  at
> >> com.seeburger.api.test.helpers.BuilderTestHelper.validateAnd
> >> Compare(BuilderTestHelper.java:73)
> >>  at
> >> com.seeburger.api.test.KSMBuilderTest.testDeletePGP(KSMBuild
> >> erTest.java:196)
> >>  at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> >>  at
> >> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAcce
> >> ssorImpl.java:62)
> >>  at
> >> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMe
> >> thodAccessorImpl.java:43)
> >>  at java.lang.reflect.Method.invoke(Method.java:498)
> >>  at org.junit.internal.runners.TestMethod.invoke(TestMethod.java:68)
> >>  at
> >> org.powermock.modules.junit4.internal.impl.PowerMockJUnit44R
> >> unnerDelegateImpl$PowerMockJUnit44MethodRunner.runTestMethod
> >> (PowerMockJUnit44RunnerDelegateImpl.java:310)
> >>  at org.junit.internal.runners.MethodRoadie$2.run(MethodRoad

RE: Draft JEP: To use UTF-8 as the default charset for the Java virtual machine.

[Uwe Schindler]

Hi,

Thanks Alan for the link to this issue about FileReader/Writer!

Uwe

-
Uwe Schindler
uschind...@apache.org 
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
http://lucene.apache.org/

> -Original Message-
> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
> Behalf Of Alan Bateman
> Sent: Wednesday, February 21, 2018 2:55 PM
> To: Stephen Colebourne ; core-libs-
> d...@openjdk.java.net
> Subject: Re: Draft JEP: To use UTF-8 as the default charset for the Java 
> virtual
> machine.
> 
> On 21/02/2018 13:41, Stephen Colebourne wrote:
> > On 21 February 2018 at 13:37, Alan Bateman 
> wrote:
> >> The proposal is to eventually get to the point that the default charset
> >> cannot be changed. It will take several releases to get there due to the
> >> potential compatibility impact.
> > This seems like a reasonable strategy to solve the problem.
> >
> > I also agree that all locations where a default charset is used need
> > to have a method alongside that takes a CharSet, eg. FileWriter.
> >
> Good progress was made via JDK-8183743 [1] in Java SE 10 to add
> constructors and methods that take a Charset and eliminate the
> historical inconsistencies. The issue of legacy FileReader/FileWriter is
> linked from that JIRA issue.
> 
> -Alan
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8183743

Re: Draft JEP: To use UTF-8 as the default charset for the Java virtual machine.

[Robert Muir]

On Wed, Feb 21, 2018 at 8:55 AM, Alan Bateman  wrote:

> Good progress was made via JDK-8183743 [1] in Java SE 10 to add constructors
> and methods that take a Charset and eliminate the historical
> inconsistencies. The issue of legacy FileReader/FileWriter is linked from
> that JIRA issue.
>

Can we ensure we have CharsetDecoder/Encoder params too? There is
unfortunately a huge difference between InputStreamReader(x,
StandardCharsets.UTF_8) and InputStreamReader(x,
StandardCharsets.UTF_8.newDecoder()). And the silent replacement of
the "easier" one is probably not what most apps want.

Re: Draft JEP: To use UTF-8 as the default charset for the Java virtual machine.

[Alan Bateman]

On 21/02/2018 08:53, Uwe Schindler wrote:

:
The Java 7+ methods in java.nio.file.Files already ignore the default charset 
and always use UTF-8. How to proceed with those? Should they be changed to 
behave to the new mechanisms? I'd suggest to not do this, as its part of the 
spec (to use UTF-8) and should not rely on external forces, but I wanted to 
bring this in.


There is no proposal to change these methods.

-Alan

Re: RFR: Here are some URLClassPath patches

[Alan Bateman]

On 21/02/2018 06:08, Martin Buchholz wrote:

:

8198485: Simplify a URLClassPath constructor
http://cr.openjdk.java.net/~martin/webrevs/jdk/URLClassPath-simplify-constructor/ 


https://bugs.openjdk.java.net/browse/JDK-8198485

This one looks okay to me.

-Alan

Re: Draft JEP: To use UTF-8 as the default charset for the Java virtual machine.

[Alan Bateman]

On 21/02/2018 13:41, Stephen Colebourne wrote:

On 21 February 2018 at 13:37, Alan Bateman  wrote:

The proposal is to eventually get to the point that the default charset
cannot be changed. It will take several releases to get there due to the
potential compatibility impact.

This seems like a reasonable strategy to solve the problem.

I also agree that all locations where a default charset is used need
to have a method alongside that takes a CharSet, eg. FileWriter.

Good progress was made via JDK-8183743 [1] in Java SE 10 to add 
constructors and methods that take a Charset and eliminate the 
historical inconsistencies. The issue of legacy FileReader/FileWriter is 
linked from that JIRA issue.


-Alan

[1] https://bugs.openjdk.java.net/browse/JDK-8183743

Re: Draft JEP: To use UTF-8 as the default charset for the Java virtual machine.

[Stephen Colebourne]

On 21 February 2018 at 13:37, Alan Bateman  wrote:
> The proposal is to eventually get to the point that the default charset
> cannot be changed. It will take several releases to get there due to the
> potential compatibility impact.

This seems like a reasonable strategy to solve the problem.

I also agree that all locations where a default charset is used need
to have a method alongside that takes a CharSet, eg. FileWriter.

Stephen

Re: Draft JEP: To use UTF-8 as the default charset for the Java virtual machine.

[Alan Bateman]

On 21/02/2018 13:19, David Lloyd wrote:

I agree with Uwe and Remi; if the default is still changeable, the
problem doesn't go away, it simply becomes slightly more insidious.

The proposal is to eventually get to the point that the default charset 
cannot be changed. It will take several releases to get there due to the 
potential compatibility impact. This draft JEP is the first step to 
switch to UTF-8 by default. A first step has to allow it be changed in 
order to keep some existing code/deployments working. Sorry this isn't 
clear in the JEP yet, there several clarifications to this JEP that 
haven't been included yet (on my list, I didn't realize it would be 
discussed here this week).


-Alan

Re: Draft JEP: To use UTF-8 as the default charset for the Java virtual machine.

[David Lloyd]

I agree with Uwe and Remi; if the default is still changeable, the
problem doesn't go away, it simply becomes slightly more insidious.

On Wed, Feb 21, 2018 at 12:31 AM, Xueming Shen  wrote:
> This draft JEP contains a proposal to use UTF-8 as the default charset for
> the JVM, so that
> APIs that depend on the default charset behave consistently cross all
> platforms.
>
> For more details, please see:
> https://bugs.openjdk.java.net/browse/JDK-8187041
>
> Sherman



-- 
- DML

Re: RFR: Here are some URLClassPath patches

[Peter Levart]

Hi Martin,

I checked this one...

On 02/21/2018 07:08 AM, Martin Buchholz wrote:

8198484: URLClassPath should use an ArrayDeque instead of a Stack
http://cr.openjdk.java.net/~martin/webrevs/jdk/URLClassPath-ArrayDeque/
https://bugs.openjdk.java.net/browse/JDK-8198484


I admit I had to study the Stack API first as I don't regularly use it 
nowadays ;-). You seem to be using the following replacements consistently:


Stack vs. ArrayDequeue

push(e) vs. addFirst(e)
add(0, e) vs. addLast(e)
isEmpty() / pop() vs. pollFirst()

...but then in push(URL[]), you use:

push(e) vs. push(e)

Deque.addFirst(e) is equivalent to Deque.push(e), but it would be nice 
to keep using the same method consistently in one class.



Regards, Peter

RE: Draft JEP: To use UTF-8 as the default charset for the Java virtual machine.

[Remi Forax]

I agree with Uwe,
we should deprecate all methods/constructors that relies on the default 
charset. 

And we should do that before changing to use UTF-8 by default.

Remi


On February 21, 2018 8:53:54 AM UTC, Uwe Schindler  
wrote:
>Hi,
>
>> This draft JEP contains a proposal to use UTF-8 as the default
>charset
>> for the JVM, so that
>> APIs that depend on the default charset behave consistently cross all
>> platforms.
>> 
>> For more details, please see:
>> https://bugs.openjdk.java.net/browse/JDK-8187041
>
>Thanks for finally adding a JEP like this. Thanks also to Robert Muir
>for always insisting in fixing this problem! I have a few comments:
>
>The JEP should NOT cause that new APIs, which may convert between
>characters and bytes to no longer explicitly accept a charset. One
>example is the proposed ByteBuffer methods taking String. The default
>ones would work with UTF-8, but it should still be possible to an API
>user to always add a charset whenever there is a conversion between
>bytes and chars. This is especially important as the user may still
>change the default and breaking your app. Because the rule is still:
>Only YOU, the developer, know the charset of your stuff when you load a
>JAR resource file or pass a String to the network in a ByteBuffer!
>
>The biggest offenders on this is also given as an example: FileReader
>and FileWriter. Although both classes subclass
>InputStreamReader/OutputStreamWriter and just pass the right delegate
>to the superclass in the ctor, both classes are missing the possibility
>to specify a charset. Because of this, the use of FileReader and
>FileWriter is completely forbidden in many Apache projects (Apache
>Lucene, Solr, Elasticsearch, Apache TIKA,...). So I'd suggest to also
>fix the API here and just add the missing ctors.
>
>The Java 7+ methods in java.nio.file.Files already ignore the default
>charset and always use UTF-8. How to proceed with those? Should they be
>changed to behave to the new mechanisms? I'd suggest to not do this, as
>its part of the spec (to use UTF-8) and should not rely on external
>forces, but I wanted to bring this in.
>
>Changing the default would help many users, if they are actually using
>newer JDKs. For those with older versions (and compiling their code
>against older versions), you still have to avoid the default charsets.
>In addition, as you still can change the "default charset", any library
>developer reading resources from its own JAR file or passing Strings to
>network protocols cannot rely on the fact, that the default charset is
>really UTF-8! (a user may have changed it to something else). Because
>of this, Apache libraries will forbid usage of all methods using
>default charsets (and locales + timezones). The "changeable default"
>does not affect application developers (because they have in most cases
>control about the environment), but library developers should always be
>explicit!
>
>For this to work, I also want to do some "advertisement": All library
>projects should use the Forbidden-Apis Maven/Gradle/Ant plugin to scan
>their bytecode for offenders using default charsets, default locales or
>relying on default timezones. See the blog post about it [1] and the
>project page [2]. The tool is also useful to replace "jdeps" in
>projects with Java versions before 8, as it can scan your code for
>access to internal JDK APIs, too. See the documentation [3] and github
>wiki pages for useful examples. It may also be a good idea to mention
>it in the JEP as a "workaround" or "further reading".
>
>Finally: Because one can still change the default, I'd propose to
>deprecate all methods that use a default charset (unrelated to actually
>changing the default). Only if you do this, it would make tools like
>"forbiddenapis" irrelevant for library developers.
>
>And finally, finally: I'd also propose to change the default Locale to
>Locale.ROOT (same issues). The String.toLowerCase() in Turkish locales
>still break thousands of apps! But that's a different JEP - but I would
>strongly support it!
>
>Uwe
>
>[1]
>http://blog.thetaphi.de/2012/07/default-locales-default-charsets-and.html
>[2] https://github.com/policeman-tools/forbidden-apis
>[3] https://jenkins.thetaphi.de/job/Forbidden-APIs/javadoc/
>
>-
>Uwe Schindler
>uschind...@apache.org 
>ASF Member, Apache Lucene PMC / Committer
>Bremen, Germany
>http://lucene.apache.org/

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

RE: Draft JEP: To use UTF-8 as the default charset for the Java virtual machine.

[Uwe Schindler]

Hi,

> This draft JEP contains a proposal to use UTF-8 as the default charset
> for the JVM, so that
> APIs that depend on the default charset behave consistently cross all
> platforms.
> 
> For more details, please see:
> https://bugs.openjdk.java.net/browse/JDK-8187041

Thanks for finally adding a JEP like this. Thanks also to Robert Muir for 
always insisting in fixing this problem! I have a few comments:

The JEP should NOT cause that new APIs, which may convert between characters 
and bytes to no longer explicitly accept a charset. One example is the proposed 
ByteBuffer methods taking String. The default ones would work with UTF-8, but 
it should still be possible to an API user to always add a charset whenever 
there is a conversion between bytes and chars. This is especially important as 
the user may still change the default and breaking your app. Because the rule 
is still: Only YOU, the developer, know the charset of your stuff when you load 
a JAR resource file or pass a String to the network in a ByteBuffer!

The biggest offenders on this is also given as an example: FileReader and 
FileWriter. Although both classes subclass InputStreamReader/OutputStreamWriter 
and just pass the right delegate to the superclass in the ctor, both classes 
are missing the possibility to specify a charset. Because of this, the use of 
FileReader and FileWriter is completely forbidden in many Apache projects 
(Apache Lucene, Solr, Elasticsearch, Apache TIKA,...). So I'd suggest to also 
fix the API here and just add the missing ctors.

The Java 7+ methods in java.nio.file.Files already ignore the default charset 
and always use UTF-8. How to proceed with those? Should they be changed to 
behave to the new mechanisms? I'd suggest to not do this, as its part of the 
spec (to use UTF-8) and should not rely on external forces, but I wanted to 
bring this in.

Changing the default would help many users, if they are actually using newer 
JDKs. For those with older versions (and compiling their code against older 
versions), you still have to avoid the default charsets. In addition, as you 
still can change the "default charset", any library developer reading resources 
from its own JAR file or passing Strings to network protocols cannot rely on 
the fact, that the default charset is really UTF-8! (a user may have changed it 
to something else). Because of this, Apache libraries will forbid usage of all 
methods using default charsets (and locales + timezones). The "changeable 
default" does not affect application developers (because they have in most 
cases control about the environment), but library developers should always be 
explicit!

For this to work, I also want to do some "advertisement": All library projects 
should use the Forbidden-Apis Maven/Gradle/Ant plugin to scan their bytecode 
for offenders using default charsets, default locales or relying on default 
timezones. See the blog post about it [1] and the project page [2]. The tool is 
also useful to replace "jdeps" in projects with Java versions before 8, as it 
can scan your code for access to internal JDK APIs, too. See the documentation 
[3] and github wiki pages for useful examples. It may also be a good idea to 
mention it in the JEP as a "workaround" or "further reading".

Finally: Because one can still change the default, I'd propose to deprecate all 
methods that use a default charset (unrelated to actually changing the 
default). Only if you do this, it would make tools like "forbiddenapis" 
irrelevant for library developers.

And finally, finally: I'd also propose to change the default Locale to 
Locale.ROOT (same issues). The String.toLowerCase() in Turkish locales still 
break thousands of apps! But that's a different JEP - but I would strongly 
support it!

Uwe

[1] http://blog.thetaphi.de/2012/07/default-locales-default-charsets-and.html
[2] https://github.com/policeman-tools/forbidden-apis
[3] https://jenkins.thetaphi.de/job/Forbidden-APIs/javadoc/

-
Uwe Schindler
uschind...@apache.org 
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
http://lucene.apache.org/

Re: RFR: Here are some URLClassPath patches

[Alan Bateman]

On 21/02/2018 06:08, Martin Buchholz wrote:

:

8198480: Improve ClassLoaders static init block
http://cr.openjdk.java.net/~martin/webrevs/jdk/ClassLoaders-static/ 


https://bugs.openjdk.java.net/browse/JDK-8198480

This initializer has changed a few times and I agree it's a bit awkward 
to understand when there is a class path or not. So I think you're 
rejigging is okay, as is the rewording of the comment. The blank line at 
L45 was intentional, no need to remove that.


-Alan