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

[Jaikiran Pai]

Consider the following code:

import java.util.Arrays;
import java.util.List;

public class ArraysAsListTest {

    public static void main(final String[] args) throws Exception {
        final List someList = Arrays.asList(new String[] {"a"});
        System.out.println("Removed? " + someList.remove("a"));
    }
}

It uses Arrays.asList to create a java.util.List and then calls a
list.remove(...) on it. This currently runs intothe following exception:

Exception in thread "main" java.lang.UnsupportedOperationException: remove
    at java.base/java.util.Iterator.remove(Iterator.java:102)
    at
java.base/java.util.AbstractCollection.remove(AbstractCollection.java:299)
    at ArraysAsListTest.main(ArraysAsListTest.java:8)

The javadoc of Arrays.asList[1] states the following:

"Returns a fixed-size list backed by the specified array. (Changes to
the returned list "write through" to the array.)..."

and has no other mention of how it's supposed to behave with "write"
operations. Given that the javadoc states that the returned list is
"write through", would that mean a "add/removal" is allowed? Probably
not, because it does say it's a fixed size list.So the size altering,
write operations (like add(), remove()) aren't allowed, but that isn't
clear from the javadoc. So should the javadoc be updated to clarify the
semantics of what changes are "write through" and what changes are not
supported?

Furthermore, the UnsupportedOperationException isn't always thrown from
such a returned list. Consider this minor modification to the above code:

        final List someList = Arrays.asList(new String[] {"a"});
        System.out.println("Removed? " + someList.remove("b"));

I add "a" and remove "b". This now runs fine without exceptions and
prints "Removed? false". Should the implementation just throw the
UnsupportedOperationException irrespective of whether or not the element
is contained insuch a returned list? If not, should that be made clear
in the javadoc (maybe same for addoperations)?

[1]
https://docs.oracle.com/javase/10/docs/api/java/util/Arrays.html#asList(T...)

-Jaikiran

JDK 12 RFR of 5075463 : (enum) Serialized Form javadoc for java.lang.Enum is misleading

[joe darcy]

Hello,

Cleaning out an old bug, please review the CSR and fix for:

    JDK-5075463 : (enum) Serialized Form javadoc for java.lang.Enum is 
misleading

    http://cr.openjdk.java.net/~darcy/5075463.0/
    CSR: https://bugs.openjdk.java.net/browse/JDK-8209524

The abstract class java.lang.Enum implements Serializable and is the 
superclass of all enum types. By default a serializable type is added to 
the serialized form page of the javadoc output. However, in the case of 
java.lang.Enum, including the type in the serialized form output is 
misleading since java.lang.Enum cannot be directly instantiated and enum 
types have special-cased handling in serialization. Adding a "@serial 
exclude" javadoc tag to the class-level javadoc of java.lang.Enum 
prevent an entry the the type in the serialized form page.


Patch below.

Thanks,

-Joe

--- old/src/java.base/share/classes/java/lang/Enum.java 2018-08-14 
20:48:04.20200 -0700
+++ new/src/java.base/share/classes/java/lang/Enum.java 2018-08-14 
20:48:04.05800 -0700

@@ -45,6 +45,7 @@
  * java.util.EnumMap map} implementations are available.
  *
  * @param  The enum type subclass
+ * @serial exclude
  * @author  Josh Bloch
  * @author  Neal Gafter
  * @see Class#getEnumConstants()

JDK 12 RFR of 8176425: Add radix indication in NumberFormatException message for Integer.decode

[joe darcy]

Hello,

Please review the changes to address:

    8176425: Add radix indication in NumberFormatException message for 
Integer.decode

    http://cr.openjdk.java.net/~darcy/8176425.0/

Basically the radix used for parsing an int or long is passed along to a 
factory method in the NumberFormatException class and the radix is 
included in the exception message for non-decimal radices.


Patch below.

Thanks,

-Joe


