Re: RFR(s): 8148425: strerror() function is not thread-safe

[Thomas Stüfe]

Hi Volker,

On Fri, Feb 26, 2016 at 8:00 PM, Volker Simonis 
wrote:

> Hi Thomas,
>
> it's good that somebody finally addresses this long standing issue :)
>
>
 Thanks :)


> However I wonder if it would be possible to align this effort with the
> core libraries group (CC'ed). They already fix this issue with:
>
> 8133249: Occasional SIGSEGV: non thread-safe use of strerr in
> getLastErrorString
> https://bugs.openjdk.java.net/browse/JDK-8133249
>
> I would be nice if we could use the same version in hotspot and the
> core libraries.
>
>
David already gave a good answer to this. For details see the comments in
the bug report.

In addition, there is the localization strerror(_s) provides: it is useless
and actually counterproductive in log files intended for the VM developers
themselves; but if those error messages get stuffed into Java Exception
texts and presented to the user, they might make more sense.

Kind Regards, Thomas


> Regards,
> Volker
>
>
> On Fri, Feb 26, 2016 at 5:05 PM, Thomas Stüfe 
> wrote:
> > Hi,
> >
> > please take a look at this proposed fix:
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8148425
> > Webrev:
> >
> http://cr.openjdk.java.net/~stuefe/webrevs/8141425-strerror-replacement/webrev.00/webrev/
> >
> > This adds a replacement function os::strerror() as a drop-in for
> > strerror(), which has a number of issues.
> >
> > strerror() is unsafe to use and may cause (and has caused) crashes in
> > multithreaded scenarios. It is also not ideal for log files because of
> the
> > implicit localization of the error messages.
> >
> > For details please see the discussion under the bug report.
> >
> > Please note that I did not yet change any call sites, although all call
> > sites in the os namespace should already use the new function. I wanted
> to
> > see whether there would be any general objections.
> >
> > Kind Regards, Thomas
>

Re: Replacement of Quicksort in java.util.Arrays with new Dual-Pivot Quicksort

[Stuart Marks]

Wow, is this a reply to a nearly seven-year-old email? [1]

I don't know if Vladimir Yaroslavskiy is still on core-libs-dev. I dug through 
tthe archives and found that he had posted a couple messages somewhat later [2] 
[3] using different email addresses:


  Vladimir Iaroslavski 
  Vladimir Iaroslavski 

You might try to contact him at one of these addresses. Note however that the 
more recent one is still over five years old.


He's also on LinkedIn. His profile says he works for Oracle, but as far as I can 
see he no longer does.


Good luck,

s'marks


[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2009-September/002630.html

[2] http://mail.openjdk.java.net/pipermail/core-libs-dev/2010-April/004133.html

[3] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2011-February/005821.html



On 2/23/16 6:05 PM, Jason C. McDonald wrote:

I think this is the best place to contact the original authors.

The link to Vladimir Yaroslavskiy's original whitepaper describing the
algorithm and its proofs was, unfortunately, broken. Using Archive.org's
Wayback Machine, I was able to get the last known revision. I reformatted
the document in LibreOffice for ease of reading, and fixed some minor
grammatical errors.

I also implemented the algorithm in an open-source (MIT License) C++
library, which I hope to release in the coming few months.

In order to make this algorithm and its proofs more easily accessible, I
would like to make the revised whitepaper publicly and freely available from
my own web servers, but I wanted to check with the original author(s) first.
Furthermore, I wanted to find out if there have been any revisions since the
22 September 2009 version of the whitepaper I acquired.

Thank you in advance!

Re: RFR(s): 8148425: strerror() function is not thread-safe

[David Holmes]

On 27/02/2016 5:00 AM, Volker Simonis wrote:

Hi Thomas,

it's good that somebody finally addresses this long standing issue :)

However I wonder if it would be possible to align this effort with the
core libraries group (CC'ed). They already fix this issue with:

8133249: Occasional SIGSEGV: non thread-safe use of strerr in getLastErrorString
https://bugs.openjdk.java.net/browse/JDK-8133249

I would be nice if we could use the same version in hotspot and the
core libraries.


I don't find this:

+#if defined(LINUX) && (defined(_GNU_SOURCE) || \
+ (defined(_POSIX_C_SOURCE) && _POSIX_C_SOURCE < 200112L \
+ && defined(_XOPEN_SOURCE) && _XOPEN_SOURCE < 600))
+extern int __xpg_strerror_r(int, char *, size_t);
+#define strerror_r(a, b, c) __xpg_strerror_r((a), (b), (c))
+#endif

particularly appealing at all - you can't even decode it without having 
a POSIX and XOpen version history in front of you :(


And a version that requires a buffer to be passed in is problematic in a 
number of usage contexts in hotspot - see the comments in the bug 
report. A simplified version that converts symbolic error values to 
their string equivalent is much more appealing - albeit fixing an issue 
that "should" be handled at the library level.


David
-


Regards,
Volker


On Fri, Feb 26, 2016 at 5:05 PM, Thomas Stüfe  wrote:

Hi,

please take a look at this proposed fix:

Bug: https://bugs.openjdk.java.net/browse/JDK-8148425
Webrev:
http://cr.openjdk.java.net/~stuefe/webrevs/8141425-strerror-replacement/webrev.00/webrev/

This adds a replacement function os::strerror() as a drop-in for
strerror(), which has a number of issues.

strerror() is unsafe to use and may cause (and has caused) crashes in
multithreaded scenarios. It is also not ideal for log files because of the
implicit localization of the error messages.

For details please see the discussion under the bug report.

Please note that I did not yet change any call sites, although all call
sites in the os namespace should already use the new function. I wanted to
see whether there would be any general objections.

Kind Regards, Thomas

Re: RFR: JDK-8149925 We don't need jdk.internal.ref.Cleaner any more

[Roger Riggs]

Hi Peter,

I think this cleans up all the points raised earlier.
The optimization for enqueuing from the reference queue seems ok to me 
and should be
more efficient than the previous implementation but I think Mandy or 
Alan should look at it also.


Thanks, Roger


On 2/25/2016 4:17 AM, Peter Levart wrote:

Hi Alan,

On 02/25/2016 09:00 AM, Alan Bateman wrote:



On 25/02/2016 07:24, Peter Levart wrote:

:

I kept the public boolean Cleaner.cleanNextPending() method which 
now only deals with enqueued Cleanable(s). I think this method might 
still be beneficial for public use in situations where cleanup 
actions take relatively long time to execute so that the rate of 
cleanup falls behind the rate of registration of new cleanup actions.
I think we need also need to look at the option where this is not 
public. I have concerns that it is exposing implementation to some 
extent and that may become an attractive nuisance in the future. This 
shouldn't be an issue for the NIO buffer usage, we can keep the usage 
via the shared secrets mechanism. I think this is what Mandy is 
suggesting too.


-Alan.


Sure, no problem. Here's a variant that keeps the 
Cleaner.cleanNextPending() method private and exposed via 
SharedSecrets to nio Bits but is otherwise equivalent to webrev.06:


http://cr.openjdk.java.net/~plevart/jdk9-dev/removeInternalCleaner/webrev.06priv/ 



Regards, Peter

Re: RFR(s): 8148425: strerror() function is not thread-safe

[Volker Simonis]

Hi Thomas,

it's good that somebody finally addresses this long standing issue :)

However I wonder if it would be possible to align this effort with the
core libraries group (CC'ed). They already fix this issue with:

8133249: Occasional SIGSEGV: non thread-safe use of strerr in getLastErrorString
https://bugs.openjdk.java.net/browse/JDK-8133249

I would be nice if we could use the same version in hotspot and the
core libraries.

Regards,
Volker


On Fri, Feb 26, 2016 at 5:05 PM, Thomas Stüfe  wrote:
> Hi,
>
> please take a look at this proposed fix:
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8148425
> Webrev:
> http://cr.openjdk.java.net/~stuefe/webrevs/8141425-strerror-replacement/webrev.00/webrev/
>
> This adds a replacement function os::strerror() as a drop-in for
> strerror(), which has a number of issues.
>
> strerror() is unsafe to use and may cause (and has caused) crashes in
> multithreaded scenarios. It is also not ideal for log files because of the
> implicit localization of the error messages.
>
> For details please see the discussion under the bug report.
>
> Please note that I did not yet change any call sites, although all call
> sites in the os namespace should already use the new function. I wanted to
> see whether there would be any general objections.
>
> Kind Regards, Thomas

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

[Stephen Colebourne]

It is important to also consider the case where the user wants to
format using HH:MM but parse seconds if they are provided.

As I said above, this is no different to SignStyle, where the user
requests something specific on format, but accepts anything on input.

The pattern is still used for formatting and strict parsing under
these changes. It is effectively ignored in lenient parsing (which is
the very definition of leniency).

Another way to look at it:

using a pattern of HH:MM and strict:
+02 - disallowed
+02:00 - allowed
+02:00:00 - disallowed

using a pattern of HH:mm and strict:
+02 - allowed
+02:00 - allowed
+02:00:00 - disallowed

using any pattern and lenient:
+02 - allowed
+02:00 - allowed
+02:00:00 - allowed

This covers pretty much anything a user needs when parsing.

Stephen


On 26 February 2016 at 17:38, Roger Riggs  wrote:
> Hi Stephen,
>
> Even in lenient mode the parser needs to stick to the fields provided in the
> pattern.
> If the caller intends to parse seconds, the pattern should include seconds.
> Otherwise the caller has not been able to specify their intent.
> That's consistent with lenient mode used in the other fields.
> Otherwise, the pattern is irrelevant except for whether it contains a ":"
> and makes
> the spec nearly useless.
>
> Roger
>
>
>
> On 2/26/2016 12:09 PM, Stephen Colebourne wrote:
>>
>> On 26 February 2016 at 15:00, Roger Riggs  wrote:
>>>
>>> Hi Stephen,
>>>
>>> It does not seem natural to me with a pattern of HHMM for it to parse
>>> more
>>> than 4 digits.
>>> I can see lenient modifying the behavior as it it were HHmm, but there is
>>> no
>>> indication in the pattern
>>> that seconds would be considered.  How it would it be implied from the
>>> spec?
>>
>> The spec is being expanded to define what happens. Previously it
>> didn't define it at all, and would throw an error.
>>
>> Lenient parsing typically accepts much more than the strict parsing.
>>
>> When parsing numbers, you may set the SignStyle to NEVER, but the sign
>> will still be parsed in lenient mode
>>
>> When parsing text, you may select the short output format, but any
>> length of text will be parsed in lenient mode.
>>
>> As such, it is very much in line with the behavour of the API to parse
>> a much broader format than the one requested in lenient mode. (None of
>> this affects strict mode).
>>
>> Stephen
>>
>>
>>> In the original issue, appendOffsetId is defined as using the +HH:MM:ss
>>> pattern and
>>> specific to ISO the MM should be allowed to be optional.   There is no
>>> question of parsing
>>> extra digits not included in the requested pattern.
>>>
>>> Separately, this is specifying the new lenient behavior of
>>> appendOffset(pattern, noffsetText).
>>> In that case, I don't think it will be understood that patterns 'shorter'
>>> than the input will
>>> gobble up extra digits and ':'s.
>>>
>>> Roger
>>>
>>>
>>>
>>>
>>>
>>> On 2/26/2016 9:42 AM, Stephen Colebourne wrote:
>>>
>>> Lenient can be however lenient we define it to be. Allowing minutes
>>> and seconds to be parsed when not specified in the pattern is the key
>>> part of the change. Whether the parser copes with both colons and
>>> no-colons is the choice at hand here. It seems to me that since the
>>> parser can easily handle figuring out whether the colon is present or
>>> not, we should just allow the parser to be fully lenient.
>>>
>>> Stephen
>>>
>>>
>>> On 26 February 2016 at 14:15, Roger Riggs  wrote:
>>>
>>> HI Stephen,
>>>
>>> How lenient is lenient supposed to be? Looking at the offset test cases,
>>> it
>>> seems to allow  minutes
>>> and seconds digits to be parsed even if the pattern did not include them.
>>>
>>> + @DataProvider(name="lenientOffsetParseData")
>>> + Object[][] data_lenient_offset_parse() {
>>> + return new Object[][] {
>>> + {"+HH", "+01", 3600},
>>> + {"+HH", "+0101", 3660},
>>> + {"+HH", "+010101", 3661},
>>> + {"+HH", "+01", 3600},
>>> + {"+HH", "+01:01", 3660},
>>> + {"+HH", "+01:01:01", 3661},
>>>
>>> Thanks, Roger
>>>
>>>
>>>
>>> On 2/26/2016 6:16 AM, Stephen Colebourne wrote:
>>>
>>> I don't think this is quite right.
>>>
>>> if ((length > position + 3) && (text.charAt(position + 3) == ':')) {
>>>  parseType = 10;
>>> }
>>>
>>> This code will *always* select type 10 (colons) if a colon is found at
>>> position+3. Whereas the spec now says that it should only do this if
>>> the pattern is "HH". For other patterns, the colon/no-colon choice is
>>> defined to be based on the pattern.
>>>
>>> That said, I'm thinking it is better to make the spec more lenient to
>>> match the behaviour as implemented:
>>>
>>>
>>> When parsing in lenient mode, only the hours are mandatory - minutes
>>> and seconds are optional. If the character after the hour digits is a
>>> colon
>>> then the parser will parse using the pattern "HH:mm:ss", otherwise the
>>> parser will parse using the pattern "HHmmss".
>>>
>>>
>>> Additional TCKDateTimeFormatterBuilder tests will be needed to
>>> demonstrate the above.

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

[Roger Riggs]

Hi Stephen,

Even in lenient mode the parser needs to stick to the fields provided in 
the pattern.

If the caller intends to parse seconds, the pattern should include seconds.
Otherwise the caller has not been able to specify their intent.
That's consistent with lenient mode used in the other fields.
Otherwise, the pattern is irrelevant except for whether it contains a 
":" and makes

the spec nearly useless.

Roger


On 2/26/2016 12:09 PM, Stephen Colebourne wrote:

On 26 February 2016 at 15:00, Roger Riggs  wrote:

Hi Stephen,

It does not seem natural to me with a pattern of HHMM for it to parse more
than 4 digits.
I can see lenient modifying the behavior as it it were HHmm, but there is no
indication in the pattern
that seconds would be considered.  How it would it be implied from the spec?

The spec is being expanded to define what happens. Previously it
didn't define it at all, and would throw an error.

Lenient parsing typically accepts much more than the strict parsing.

When parsing numbers, you may set the SignStyle to NEVER, but the sign
will still be parsed in lenient mode

When parsing text, you may select the short output format, but any
length of text will be parsed in lenient mode.

As such, it is very much in line with the behavour of the API to parse
a much broader format than the one requested in lenient mode. (None of
this affects strict mode).

Stephen



In the original issue, appendOffsetId is defined as using the +HH:MM:ss
pattern and
specific to ISO the MM should be allowed to be optional.   There is no
question of parsing
extra digits not included in the requested pattern.

Separately, this is specifying the new lenient behavior of
appendOffset(pattern, noffsetText).
In that case, I don't think it will be understood that patterns 'shorter'
than the input will
gobble up extra digits and ':'s.

Roger





On 2/26/2016 9:42 AM, Stephen Colebourne wrote:

Lenient can be however lenient we define it to be. Allowing minutes
and seconds to be parsed when not specified in the pattern is the key
part of the change. Whether the parser copes with both colons and
no-colons is the choice at hand here. It seems to me that since the
parser can easily handle figuring out whether the colon is present or
not, we should just allow the parser to be fully lenient.

Stephen


On 26 February 2016 at 14:15, Roger Riggs  wrote:

HI Stephen,

How lenient is lenient supposed to be? Looking at the offset test cases, it
seems to allow  minutes
and seconds digits to be parsed even if the pattern did not include them.

+ @DataProvider(name="lenientOffsetParseData")
+ Object[][] data_lenient_offset_parse() {
+ return new Object[][] {
+ {"+HH", "+01", 3600},
+ {"+HH", "+0101", 3660},
+ {"+HH", "+010101", 3661},
+ {"+HH", "+01", 3600},
+ {"+HH", "+01:01", 3660},
+ {"+HH", "+01:01:01", 3661},

Thanks, Roger



On 2/26/2016 6:16 AM, Stephen Colebourne wrote:

I don't think this is quite right.

if ((length > position + 3) && (text.charAt(position + 3) == ':')) {
 parseType = 10;
}

This code will *always* select type 10 (colons) if a colon is found at
position+3. Whereas the spec now says that it should only do this if
the pattern is "HH". For other patterns, the colon/no-colon choice is
defined to be based on the pattern.

That said, I'm thinking it is better to make the spec more lenient to
match the behaviour as implemented:


When parsing in lenient mode, only the hours are mandatory - minutes
and seconds are optional. If the character after the hour digits is a
colon
then the parser will parse using the pattern "HH:mm:ss", otherwise the
parser will parse using the pattern "HHmmss".


Additional TCKDateTimeFormatterBuilder tests will be needed to
demonstrate the above. There should also be a test for data following
the lenient parse. The following should all succeed:


DateTimeFormatterBuilder().parseLenient().appendOffset("HH:MM").appendZoneId();
"+01:00Europe/London"
"+0100Europe/London"


DateTimeFormatterBuilder().parseLenient().appendOffset("HH:MM").appendLiteral(":").appendZoneId();
"+01:Europe/London"

Note this special case, where the colon affects the parse type, but is
not ultimately part of the offset, thus it is left to match the
appendLiteral(":")

You may want to think of some additional nasty edge cases!

Stephen

On 25 February 2016 at 15:44, nadeesh tv  wrote:

Hi all,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8032051/webrev.02/

Thanks and Regards,
Nadeesh

On 2/23/2016 5:17 PM, Stephen Colebourne wrote:

Thanks for the changes.

In `DateTimeFormatter`, the code should be

.parseLenient()
.appendOffsetId()
.parseStrict()

and the same in the other case. This ensures that existing callers who
then embed the formatter in another formatter (like the
ZONED_DATE_TIME constant) are unaffected.


The logic for lenient parsing does not look right as it only handles
types 5 and 6. This table shows the mappings needed:

"+HH",  -> "+HHmmss" or "+HH:mm:ss"
"+HHmm",  -> "+HHmmss",

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

[Stephen Colebourne]

On 26 February 2016 at 15:00, Roger Riggs  wrote:
> Hi Stephen,
>
> It does not seem natural to me with a pattern of HHMM for it to parse more
> than 4 digits.
> I can see lenient modifying the behavior as it it were HHmm, but there is no
> indication in the pattern
> that seconds would be considered.  How it would it be implied from the spec?

The spec is being expanded to define what happens. Previously it
didn't define it at all, and would throw an error.

Lenient parsing typically accepts much more than the strict parsing.

When parsing numbers, you may set the SignStyle to NEVER, but the sign
will still be parsed in lenient mode

When parsing text, you may select the short output format, but any
length of text will be parsed in lenient mode.

As such, it is very much in line with the behavour of the API to parse
a much broader format than the one requested in lenient mode. (None of
this affects strict mode).

Stephen


> In the original issue, appendOffsetId is defined as using the +HH:MM:ss
> pattern and
> specific to ISO the MM should be allowed to be optional.   There is no
> question of parsing
> extra digits not included in the requested pattern.
>
> Separately, this is specifying the new lenient behavior of
> appendOffset(pattern, noffsetText).
> In that case, I don't think it will be understood that patterns 'shorter'
> than the input will
> gobble up extra digits and ':'s.
>
> Roger
>
>
>
>
>
> On 2/26/2016 9:42 AM, Stephen Colebourne wrote:
>
> Lenient can be however lenient we define it to be. Allowing minutes
> and seconds to be parsed when not specified in the pattern is the key
> part of the change. Whether the parser copes with both colons and
> no-colons is the choice at hand here. It seems to me that since the
> parser can easily handle figuring out whether the colon is present or
> not, we should just allow the parser to be fully lenient.
>
> Stephen
>
>
> On 26 February 2016 at 14:15, Roger Riggs  wrote:
>
> HI Stephen,
>
> How lenient is lenient supposed to be? Looking at the offset test cases, it
> seems to allow  minutes
> and seconds digits to be parsed even if the pattern did not include them.
>
> + @DataProvider(name="lenientOffsetParseData")
> + Object[][] data_lenient_offset_parse() {
> + return new Object[][] {
> + {"+HH", "+01", 3600},
> + {"+HH", "+0101", 3660},
> + {"+HH", "+010101", 3661},
> + {"+HH", "+01", 3600},
> + {"+HH", "+01:01", 3660},
> + {"+HH", "+01:01:01", 3661},
>
> Thanks, Roger
>
>
>
> On 2/26/2016 6:16 AM, Stephen Colebourne wrote:
>
> I don't think this is quite right.
>
> if ((length > position + 3) && (text.charAt(position + 3) == ':')) {
> parseType = 10;
> }
>
> This code will *always* select type 10 (colons) if a colon is found at
> position+3. Whereas the spec now says that it should only do this if
> the pattern is "HH". For other patterns, the colon/no-colon choice is
> defined to be based on the pattern.
>
> That said, I'm thinking it is better to make the spec more lenient to
> match the behaviour as implemented:
>
>
> When parsing in lenient mode, only the hours are mandatory - minutes
> and seconds are optional. If the character after the hour digits is a
> colon
> then the parser will parse using the pattern "HH:mm:ss", otherwise the
> parser will parse using the pattern "HHmmss".
>
>
> Additional TCKDateTimeFormatterBuilder tests will be needed to
> demonstrate the above. There should also be a test for data following
> the lenient parse. The following should all succeed:
>
>
> DateTimeFormatterBuilder().parseLenient().appendOffset("HH:MM").appendZoneId();
> "+01:00Europe/London"
> "+0100Europe/London"
>
>
> DateTimeFormatterBuilder().parseLenient().appendOffset("HH:MM").appendLiteral(":").appendZoneId();
> "+01:Europe/London"
>
> Note this special case, where the colon affects the parse type, but is
> not ultimately part of the offset, thus it is left to match the
> appendLiteral(":")
>
> You may want to think of some additional nasty edge cases!
>
> Stephen
>
> On 25 February 2016 at 15:44, nadeesh tv  wrote:
>
> Hi all,
> Please see the updated webrev
> http://cr.openjdk.java.net/~ntv/8032051/webrev.02/
>
> Thanks and Regards,
> Nadeesh
>
> On 2/23/2016 5:17 PM, Stephen Colebourne wrote:
>
> Thanks for the changes.
>
> In `DateTimeFormatter`, the code should be
>
> .parseLenient()
> .appendOffsetId()
> .parseStrict()
>
> and the same in the other case. This ensures that existing callers who
> then embed the formatter in another formatter (like the
> ZONED_DATE_TIME constant) are unaffected.
>
>
> The logic for lenient parsing does not look right as it only handles
> types 5 and 6. This table shows the mappings needed:
>
> "+HH",  -> "+HHmmss" or "+HH:mm:ss"
> "+HHmm",  -> "+HHmmss",
> "+HH:mm",  -> "+HH:mm:ss",
> "+HHMM",  -> "+HHmmss",
> "+HH:MM",  -> "+HH:mm:ss",
> "+HHMMss",  -> "+HHmmss",
> "+HH:MM:ss",  -> "+HH:mm:ss",
> "+HHMMSS",  -> "+HHmmss",
> "+HH:MM:SS",  -> "+HH:mm:ss",
> "+HHmmss",
> "+HH:mm:ss",
>
> Note that the "+HH"

RE: RFR 8124977 cmdline encoding challenges on Windows

[Vladimir Shcherbakov]

Hi Sherman,

1) If you can point out the regression test cases that are compromised by the 
fix - it would be very helpful;
2) From my understanding you can change default encoding by starting java with 
-Dsun.jnu.encoding=UTF-8 - this is well known feature that never caused 
problems (javac doesn't have such a switch );
3) If you state that java is non-Unicode on Windows by nature - the issue  
JDK-8124977 is a feature not a bug  :)

Thanks,
Vladimir.

-Original Message-
From: Xueming Shen [mailto:xueming.s...@oracle.com] 
Sent: Tuesday, February 23, 2016 8:54 PM
To: Vladimir Shcherbakov 
Cc: Naoto Sato ; Kumar Srinivasan 
; Martin Sawicki ; 
core-libs-dev Libs 
Subject: Re: RFR 8124977 cmdline encoding challenges on Windows

Vladimir,

sun.jnu.encoding is used by
JNU_NewStringPlatform/JNU_GetStringPlatformChars. The JNU_ pair is "widely" 
used by the various native library code to convert between the jstring and 
native char*, with the assumption that the "platform encoding" for the native 
char* is the "default" encoding used by the underlying platform/os APIs that 
takes char* parameters or return char* values, in case of Windows, it's the 
code page decided by the system locale. We have migrated certain areas 
completely to use the "W" version/WChar APIs, such as the 
https://na01.safelinks.protection.outlook.com/?url=java.io&data=01%7c01%7cvlashch%40microsoft.com%7c635061d867af4ad4105008d33cd679e7%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=l4G1yzKKhniPRYJvBsGxchsBZvuWliVq8tILa0pLoY8%3d,
 the system properties initialization, but I'm think lots of areas still work 
on the "A" apis, especially I think the "char*" interface between the jvm and 
the libraries is still the the "ansi" codepage, not the utf8. Those work on 
utf8 have their names explicitly named as "xyzUTF" or similar.

For example, the "java_home_dir" path used in 
libjava/TimeZone.c/getSystemTimeZoneID/
TimeZone_md.c/findJavaTZ_md is encoded frm jstring java_home to char* via 
JNU_GetSTringPlatformChars.
Simply change/hardcode the jnu_sun.encoding to utf8 probably will cause the 
timezone code stop to work if the java_home_dir path has some non-ascii 
characters in it (the jdk/jre is installed in a Japanese/Chinese directory, for 
example).

A quick "grep" indicates java.desktop/windows/native/libawt/windows
package has a heavily
usage of the JNU_ pair as well. I'm not sure if this awt implementation is 
still being used though :-)

Before we clear all these internal "StringPlatform" use cases (I'm not sure if 
they are also used by external), I don't think we can simply set the 
sun.jnu.encoding to utf8, though it's very attractive.

Thanks,
-Sherman

On 2/23/16 4:34 PM, Naoto Sato wrote:
> Hi Vladimir,
>
> I think it would work fine with the Java launcher, but what about 
> other areas, which may rely on the native encodings? Java runtime is 
> in itself a "non-Unicode" application, so still there may be the area 
> affected by hardcoding "UTF-8" as the native encoding. Have you 
> checked in such cases? Sherman, will you comment on this too?
>
> Naoto
>
> On 2/23/16 2:12 PM, Vladimir Shcherbakov wrote:
>> Hi Naoto,
>>
>> 1) The system locale determines which code page is used on the system 
>> by default on operating systems that use Unicode as their native 
>> encoding (all OSes  from Windows 2000 to Windows 10) to convert text 
>> data from Unicode to code page whenever dealing with legacy 
>> non-Unicode applications. Only applications that do not use Unicode 
>> as their default character-encoding mechanism are affected by this 
>> setting; therefore, applications that are already Unicode-encoded can 
>> safely ignore the value and functionality of this setting.
>>
>> 2) The fundamental representation of text in Windows NT-based 
>> operating systems is UTF-16, and the WCHAR data type is a UTF-16 code 
>> unit. Java launcher, from the other side, uses CHAR as a code unit - 
>> so to use UNICODE charset with Java launcher we had to encode entire 
>> command line with UTF-8 (convert from UTF-16 to UTF-8). After that 
>> step we can state that Java launcher is Unicode-encoded and can 
>> safely ignore the value and functionality of the system locale. To 
>> let JVM know that we use UTF-8 as a default UNICODE encoding for 
>> platform string  - we assign the value to sprops.sun_jnu_encoding 
>> property (mac osx does the same) instead of reading system locale 
>> code page.
>>
>> The main idea of the fix was to change the way of how java and javac 
>> works with so called platform string on Windows. Before the fix the 
>> platform string was read as ANSI encoded - that's why the system 
>> locale code page was very important. The sun.jnu.encoding property is 
>> responsible for storing the platform string encoding. On Windows the 
>> property could be set with the system locale but the system locale 
>> doesn't support (by design) UTF-8 or with -Dsun.jnu.encoding switch, 
>> but the switch only works with java not with javac,

RE: RFR 8124977 cmdline encoding challenges on Windows

[Vladimir Shcherbakov]

Hi Naoto,

1) The system locale determines which code page is used on the system by 
default on operating systems that use Unicode as their native encoding (all 
OSes  from Windows 2000 to Windows 10) to convert text data from Unicode to 
code page whenever dealing with legacy non-Unicode applications. Only 
applications that do not use Unicode as their default character-encoding 
mechanism are affected by this setting; therefore, applications that are 
already Unicode-encoded can safely ignore the value and functionality of this 
setting.

2) The fundamental representation of text in Windows NT-based operating systems 
is UTF-16, and the WCHAR data type is a UTF-16 code unit. Java launcher, from 
the other side, uses CHAR as a code unit - so to use UNICODE charset with Java 
launcher we had to encode entire command line with UTF-8 (convert from UTF-16 
to UTF-8). After that step we can state that Java launcher is Unicode-encoded 
and can safely ignore the value and functionality of the system locale. To let 
JVM know that we use UTF-8 as a default UNICODE encoding for platform string  - 
we assign the value to sprops.sun_jnu_encoding property (mac osx does the same) 
instead of reading system locale code page. 

The main idea of the fix was to change the way of how java and javac works with 
so called platform string on Windows. Before the fix the platform string was 
read as ANSI encoded - that's why the system locale code page was very 
important. The sun.jnu.encoding property is responsible for storing the 
platform string encoding. On Windows the property could be set with the system 
locale but the system locale doesn't support (by design) UTF-8 or with 
-Dsun.jnu.encoding switch, but the switch only works with java not with javac, 
and the switch was useless for ANSI encoded platform string.

Thanks,
Vladimir.

-Original Message-
From: Naoto Sato [mailto:naoto.s...@oracle.com] 
Sent: Tuesday, February 23, 2016 10:47 AM
To: Kumar Srinivasan ; Vladimir Shcherbakov 
; SHEN,XUEMING 
Cc: Martin Sawicki ; core-libs-dev Libs 

Subject: Re: RFR 8124977 cmdline encoding challenges on Windows

Hello,

Sorry if this has already been discussed, but this is my first time looking at 
the fix. In java_props_md.c, sprops.sun_jnu_encoding is now always "UTF-8". Is 
it always the case? What if the system admin switches the locale for 
"non-Unicode" applications in the Windows control panel?

Naoto

On 2/22/16 8:00 AM, Kumar Srinivasan wrote:
>
> Hi Naoto, Sherman,  can you please take a look.
> I tested with the jprt build and test all tests pass.
>
> Hi Vladimir, et. al.,
>
> It appears that there has been more simplifications from the previous 
> webrev.04. :-)
>
> It would've helped if you highlight the changes you have made from the 
> previous revision, unfortunately this is one of the deficiencies of 
> webrev.
>
> There are some inconsistencies in the coding conventions:
>
> parse_manifest.c
> + if (q == 0) return -1;
>
> we expect the return to be on the next line.
>
> similarly main.c
>
> if (0 == q)
> {
>
> I can fix those up. If I were to push this change, who should I 
> attribute the changes to ? ie. in the Contributed-by: line of the 
> commit info ?
> Please note these have to be email addresses of the contributors.
>
> Thanks
> Kumar
>
>> Hi Kumar,
>>
>> We posted another web review here:
>> https://na01.safelinks.protection.outlook.com/?url=http:%2f%2fcr.open
>> jdk.java.net%2f~kshoop%2f8124977%2fwebrev.05%2f&data=01%7C01%7Cvlashc
>> h%40microsoft.com%7Cf33316507f214e013a4008d33c81c785%7C72f988bf86f141
>> af91ab2d7cd011db47%7C1&sdata=%2fTQaWH0KGurgvZcdCQRZHSyaftjlMsW5FVc%2f
>> 14Wc5fA%3d
>>
>> The patch was successfully tested.
>>
>> Test details:
>> * Regression tests folder: jdk/test/tools/launcher/
>> * Builds were used: windows-x86_64-normal-server-fastdebug,
>> windows-x86_64-normal-server-release, 
>> windows-x86-normal-server-release;
>> * Platforms were used:  Windows 7(64 bit), Windows 8.1, Windows 
>> Server
>> 2012 R2 DC, Windows 10 ;
>> * System locales were used: English (United States), Persian, 
>> Japanese (Japan), Chinese (Traditional, Taiwan), Russian (Russia);
>>
>> Thanks,
>> Vladimir.
>>
>> -Original Message-
>> From: Martin Sawicki
>> Sent: Thursday, January 14, 2016 11:34 AM
>> To: Kumar Srinivasan ; Vladimir 
>> Shcherbakov 
>> Cc: core-libs-dev Libs ; Naoto Sato 
>> 
>> Subject: RE: RFR 8124977 cmdline encoding challenges on Windows
>>
>> Thanks for the feedback.
>> Investigating the regression failure.
>> We'll get back as soon as we figure this out.  (and yes, we'll run 
>> this through some localized Windows VMs)
>>
>> Cheers
>>
>> -Original Message-
>> From: Kumar Srinivasan [mailto:kumar.x.sriniva...@oracle.com]
>> Sent: Tuesday, January 12, 2016 2:35 PM
>> To: Martin Sawicki ; Vladimir Shcherbakov 
>> 
>> Cc: core-libs-dev Libs ; Naoto Sato 
>> 
>> Subject: Re: RFR 8124977 cmdline encoding challenges on Windows
>>
>> Hi Martin, Vladimir,
>>
>> It

Re: RFR 9: 8150346 : java/lang/ProcessHandle/Infotest.java failed: startTime after process spawn completed

[Claes Redestad]

Looks good to me.

/Claes

On 2016-02-26 16:02, Roger Riggs wrote:

Ping...

On 2/23/2016 5:30 PM, Roger Riggs wrote:
Please review a test fix to resolve a timing race with the start time 
of a spawned process.


Webrev:
  http://cr.openjdk.java.net/~rriggs/webrev-start-8150346/

Jira:
   https://bugs.openjdk.java.net/browse/JDK-8150346

Thanks, Roger

Re: RFR 9: 8150346 : java/lang/ProcessHandle/Infotest.java failed: startTime after process spawn completed

[Roger Riggs]

Ping...

On 2/23/2016 5:30 PM, Roger Riggs wrote:
Please review a test fix to resolve a timing race with the start time 
of a spawned process.


Webrev:
  http://cr.openjdk.java.net/~rriggs/webrev-start-8150346/

Jira:
   https://bugs.openjdk.java.net/browse/JDK-8150346

Thanks, Roger

RFR: JDK-8150704 XALAN: ERROR: 'No more DTM IDs are available' when transforming with lots of temporary result trees

[Langer, Christoph]

Hi,

I've created a fix proposal for the issue I have reported in this bug:
https://bugs.openjdk.java.net/browse/JDK-8150704

The webrev can be found here:
http://cr.openjdk.java.net/~clanger/webrevs/8150704.1/

The Xalan parser would eventually run out of DTM IDs if xsl transformations 
involve lots of temporary result trees. Those are never released although they 
could. A testcase is included for this. I've also done some cleanups in the 
Xalan code and in the tests.

Thanks in advance for looking at this :)

Best regards
Christoph

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

[Roger Riggs]

Hi Stephen,

It does not seem natural to me with a pattern of HHMM for it to parse 
more than 4 digits.
I can see lenient modifying the behavior as it it were HHmm, but there 
is no indication in the pattern

that seconds would be considered.  How it would it be implied from the spec?

In the original issue, appendOffsetId is defined as using the +HH:MM:ss 
pattern and
specific to ISO the MM should be allowed to be optional.   There is no 
question of parsing

extra digits not included in the requested pattern.

Separately, this is specifying the new lenient behavior of 
appendOffset(pattern, noffsetText).
In that case, I don't think it will be understood that patterns 
'shorter' than the input will

gobble up extra digits and ':'s.

Roger




On 2/26/2016 9:42 AM, Stephen Colebourne wrote:

Lenient can be however lenient we define it to be. Allowing minutes
and seconds to be parsed when not specified in the pattern is the key
part of the change. Whether the parser copes with both colons and
no-colons is the choice at hand here. It seems to me that since the
parser can easily handle figuring out whether the colon is present or
not, we should just allow the parser to be fully lenient.

Stephen


On 26 February 2016 at 14:15, Roger Riggs  wrote:

HI Stephen,

How lenient is lenient supposed to be? Looking at the offset test cases, it
seems to allow  minutes
and seconds digits to be parsed even if the pattern did not include them.