--- old/src/java.base/share/classes/java/lang/Integer.java 2018-08-14 
20:15:45.64200 -0700
+++ new/src/java.base/share/classes/java/lang/Integer.java 2018-08-14 
20:15:45.49800 -0700

@@ -635,11 +635,11 @@
 negative = true;
 limit = Integer.MIN_VALUE;
 } else if (firstChar != '+') {
-    throw NumberFormatException.forInputString(s);
+    throw NumberFormatException.forInputString(s, radix);
 }

 if (len == 1) { // Cannot have lone "+" or "-"
-    throw NumberFormatException.forInputString(s);
+    throw NumberFormatException.forInputString(s, radix);
 }
 i++;
 }
@@ -649,17 +649,17 @@
 // Accumulating negatively avoids surprises near MAX_VALUE
 int digit = Character.digit(s.charAt(i++), radix);
 if (digit < 0 || result < multmin) {
-    throw NumberFormatException.forInputString(s);
+    throw NumberFormatException.forInputString(s, radix);
 }
 result *= radix;
 if (result < limit + digit) {
-    throw NumberFormatException.forInputString(s);
+    throw NumberFormatException.forInputString(s, radix);
 }
 result -= digit;
 }
 return negative ? result : -result;
 } else {
-    throw NumberFormatException.forInputString(s);
+    throw NumberFormatException.forInputString(s, radix);
 }
 }

@@ -745,7 +745,7 @@
 }
 return negative ? result : -result;
 } else {
-    throw NumberFormatException.forInputString("");
+    throw NumberFormatException.forInputString("", radix);
 }
 }

@@ -842,7 +842,7 @@
 }
 }
 } else {
-    throw NumberFormatException.forInputString(s);
+    throw NumberFormatException.forInputString(s, radix);
 }
 }

--- old/src/java.base/share/classes/java/lang/Long.java 2018-08-14 
20:15:46.03000 -0700
+++ new/src/java.base/share/classes/java/lang/Long.java 2018-08-14 
20:15:45.88200 -0700

@@ -675,11 +675,11 @@
 negative = true;
 limit = Long.MIN_VALUE;
 } else if (firstChar != '+') {
-    throw NumberFormatException.forInputString(s);
+    throw NumberFormatException.forInputString(s, radix);
 }

 if (len == 1) { // Cannot have lone "+" or "-"
-    throw NumberFormatException.forInputString(s);
+    throw NumberFormatException.forInputString(s, radix);
 }
 i++;
 }
@@ -689,17 +689,17 @@
 // Accumulating negatively avoids surprises near MAX_VALUE
 int digit = Character.digit(s.charAt(i++),radix);
 if (digit < 0 || result < multmin) {
-    throw NumberFormatException.forInputString(s);
+    throw NumberFormatException.forInputString(s, radix);
 }
 result *= radix;
 if (result < limit + digit) {
-    throw NumberFormatException.forInputString(s);
+    throw NumberFormatException.forInputString(s, radix);
 }
 result -= digit;
 }
 return negative ? result : -result;
 } else {
-    throw NumberFormatException.forInputString(s);
+    throw NumberFormatException.forInputString(s, radix);
 }
 }

@@ -945,7 +945,7 @@
 return result;
 }
 } else {
-    throw NumberFormatException.forInputString(s);
+    throw NumberFormatException.forInputString(s, radix);
 }
 }

@@ -1063,7 +1063,7 @@
 return result;
 }
 } else {
-    throw NumberFormatException.forInputString("");
+    throw NumberFormatException.forInputString("", radix);
 }
 }

--- old/src/java.base/share/classes/java/lang/NumberFormatException.java 
2018-08-14 20:15:46.41800 -0700
+++ new/src/java.base/share/classes/java/lang/NumberFormatException.java 
2018-08-14 20:15:46.27400 -0700

@@ -61,8 +61,11 @@
  *
  * @param   s   the input causing the error
  */
-    static NumberFormatEx

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

[mandy chung]

Thanks for the information.  Not sure what's the best option we can do
in 8u.  I think it's acceptable to have a fix that works in the
current context (like empty static method).

Thoughts?

Mandy

On 8/14/18 4:22 PM, Hans Boehm wrote:
I haven't looked at the details here, but comparing against a volatile 
is not in general a portable way to keep an object live. Like empty 
static methods, it may be fine in the current (OpenJDK) context.


Volatile loads have roach motel semantics and can be advanced past other 
operations. The comparison can be done early, too, keeping only the 
condition code. There is no guarantee the reference will still be around 
when you expect it.


I haven't come up with a great compiler-independent replacement for 
reachabilityFence.


On Tue, Aug 14, 2018 at 8:25 AM mandy chung > wrote:


Hi Adam,

Have you tried Peter's suggestion if an empty static method taking an
Object parameter?  Does it work for you?

Your proposed approach seems fine and I would suggest to put the
check in a static keepAlive method that will make it explicitly.

Mandy

On 8/10/18 8:42 AM, Adam Farley8 wrote:
 > Hi All,
 >
 > This bug could be fixed by comparing the Class Loader with a blank
 > static volatile Object (defined outside the method scope) at the
 > end of the getBundleImpl class.
 >
 > E.g.
 >
 > -
 > +1322
 >      /**
 >       * volatile reference object to guard the ClassLoader object
 >       * being garbage collected before getBundleImpl() method
completes
 >       * the caching and retrieving of requested Resourcebundle object
 >       */
 >      private static volatile Object vo = new Object();
 >
 >
 > +1400
 > //Should never be true. Using the loader here prevents it being GC'd.
 >              if (loader == vo) throw new Error("Unexpected error.");
 > -
 >
 > Will upload a webrev after debate.
 >
 > - Adam
 > Unless stated otherwise above:
 > IBM United Kingdom Limited - Registered in England and Wales with
number
 > 741598.
 > Registered office: PO Box 41, North Harbour, Portsmouth,
Hampshire PO6 3AU
 >

Re: RFR 8208715: Conversion of milliseconds to nanoseconds in UNIXProcess contains bug

[Martin Buchholz]

Looks good to me ... but ...

my intent was that the self-interrupt was added in __addition__ to
interrupt by other thread, because there is often different code to handle
pre-existing interrupt.  As with BlockingQueueTest.testTimedPollWithOffer.