+ @DataProvider(name="lenientOffsetParseData")
+ Object[][] data_lenient_offset_parse() {
+ return new Object[][] {
+ {"+HH", "+01", 3600},
+ {"+HH", "+0101", 3660},
+ {"+HH", "+010101", 3661},
+ {"+HH", "+01", 3600},
+ {"+HH", "+01:01", 3660},
+ {"+HH", "+01:01:01", 3661},

Thanks, Roger



On 2/26/2016 6:16 AM, Stephen Colebourne wrote:

I don't think this is quite right.

if ((length > position + 3) && (text.charAt(position + 3) == ':')) {
 parseType = 10;
}

This code will *always* select type 10 (colons) if a colon is found at
position+3. Whereas the spec now says that it should only do this if
the pattern is "HH". For other patterns, the colon/no-colon choice is
defined to be based on the pattern.

That said, I'm thinking it is better to make the spec more lenient to
match the behaviour as implemented:


When parsing in lenient mode, only the hours are mandatory - minutes
and seconds are optional. If the character after the hour digits is a
colon
then the parser will parse using the pattern "HH:mm:ss", otherwise the
parser will parse using the pattern "HHmmss".


Additional TCKDateTimeFormatterBuilder tests will be needed to
demonstrate the above. There should also be a test for data following
the lenient parse. The following should all succeed:


DateTimeFormatterBuilder().parseLenient().appendOffset("HH:MM").appendZoneId();
"+01:00Europe/London"
"+0100Europe/London"


DateTimeFormatterBuilder().parseLenient().appendOffset("HH:MM").appendLiteral(":").appendZoneId();
"+01:Europe/London"

Note this special case, where the colon affects the parse type, but is
not ultimately part of the offset, thus it is left to match the
appendLiteral(":")

You may want to think of some additional nasty edge cases!

Stephen

On 25 February 2016 at 15:44, nadeesh tv  wrote:

Hi all,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8032051/webrev.02/

Thanks and Regards,
Nadeesh

On 2/23/2016 5:17 PM, Stephen Colebourne wrote:

Thanks for the changes.

In `DateTimeFormatter`, the code should be

.parseLenient()
.appendOffsetId()
.parseStrict()

and the same in the other case. This ensures that existing callers who
then embed the formatter in another formatter (like the
ZONED_DATE_TIME constant) are unaffected.


The logic for lenient parsing does not look right as it only handles
types 5 and 6. This table shows the mappings needed:

"+HH",  -> "+HHmmss" or "+HH:mm:ss"
"+HHmm",  -> "+HHmmss",
"+HH:mm",  -> "+HH:mm:ss",
"+HHMM",  -> "+HHmmss",
"+HH:MM",  -> "+HH:mm:ss",
"+HHMMss",  -> "+HHmmss",
"+HH:MM:ss",  -> "+HH:mm:ss",
"+HHMMSS",  -> "+HHmmss",
"+HH:MM:SS",  -> "+HH:mm:ss",
"+HHmmss",
"+HH:mm:ss",

Note that the "+HH" pattern is a special case, as we don't know
whether to use the colon or non-colon pattern. Whether to require
colon or not is based on whether the next character after the HH is a
colon or not.

Proposed appendOffsetId() Javadoc:

* Appends the zone offset, such as '+01:00', to the formatter.
* 
* This appends an instruction to format/parse the offset ID to the
builder.
* This is equivalent to calling {@code appendOffset("+HH:MM:ss", "Z")}.
* See {@link #appendOffset(String, String)} for details on formatting
and parsing.

Proposed appendOffset(String, String) Javadoc:

* During parsing, the offset...

changed to:

* When parsing in strict mode, the input must contain the mandatory
and optional elements are defined by the specified pattern.
* If the offset cannot be parsed then an exception is thrown unless
the section of the formatter is optional.
* 
* When p

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

[Stephen Colebourne]

Lenient can be however lenient we define it to be. Allowing minutes
and seconds to be parsed when not specified in the pattern is the key
part of the change. Whether the parser copes with both colons and
no-colons is the choice at hand here. It seems to me that since the
parser can easily handle figuring out whether the colon is present or
not, we should just allow the parser to be fully lenient.

Stephen


On 26 February 2016 at 14:15, Roger Riggs  wrote:
> HI Stephen,
>
> How lenient is lenient supposed to be? Looking at the offset test cases, it
> seems to allow  minutes
> and seconds digits to be parsed even if the pattern did not include them.
>
> + @DataProvider(name="lenientOffsetParseData")
> + Object[][] data_lenient_offset_parse() {
> + return new Object[][] {
> + {"+HH", "+01", 3600},
> + {"+HH", "+0101", 3660},
> + {"+HH", "+010101", 3661},
> + {"+HH", "+01", 3600},
> + {"+HH", "+01:01", 3660},
> + {"+HH", "+01:01:01", 3661},
>
> Thanks, Roger
>
>
>
> On 2/26/2016 6:16 AM, Stephen Colebourne wrote:
>>
>> I don't think this is quite right.
>>
>> if ((length > position + 3) && (text.charAt(position + 3) == ':')) {
>> parseType = 10;
>> }
>>
>> This code will *always* select type 10 (colons) if a colon is found at
>> position+3. Whereas the spec now says that it should only do this if
>> the pattern is "HH". For other patterns, the colon/no-colon choice is
>> defined to be based on the pattern.
>>
>> That said, I'm thinking it is better to make the spec more lenient to
>> match the behaviour as implemented:
>>
>>
>> When parsing in lenient mode, only the hours are mandatory - minutes
>> and seconds are optional. If the character after the hour digits is a
>> colon
>> then the parser will parse using the pattern "HH:mm:ss", otherwise the
>> parser will parse using the pattern "HHmmss".
>>
>>
>> Additional TCKDateTimeFormatterBuilder tests will be needed to
>> demonstrate the above. There should also be a test for data following
>> the lenient parse. The following should all succeed:
>>
>>
>> DateTimeFormatterBuilder().parseLenient().appendOffset("HH:MM").appendZoneId();
>> "+01:00Europe/London"
>> "+0100Europe/London"
>>
>>
>> DateTimeFormatterBuilder().parseLenient().appendOffset("HH:MM").appendLiteral(":").appendZoneId();
>> "+01:Europe/London"
>>
>> Note this special case, where the colon affects the parse type, but is
>> not ultimately part of the offset, thus it is left to match the
>> appendLiteral(":")
>>
>> You may want to think of some additional nasty edge cases!
>>
>> Stephen
>>
>> On 25 February 2016 at 15:44, nadeesh tv  wrote:
>>>
>>> Hi all,
>>> Please see the updated webrev
>>> http://cr.openjdk.java.net/~ntv/8032051/webrev.02/
>>>
>>> Thanks and Regards,
>>> Nadeesh
>>>
>>> On 2/23/2016 5:17 PM, Stephen Colebourne wrote:

 Thanks for the changes.

 In `DateTimeFormatter`, the code should be

 .parseLenient()
 .appendOffsetId()
 .parseStrict()

 and the same in the other case. This ensures that existing callers who
 then embed the formatter in another formatter (like the
 ZONED_DATE_TIME constant) are unaffected.


 The logic for lenient parsing does not look right as it only handles
 types 5 and 6. This table shows the mappings needed:

 "+HH",  -> "+HHmmss" or "+HH:mm:ss"
 "+HHmm",  -> "+HHmmss",
 "+HH:mm",  -> "+HH:mm:ss",
 "+HHMM",  -> "+HHmmss",
 "+HH:MM",  -> "+HH:mm:ss",
 "+HHMMss",  -> "+HHmmss",
 "+HH:MM:ss",  -> "+HH:mm:ss",
 "+HHMMSS",  -> "+HHmmss",
 "+HH:MM:SS",  -> "+HH:mm:ss",
 "+HHmmss",
 "+HH:mm:ss",

 Note that the "+HH" pattern is a special case, as we don't know
 whether to use the colon or non-colon pattern. Whether to require
 colon or not is based on whether the next character after the HH is a
 colon or not.

 Proposed appendOffsetId() Javadoc:

 * Appends the zone offset, such as '+01:00', to the formatter.
 * 
 * This appends an instruction to format/parse the offset ID to the
 builder.
 * This is equivalent to calling {@code appendOffset("+HH:MM:ss", "Z")}.
 * See {@link #appendOffset(String, String)} for details on formatting
 and parsing.

 Proposed appendOffset(String, String) Javadoc:

 * During parsing, the offset...

 changed to:

 * When parsing in strict mode, the input must contain the mandatory
 and optional elements are defined by the specified pattern.
 * If the offset cannot be parsed then an exception is thrown unless
 the section of the formatter is optional.
 * 
 * When parsing in lenient mode, only the hours are mandatory - minutes
 and seconds are optional.
 * The colons are required if the specified pattern contains a colon.
 * If the specified pattern is "+HH", the presence of colons is
 determined by whether the character after the hour digits is a colon
 or not.
 * If the offset cannot b

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

[Roger Riggs]

HI Stephen,

How lenient is lenient supposed to be? Looking at the offset test cases, 
it seems to allow  minutes

and seconds digits to be parsed even if the pattern did not include them.

+ @DataProvider(name="lenientOffsetParseData")
+ Object[][] data_lenient_offset_parse() {
+ return new Object[][] {
+ {"+HH", "+01", 3600},
+ {"+HH", "+0101", 3660},
+ {"+HH", "+010101", 3661},
+ {"+HH", "+01", 3600},
+ {"+HH", "+01:01", 3660},
+ {"+HH", "+01:01:01", 3661},

Thanks, Roger


On 2/26/2016 6:16 AM, Stephen Colebourne wrote:

I don't think this is quite right.

if ((length > position + 3) && (text.charAt(position + 3) == ':')) {
parseType = 10;
}

This code will *always* select type 10 (colons) if a colon is found at
position+3. Whereas the spec now says that it should only do this if
the pattern is "HH". For other patterns, the colon/no-colon choice is
defined to be based on the pattern.

That said, I'm thinking it is better to make the spec more lenient to
match the behaviour as implemented:


When parsing in lenient mode, only the hours are mandatory - minutes
and seconds are optional. If the character after the hour digits is a colon
then the parser will parse using the pattern "HH:mm:ss", otherwise the
parser will parse using the pattern "HHmmss".


Additional TCKDateTimeFormatterBuilder tests will be needed to
demonstrate the above. There should also be a test for data following
the lenient parse. The following should all succeed:

DateTimeFormatterBuilder().parseLenient().appendOffset("HH:MM").appendZoneId();
"+01:00Europe/London"
"+0100Europe/London"

DateTimeFormatterBuilder().parseLenient().appendOffset("HH:MM").appendLiteral(":").appendZoneId();
"+01:Europe/London"

Note this special case, where the colon affects the parse type, but is
not ultimately part of the offset, thus it is left to match the
appendLiteral(":")

You may want to think of some additional nasty edge cases!

Stephen

On 25 February 2016 at 15:44, nadeesh tv  wrote:

Hi all,
Please see the updated webrev
http://cr.openjdk.java.net/~ntv/8032051/webrev.02/

Thanks and Regards,
Nadeesh

On 2/23/2016 5:17 PM, Stephen Colebourne wrote:

Thanks for the changes.

In `DateTimeFormatter`, the code should be

.parseLenient()
.appendOffsetId()
.parseStrict()

and the same in the other case. This ensures that existing callers who
then embed the formatter in another formatter (like the
ZONED_DATE_TIME constant) are unaffected.


The logic for lenient parsing does not look right as it only handles
types 5 and 6. This table shows the mappings needed:

"+HH",  -> "+HHmmss" or "+HH:mm:ss"
"+HHmm",  -> "+HHmmss",
"+HH:mm",  -> "+HH:mm:ss",
"+HHMM",  -> "+HHmmss",
"+HH:MM",  -> "+HH:mm:ss",
"+HHMMss",  -> "+HHmmss",
"+HH:MM:ss",  -> "+HH:mm:ss",
"+HHMMSS",  -> "+HHmmss",
"+HH:MM:SS",  -> "+HH:mm:ss",
"+HHmmss",
"+HH:mm:ss",

Note that the "+HH" pattern is a special case, as we don't know
whether to use the colon or non-colon pattern. Whether to require
colon or not is based on whether the next character after the HH is a
colon or not.

Proposed appendOffsetId() Javadoc:

* Appends the zone offset, such as '+01:00', to the formatter.
* 
* This appends an instruction to format/parse the offset ID to the
builder.
* This is equivalent to calling {@code appendOffset("+HH:MM:ss", "Z")}.
* See {@link #appendOffset(String, String)} for details on formatting
and parsing.

Proposed appendOffset(String, String) Javadoc:

* During parsing, the offset...

changed to:

* When parsing in strict mode, the input must contain the mandatory
and optional elements are defined by the specified pattern.
* If the offset cannot be parsed then an exception is thrown unless
the section of the formatter is optional.
* 
* When parsing in lenient mode, only the hours are mandatory - minutes
and seconds are optional.
* The colons are required if the specified pattern contains a colon.
* If the specified pattern is "+HH", the presence of colons is
determined by whether the character after the hour digits is a colon
or not.
* If the offset cannot be parsed then an exception is thrown unless
the section of the formatter is optional.

thanks and sorry for delay
Stephen



On 11 February 2016 at 20:22, nadeesh tv  wrote:

Hi all,

Please review a fix for

Bug Id  https://bugs.openjdk.java.net/browse/JDK-8032051

webrev http://cr.openjdk.java.net/~ntv/8032051/webrev.01/

--
Thanks and Regards,
Nadeesh TV


--
Thanks and Regards,
Nadeesh TV

RFR(M): 8150635: j.l.i.MethodHandles.loop(...) throws IndexOutOfBoundsException

[Michael Haupt]

Dear all,

please review this fix.
Bug: https://bugs.openjdk.java.net/browse/JDK-8150635
Webrev: http://cr.openjdk.java.net/~mhaupt/8150635/webrev.00

There were two issues to address that led to IOOBE being thrown.

1. Lack of support for deriving the loop signature from given parameter type 
lists in case no init function is given at all.

2. The loop combinator reporting a wrong exception in case a parameter type 
list is longer than the common parameter sequence. In this case, an error about 
non-effectively identical parameter type lists should be signalled.

See the JIRA issue for details and examples.

Thanks,

Michael

-- 

 
Dr. Michael Haupt | Principal Member of Technical Staff
Phone: +49 331 200 7277 | Fax: +49 331 200 7561
Oracle Java Platform Group | LangTools Team | Nashorn
Oracle Deutschland B.V. & Co. KG | Schiffbauergasse 14 | 14467 Potsdam, Germany

ORACLE Deutschland B.V. & Co. KG | Hauptverwaltung: Riesstraße 25, D-80992 
München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V. | Hertogswetering 163/167, 
3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
  Oracle is committed to developing 
practices and products that help protect the environment

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

[Stephen Colebourne]

I don't think this is quite right.

if ((length > position + 3) && (text.charAt(position + 3) == ':')) {
   parseType = 10;
}

This code will *always* select type 10 (colons) if a colon is found at
position+3. Whereas the spec now says that it should only do this if
the pattern is "HH". For other patterns, the colon/no-colon choice is
defined to be based on the pattern.

That said, I'm thinking it is better to make the spec more lenient to
match the behaviour as implemented:


When parsing in lenient mode, only the hours are mandatory - minutes
and seconds are optional. If the character after the hour digits is a colon
then the parser will parse using the pattern "HH:mm:ss", otherwise the
parser will parse using the pattern "HHmmss".


Additional TCKDateTimeFormatterBuilder tests will be needed to
demonstrate the above. There should also be a test for data following
the lenient parse. The following should all succeed:

DateTimeFormatterBuilder().parseLenient().appendOffset("HH:MM").appendZoneId();
"+01:00Europe/London"
"+0100Europe/London"

DateTimeFormatterBuilder().parseLenient().appendOffset("HH:MM").appendLiteral(":").appendZoneId();
"+01:Europe/London"

Note this special case, where the colon affects the parse type, but is
not ultimately part of the offset, thus it is left to match the
appendLiteral(":")

You may want to think of some additional nasty edge cases!

Stephen

On 25 February 2016 at 15:44, nadeesh tv  wrote:
>
> Hi all,
> Please see the updated webrev
> http://cr.openjdk.java.net/~ntv/8032051/webrev.02/
>
> Thanks and Regards,
> Nadeesh
>
> On 2/23/2016 5:17 PM, Stephen Colebourne wrote:
>>
>> Thanks for the changes.
>>
>> In `DateTimeFormatter`, the code should be
>>
>> .parseLenient()
>> .appendOffsetId()
>> .parseStrict()
>>
>> and the same in the other case. This ensures that existing callers who
>> then embed the formatter in another formatter (like the
>> ZONED_DATE_TIME constant) are unaffected.
>>
>>
>> The logic for lenient parsing does not look right as it only handles
>> types 5 and 6. This table shows the mappings needed:
>>
>> "+HH",  -> "+HHmmss" or "+HH:mm:ss"
>> "+HHmm",  -> "+HHmmss",
>> "+HH:mm",  -> "+HH:mm:ss",
>> "+HHMM",  -> "+HHmmss",
>> "+HH:MM",  -> "+HH:mm:ss",
>> "+HHMMss",  -> "+HHmmss",
>> "+HH:MM:ss",  -> "+HH:mm:ss",
>> "+HHMMSS",  -> "+HHmmss",
>> "+HH:MM:SS",  -> "+HH:mm:ss",
>> "+HHmmss",
>> "+HH:mm:ss",
>>
>> Note that the "+HH" pattern is a special case, as we don't know
>> whether to use the colon or non-colon pattern. Whether to require
>> colon or not is based on whether the next character after the HH is a
>> colon or not.
>>
>> Proposed appendOffsetId() Javadoc:
>>
>> * Appends the zone offset, such as '+01:00', to the formatter.
>> * 
>> * This appends an instruction to format/parse the offset ID to the
>> builder.
>> * This is equivalent to calling {@code appendOffset("+HH:MM:ss", "Z")}.
>> * See {@link #appendOffset(String, String)} for details on formatting
>> and parsing.
>>
>> Proposed appendOffset(String, String) Javadoc:
>>
>> * During parsing, the offset...
>>
>> changed to:
>>
>> * When parsing in strict mode, the input must contain the mandatory
>> and optional elements are defined by the specified pattern.
>> * If the offset cannot be parsed then an exception is thrown unless
>> the section of the formatter is optional.
>> * 
>> * When parsing in lenient mode, only the hours are mandatory - minutes
>> and seconds are optional.
>> * The colons are required if the specified pattern contains a colon.
>> * If the specified pattern is "+HH", the presence of colons is
>> determined by whether the character after the hour digits is a colon
>> or not.
>> * If the offset cannot be parsed then an exception is thrown unless
>> the section of the formatter is optional.
>>
>> thanks and sorry for delay
>> Stephen
>>
>>
>>
>> On 11 February 2016 at 20:22, nadeesh tv  wrote:
>>>
>>> Hi all,
>>>
>>> Please review a fix for
>>>
>>> Bug Id  https://bugs.openjdk.java.net/browse/JDK-8032051
>>>
>>> webrev http://cr.openjdk.java.net/~ntv/8032051/webrev.01/
>>>
>>> --
>>> Thanks and Regards,
>>> Nadeesh TV
>>>
>
> --
> Thanks and Regards,
> Nadeesh TV
>

Re: RFR[9u-dev] 8130425: libjvm crash due to stack overflow in executables with 32k tbss/tdata

[Kevin Walls]

Thanks Martin - I think we're going to proceed with the arguably 
somewhat ugly change as it does solve a real problem, although somewhat 
unusual to have such large thread local sizes for it to matter.  And we 
may have to deal with a thread library that takes away stack space for 
thread local data for a while.


Thanks
Kevin


On 25/02/2016 16:30, Martin Buchholz wrote:

My hope is that we eventually eliminate StackOverflowError by
implementing dynamically growable thread stacks.

Until then, my hope is that hotspot give the thread the memory it asks
for, for actually storing stack frames, and thread locals should not
steal space from that.

The system property introduced in this change is an ugly workaround for that.

On Thu, Feb 18, 2016 at 2:24 AM, Kevin Walls  wrote:

Hi Cheleswer,

Looks good to me.

Thanks
Kevin
(Also, as one of the comments was that there may be no real cost to using
default stack sizes here (on most systems...?), having
jdk.lang.processReaperUseDefaultStackSize gives us a way to test that
widely, and one day the 32k may be able to disappear.)





On 17/02/2016 15:17, cheleswer sahu wrote:

Hi,
I have made changes in the property name
(jdk.lang.processReaperUseDefaultStackSize) and code as suggested in the
earlier reviews.

  --- old/src/java.base/share/classes/java/lang/ProcessHandleImpl.java
2016-02-17 18:48:10.54563 +0530
+++ new/src/java.base/share/classes/java/lang/ProcessHandleImpl.java
2016-02-17 18:48:10.08163 +0530
@@ -81,9 +81,8 @@
  ThreadGroup systemThreadGroup = tg;

  ThreadFactory threadFactory = grimReaper -> {
-// Our thread stack requirement is quite modest.
-Thread t = new Thread(systemThreadGroup, grimReaper,
-"process reaper", 32768);
+long stackSize =
Boolean.getBoolean("jdk.lang.processReaperUseDefaultStackSize") ? 0 : 32768;
+Thread t = new Thread(systemThreadGroup, grimReaper,
"process reaper", stackSize);
  t.setDaemon(true);
  // A small attempt (probably futile) to avoid priority
inversion
  t.setPriority(Thread.MAX_PRIORITY);

I would really like to thanks "Martin" for the patch
(http://cr.openjdk.java.net/~martin/webrevs/openjdk9/tls-size-guarantee/)
which he has provided. Since we support a wider range of glibc versions and
platform using patch will
require more testing and work. I think the use of system property for this
issue will be safer at this point of time.

Regards,
Cheleswer


On 1/19/2016 5:40 PM, David Holmes wrote:

On 19/01/2016 9:53 PM, Kevin Walls wrote:

|
Hi Cheleswer, I think Martin is suggesting something like:
|

// Use a modest stack size, unless requested otherwise:
long stackSize = Boolean.getBoolean("processReaperUseDefaultStackSize") ? 0
: 32768;


Someone from core-libs needs to chime on what the appropriate namespace for
such a property would be.

David
-

Thread t = new Thread(systemThreadGroup, grimReaper, "process reaper",
stackSize);

|||
If that tests OK for you, it may be the way we can go ahead with the
point fix for this.

Regards
Kevin
|
On 18/01/2016 16:52, Martin Buchholz wrote:

Historical note - I never liked having a reaper thread for each
subprocess, but no other reliable implementation is known.  Given the
potential for many reaper threads, I at least wanted to keep the
memory waste low.

On Wed, Jan 13, 2016 at 2:25 AM, cheleswer sahu
  wrote:

+   Thread t = null;
+   if
(Boolean.getBoolean("processReaperUseDefaultStackSize")) {
+   t = new Thread(systemThreadGroup, grimReaper,
"process reaper");
+} else {
+   // Our thread stack requirement is quite modest.
+   t = new Thread(systemThreadGroup, grimReaper,
"process reaper", 32768);
+}

If we do end up using this strategy, cleaner would be to use
https://docs.oracle.com/javase/8/docs/api/java/lang/Thread.html#Thread-java.lang.ThreadGroup-java.lang.Runnable-java.lang.String-long-
stackSize - the desired stack size for the new thread, or zero to
indicate that this parameter is to be ignored.