public void run() {
Thread.currentThread().interrupt();
try {
boolean result = p.waitFor(Long.MAX_VALUE,
TimeUnit.MILLISECONDS);
fail("Should have thrown InterruptedException");
} catch (InterruptedException success) {
} catch (Throwable t) { unexpected(t); }

try {
aboutToWaitFor.countDown();
boolean result = p.waitFor(Long.MAX_VALUE,
TimeUnit.MILLISECONDS);
fail("Should have thrown InterruptedException");
} catch (InterruptedException success) {
} catch (Throwable t) { unexpected(t); }




On Tue, Aug 14, 2018 at 12:59 PM, Roger Riggs 
wrote:

> Hi Martin,
>
> Updated with suggestions:
>  http://cr.openjdk.java.net/~rriggs/webrev-timeout-8208715/index.html
>
> On 8/14/2018 1:22 PM, Martin Buchholz wrote:
>
> Thanks, Roger.  I approve this version, although as always I have
> comments.
>
>  520 private static native void waitForTimeoutInterruptibly(
>  521 long handle, long timeout);
>
> The units being used should be obvious from the signature - rename timeout
> to timeoutMillis.
>
> Done
>
> But even better is to push the support for nanos into the utility method,
> so change this native method to accept a timeoutNanos.
>
> A bit far from the original issue and it might take a bit more time.
>
>
> 2465 Thread.sleep(1000);
>
> I hate sleeps, and I would just delete this one - I don't think the test
> relies on it (and if it did, one second is not long enough!).
>
> Done.  (And in the test case used for the copy/paste of the new test).
>
>
> 2454 try {
> 2455 aboutToWaitFor.countDown();
> 2456 boolean result = p.waitFor(Long.MAX_VALUE,
> TimeUnit.MILLISECONDS);
> 2457 fail("waitFor() wasn't interrupted, its
> return value was: " + result);
> 2458 } catch (InterruptedException success) {
> 2459 } catch (Throwable t) { unexpected(t); }
>
> It's easy to add a self-interrupt variant inside the run method
>
> Thread.currentThread().interrupt(); ...
>
> ok, and will run all the tests again.
>
> Thanks, Roger
>
>
>
> (TODO: Basic.java is in need of a re-write - mea culpa ...)
>
>
>
>
>
> On Tue, Aug 14, 2018 at 9:48 AM, Roger Riggs 
> wrote:
>
>> Hi Martin,
>>
>> I updated the webrev with the suggestions.
>>
>> On 8/14/2018 10:47 AM, Martin Buchholz wrote:
>>
>> hi Roger,
>>
>>  509 if (deadline <= 0) { 510 deadline = Long.MAX_VALUE; 
>> 511 }
>>
>>
>> This must be wrong.  Nanotime wraparound is normal in this sort of code.
>>
>> ok, this reader didn't make that assumption
>>
>>
>> ---
>>
>> We ought to be able to delegate the fiddling with nanos to
>> TimeUnit.timedWait.
>>
>> Does it work to simply call NANOSECONDS.timedWait(remainingNanos) ?
>> If not, is there a bug in TimeUnit.timedWait?
>>
>> That works except on Windows, that does not use wait().
>>
>>
>> It's good to add a test for this.  I've tried hard in similar tests to
>> avoid sleep and to add variants where the target thread is interrupted
>> before and after starting to wait.  Testing pre-interrupt is easy - the
>> thread can interrupt itself.  BlockingQueueTest.testTimedPollWithOffer
>> is an example.
>>
>> I added a test, using the same logic as the existing tests for the
>> Long.MAX_VALUE case
>>
>> Thanks, Roger
>>
>>
>>
>>
>>
>> On Tue, Aug 14, 2018 at 7:00 AM, Roger Riggs 
>> wrote:
>>
>>> Please review a fix for Process.waitFor(Long.MAX_VALUE,MILLIS).
>>> Catch wrap around in very large wait times and saturate at
>>> Long.MAX_VALUE.
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~rriggs/webrev-timeout-8208715/
>>>
>>> Issue:
>>> https://bugs.openjdk.java.net/browse/JDK-8208715
>>>
>>> Thanks, Roger
>>>
>>>
>>
>>
>
>

Re: [12] (AIX) 8207744: Clean up inconsistent use of opendir/closedir versus opendir64/closedir64

[Brian Burkhalter]

Hi Bernard,

On Aug 14, 2018, at 4:13 AM, B. Blaser  wrote:

> Seems quite good to me, last notes:
> 
> 1) dealing with 'stat/stat64' in 'UnixFileSystem_md.c' might be
> outside the scope of this fix (?) even if fully pertinent per [1].

It might be slightly out of scope but I think it’s OK as stat64 was defined 
inside an
#if defined(_ALLBSD_SOURCE) conditional compilation block.

> In the same file, I think '#define dirent dirent64' is probably missing
> for AIX.

Fixed.

> 2) I guess '#if defined(_AIX) ...' is now missing in 'OperatingSystemImpl.c':
> #if defined(_AIX)
>  #define DIR DIR64
>  #define dirent dirent64
>  #define opendir opendir64
>  #define readdir readdir64
>  #define closedir closedir64
> #endif

Fixed.

Webrev updated in place: http://cr.openjdk.java.net/~bpb/8207744/webrev.04/.

Checks out on Linux-x64, macOS, Solaris-sparcv9, Windows-x64.

> You'll probably need some more reviews especially for other systems
> than Linux 64-bit.

It would not hurt. In any case I do not yet have a Reviewer approval.

Thanks,

Brian

Re: RFR 8208715: Conversion of milliseconds to nanoseconds in UNIXProcess contains bug

[Roger Riggs]

Hi Martin,

Updated with suggestions:
 http://cr.openjdk.java.net/~rriggs/webrev-timeout-8208715/index.html

On 8/14/2018 1:22 PM, Martin Buchholz wrote:
Thanks, Roger.  I approve this version, although as always I have 
comments.


 520     private static native void waitForTimeoutInterruptibly(
 521         long handle, long timeout);

The units being used should be obvious from the signature - rename 
timeout to timeoutMillis.

Done
But even better is to push the support for nanos into the utility 
method, so change this native method to accept a timeoutNanos.

A bit far from the original issue and it might take a bit more time.


2465             Thread.sleep(1000);

I hate sleeps, and I would just delete this one - I don't think the 
test relies on it (and if it did, one second is not long enough!).

Done.  (And in the test case used for the copy/paste of the new test).


2454                     try {
2455                         aboutToWaitFor.countDown();
2456                         boolean result = 
p.waitFor(Long.MAX_VALUE, TimeUnit.MILLISECONDS);
2457                         fail("waitFor() wasn't interrupted, its 
return value was: " + result);

2458                     } catch (InterruptedException success) {
2459                     } catch (Throwable t) { unexpected(t); }

It's easy to add a self-interrupt variant inside the run method

Thread.currentThread().interrupt(); ...

ok, and will run all the tests again.

Thanks, Roger



(TODO: Basic.java is in need of a re-write - mea culpa ...)





On Tue, Aug 14, 2018 at 9:48 AM, Roger Riggs > wrote:


Hi Martin,

I updated the webrev with the suggestions.

On 8/14/2018 10:47 AM, Martin Buchholz wrote:

hi Roger,

509 if (deadline <= 0) {
510 deadline = Long.MAX_VALUE;
511 }

This must be wrong.  Nanotime wraparound is normal in this sort
of code.

ok, this reader didn't make that assumption


---

We ought to be able to delegate the fiddling with nanos to
TimeUnit.timedWait.

Does it work to simply call NANOSECONDS.timedWait(remainingNanos) ?
If not, is there a bug in TimeUnit.timedWait?

That works except on Windows, that does not use wait().



It's good to add a test for this.  I've tried hard in similar
tests to avoid sleep and to add variants where the target thread
is interrupted before and after starting to wait.  Testing
pre-interrupt is easy - the thread can interrupt itself. 
BlockingQueueTest.testTimedPollWithOffer is an example.

I added a test, using the same logic as the existing tests for the
Long.MAX_VALUE case

Thanks, Roger






On Tue, Aug 14, 2018 at 7:00 AM, Roger Riggs
mailto:roger.ri...@oracle.com>> wrote:

Please review a fix for Process.waitFor(Long.MAX_VALUE,MILLIS).
Catch wrap around in very large wait times and saturate at
Long.MAX_VALUE.

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-timeout-8208715/


Issue:
https://bugs.openjdk.java.net/browse/JDK-8208715


Thanks, Roger

OpenJDK 9 - Make (no deprecations flag setting)

[mr rupplin]

Where in Makefile do we add this - to suppress deprecation warnings - to our 
personal builds.

Thanks.

MR

Re: RFR 8208715: Conversion of milliseconds to nanoseconds in UNIXProcess contains bug

[Martin Buchholz]

Thanks, Roger.  I approve this version, although as always I have comments.

 520 private static native void waitForTimeoutInterruptibly(
 521 long handle, long timeout);

The units being used should be obvious from the signature - rename timeout
to timeoutMillis.  But even better is to push the support for nanos into
the utility method, so change this native method to accept a timeoutNanos.

2465 Thread.sleep(1000);

I hate sleeps, and I would just delete this one - I don't think the test
relies on it (and if it did, one second is not long enough!).

2454 try {
2455 aboutToWaitFor.countDown();
2456 boolean result = p.waitFor(Long.MAX_VALUE,
TimeUnit.MILLISECONDS);
2457 fail("waitFor() wasn't interrupted, its return
value was: " + result);
2458 } catch (InterruptedException success) {
2459 } catch (Throwable t) { unexpected(t); }

It's easy to add a self-interrupt variant inside the run method

Thread.currentThread().interrupt(); ...

(TODO: Basic.java is in need of a re-write - mea culpa ...)





On Tue, Aug 14, 2018 at 9:48 AM, Roger Riggs  wrote:

> Hi Martin,
>
> I updated the webrev with the suggestions.
>
> On 8/14/2018 10:47 AM, Martin Buchholz wrote:
>
> hi Roger,
>
>  509 if (deadline <= 0) { 510 deadline = Long.MAX_VALUE; 
> 511 }
>
>
> This must be wrong.  Nanotime wraparound is normal in this sort of code.
>
> ok, this reader didn't make that assumption
>
>
> ---
>
> We ought to be able to delegate the fiddling with nanos to
> TimeUnit.timedWait.
>
> Does it work to simply call NANOSECONDS.timedWait(remainingNanos) ?
> If not, is there a bug in TimeUnit.timedWait?
>
> That works except on Windows, that does not use wait().
>
>
> It's good to add a test for this.  I've tried hard in similar tests to
> avoid sleep and to add variants where the target thread is interrupted
> before and after starting to wait.  Testing pre-interrupt is easy - the
> thread can interrupt itself.  BlockingQueueTest.testTimedPollWithOffer is
> an example.
>
> I added a test, using the same logic as the existing tests for the
> Long.MAX_VALUE case
>
> Thanks, Roger
>
>
>
>
>
> On Tue, Aug 14, 2018 at 7:00 AM, Roger Riggs 
> wrote:
>
>> Please review a fix for Process.waitFor(Long.MAX_VALUE,MILLIS).
>> Catch wrap around in very large wait times and saturate at Long.MAX_VALUE.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~rriggs/webrev-timeout-8208715/
>>
>> Issue:
>> https://bugs.openjdk.java.net/browse/JDK-8208715
>>
>> Thanks, Roger
>>
>>
>
>

Re: RFR 8208715: Conversion of milliseconds to nanoseconds in UNIXProcess contains bug

[Roger Riggs]

Hi Martin,

I updated the webrev with the suggestions.

On 8/14/2018 10:47 AM, Martin Buchholz wrote:

hi Roger,

509 if (deadline <= 0) {
510 deadline = Long.MAX_VALUE;
511 }

This must be wrong.  Nanotime wraparound is normal in this sort of code.

ok, this reader didn't make that assumption


---

We ought to be able to delegate the fiddling with nanos to 
TimeUnit.timedWait.


Does it work to simply call NANOSECONDS.timedWait(remainingNanos) ?
If not, is there a bug in TimeUnit.timedWait?

That works except on Windows, that does not use wait().



It's good to add a test for this.  I've tried hard in similar tests to 
avoid sleep and to add variants where the target thread is interrupted 
before and after starting to wait.  Testing pre-interrupt is easy - 
the thread can interrupt itself.  
BlockingQueueTest.testTimedPollWithOffer is an example.
I added a test, using the same logic as the existing tests for the 
Long.MAX_VALUE case


Thanks, Roger






On Tue, Aug 14, 2018 at 7:00 AM, Roger Riggs > wrote:


Please review a fix for Process.waitFor(Long.MAX_VALUE,MILLIS).
Catch wrap around in very large wait times and saturate at
Long.MAX_VALUE.

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-timeout-8208715/


Issue:
https://bugs.openjdk.java.net/browse/JDK-8208715


Thanks, Roger

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

[mandy chung]

Hi Adam,

Have you tried Peter's suggestion if an empty static method taking an
Object parameter?  Does it work for you?

Your proposed approach seems fine and I would suggest to put the
check in a static keepAlive method that will make it explicitly.

Mandy

On 8/10/18 8:42 AM, Adam Farley8 wrote:

Hi All,

This bug could be fixed by comparing the Class Loader with a blank
static volatile Object (defined outside the method scope) at the
end of the getBundleImpl class.

E.g.

-
+1322
 /**
  * volatile reference object to guard the ClassLoader object
  * being garbage collected before getBundleImpl() method completes
  * the caching and retrieving of requested Resourcebundle object
  */
 private static volatile Object vo = new Object();


+1400
//Should never be true. Using the loader here prevents it being GC'd.
 if (loader == vo) throw new Error("Unexpected error.");
-

Will upload a webrev after debate.

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

Re: RFR 8208715: Conversion of milliseconds to nanoseconds in UNIXProcess contains bug

[Martin Buchholz]

hi Roger,

 509 if (deadline <= 0) { 510 deadline =
Long.MAX_VALUE; 511 }


This must be wrong.  Nanotime wraparound is normal in this sort of code.

---

We ought to be able to delegate the fiddling with nanos to
TimeUnit.timedWait.

Does it work to simply call NANOSECONDS.timedWait(remainingNanos) ?
If not, is there a bug in TimeUnit.timedWait?

It's good to add a test for this.  I've tried hard in similar tests to
avoid sleep and to add variants where the target thread is interrupted
before and after starting to wait.  Testing pre-interrupt is easy - the
thread can interrupt itself.  BlockingQueueTest.testTimedPollWithOffer is
an example.




On Tue, Aug 14, 2018 at 7:00 AM, Roger Riggs  wrote:

> Please review a fix for Process.waitFor(Long.MAX_VALUE,MILLIS).
> Catch wrap around in very large wait times and saturate at Long.MAX_VALUE.
>
> Webrev:
> http://cr.openjdk.java.net/~rriggs/webrev-timeout-8208715/
>
> Issue:
> https://bugs.openjdk.java.net/browse/JDK-8208715
>
> Thanks, Roger
>
>

MethodHandles.Lookup.defineResource?

[Stephen Colebourne]

A new method MethodHandles.Lookup defineClass() was added recently.
But what about a defineResource? For adding a new resource to the
classpath (such as a .txt file). I just needed such a thing, and
though undoubtedly rare, all the recommended solutions use reflection
and setAccessible().

thanks
Stephen

RFR 8208715: Conversion of milliseconds to nanoseconds in UNIXProcess contains bug

[Roger Riggs]

Please review a fix for Process.waitFor(Long.MAX_VALUE,MILLIS).
Catch wrap around in very large wait times and saturate at Long.MAX_VALUE.

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-timeout-8208715/

Issue:
https://bugs.openjdk.java.net/browse/JDK-8208715

Thanks, Roger

Re: RFR: 8207851 JEP Draft: Support ByteBuffer mapped over non-volatile memory

[Andrew Dinn]

Hi Alan,

Thanks for looking at this and apologies for the delayed response (c/o
our annual Red Hat OpenJDK team meeting taking place all last week).

On 06/08/18 10:29, Alan Bateman wrote:

> The current iteration, to introduce new MapMode values, is not too bad
> but it makes me wonder if we could avoid new modes altogether by
> detecting that the file is backed by NVM. Is there a fcntl cmd or some
> other syscall that can be used to detect this? I'm asking because it
> would open the potential to discuss limiting the API changes to just
> MappedByteBuffer.

That might well be a cleaner, simpler model. However, there are two
issues with it:

Firstly, there does not appear to be a supported way to identify whether
or not a device -- or a file associated with the device -- is mappable
using MAP_SYNC. A hack would be to attempt a map call with MAP_SYNC and
see if it failed with a suitably revealing errno code but that seems
rather gross and, perhaps, unreliable (the same errno code might occur
for other reasons).

Secondly, it may not always be the case that users will want to map an
NVM device with MAP_SYNC and rely on explicit line-by-line writeback. If
a user only needs block-level syncing then she might prefer to map the
file as a normal device and rely on force to perform block level
writeback via the driver.

> In the draft JEP then the Summary, Goals, Non-Goals, Success Metrics,
> and Motivation sections read well. The Description section is very wordy
> and includes a lot of implementation detail that I assume will be
> removed before it is submitted (my guess is that it can be distilled
> down to a couple of paragraphs). As a comparison, the API surface in JEP
> 337 is much larger but we were able to reduce the text down to a few
> paragraphs. The Testing sectioning highlights the challenges and reminds
> me that we have several features that will need maintainers to
> continuously test to ensure that a feature doesn't bit rot (SCTP and the
> proposed APIs for RDMA sockets are in the same boat).

I will be happy to perform this trimming down. The detail was merely to
ensure the draft implementation was understandable.

> Are you familiar with BufferPoolMXBean which can be used by management
> tools to monitor pool of buffers? I'm wondering if we should introduce a
> new pool for NVM so that it doesn't show up in the "mapped" pool.

I am not familiar with BufferPoolMXBean but I will investigate.

I'm agnostic as to whether to track NVM buffers separate from other
buffers but I am happy to consider adding this as a requirement.
However, before doing so, can you explain further why there is a need to
keep this case separate? In particular, I am puzzled by your use of
"mapped". The NVM memory is definitely mapped although obviously it is
of a different type to say private mapped volatile memory or a mapped
disk file. Also, are these latter cases distinguished as far as
BufferPoolMXBean bean counting is concerned? NVM seems to me to fall
somewhere between the two.

> I can't tell from this thread if you are looking for detailed feedback
> on the current patch. A couple of random things:

I guess its not the primary concern (which is to get the JEP submitted)
but comments are welcome now or later.

> - there are residual references to map_persistent in several places
> - MappedByteBuffer.force will need to specify IAE
> - The new methods are missing @since
> - I think it would be clearer if MappedByteBuffer.force use "region"
> rather than "sub-region" as it is used in the context of the buffer, not
> the original file.
> - There will be round of clean up needed to the changes to java.base to
> get the javadoc and code consistent with the javadoc/code in this area.
> I assume we'll get back to that when the patch is closer to review.
Ok, I'll post an update when I have trimmed the implementation details
the JEP and tweaked the code according to the above comments. Thank you
for your feedback.

regards,


Andrew Dinn
---
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander

Re: [12] (AIX) 8207744: Clean up inconsistent use of opendir/closedir versus opendir64/closedir64

[B. Blaser]

Seems quite good to me, last notes:

1) dealing with 'stat/stat64' in 'UnixFileSystem_md.c' might be
outside the scope of this fix (?) even if fully pertinent per [1]. In
the same file, I think '#define dirent dirent64' is probably missing
for AIX.

2) I guess '#if defined(_AIX) ...' is now missing in 'OperatingSystemImpl.c':
#if defined(_AIX)
  #define DIR DIR64
  #define dirent dirent64
  #define opendir opendir64
  #define readdir readdir64
  #define closedir closedir64
#endif

You'll probably need some more reviews especially for other systems
than Linux 64-bit.

Thanks,
Bernard

[1] https://www.gnu.org/software/libc/manual/html_node/Reading-Attributes.html


On 13 August 2018 at 23:26, Brian Burkhalter
 wrote:
> Hi Bernard,
>
> I updated the patch per your suggestions and it checks out on our systems.
>
> http://cr.openjdk.java.net/~bpb/8207744/webrev.04/
>
> Thanks,
>
> Brian
>
> On Aug 10, 2018, at 6:20 AM, B. Blaser  wrote:
>
> Among the files you suggest to fix, only the following ones are still
> using 'readdir64' for other systems than AIX:
>
>