Re: RFR: 8282081: java.time.DateTimeFormatter: wrong definition of symbol F

[Stephen Colebourne]

On Mon, 28 Feb 2022 23:17:57 GMT, Naoto Sato  wrote:

> Fixing the definition and implementation of the pattern symbol `F`. Although 
> it is an incompatible change, I believe it is worth the fix. For that, a CSR 
> has been drafted.

Although there is incompatibility, I believe a fix is the correct choice. The 
issue arose because of the poor description of the field in LDML.

-

Marked as reviewed by scolebourne (Author).

PR: https://git.openjdk.java.net/jdk/pull/7640

RE: Future.isCancelled and thread interruption

[Pushkar N Kulkarni]

Hi David,

Thank you. So,  I had the wrong notion of Thread.interrupt() also causing the 
signalling of the underlying OS thread. With that corrected, things have 
started making sense. Appreciate you taking the time to respond!

On 01/03/22, 7:59 AM, "David Holmes"  wrote:

Hi,

On 1/03/2022 4:17 am, Pushkar N Kulkarni wrote:
> "Future.cancel(true)" cancels the receiver Future and also attempts to 
interrupt the executor thread that is running this task. However, not all 
threads may be interrupted. An exception are threads that executing one of the 
"restart-able blocking system calls" from libnet.
> Such threads will ignore the thread interrupt(EINTR) and restart the 
blocking system call that was interrupted. See the system calls wrapped in the 
BLOCKING_IO_RETURN_INT macro: 
https://github.com/openjdk/jdk/blob/master/src/java.base/linux/native/libnet/linux_close.c#L352
 

You are mixing up two completely different notions of "interrupt". 
Thread.interrupt does not "interrupt" system calls - that only happens 
with signals. Thread.interrupt will not unblock low-level blocking I/O 
calls - like a socket connect. Only InterruptibleChannels provide some 
support for Thread.interrupt to break out of the I/O operation.

David
-

> 
> The javadoc for "java.util.concurrent.Future.cancel(boolean 
mayInterruptIfRunning)" DOES clarify that this method is an "attempt" to cancel 
the Future, and that it has no effect if the "task is already completed or 
cancelled, or could not be cancelled for *some other reason*". It is also made 
clear that "the return value from this method does not necessarily indicate 
whether the task is now cancelled". This is good, in the context of this note.
> 
> The javadoc also presents "Future.isCancelled()" as a definitive way to 
test if a Future was cancelled. However, it does not comment on the thread 
interruption attempted by Future.cancel(true). This might lead users to assume 
that if "Future.isCancelled()" returns true, the related executor thread was 
also successfully interrupted. This assumption would be invalid if the related 
executor thread was blocked in one of libnet's restart-able system calls 
(connect() could block for a couple of minutes).
> 
> I am attaching a test program that reproduces the mentioned behaviour. 
The executor thread held a lock and it was assumed that when 
"Future.isCancelled()" returned true, the executor had been interrupted and the 
lock released. In reality, the lock was held for a longer time and it blocked 
the main thread where the invalid assumption was made.
> 
> I am curious to know what others think of this matter! Any 
help/corrections/opinions will be appreciated. Thank you!
> 
> --
> import java.net.*;
> import java.util.concurrent.*;
> 
> public class ConnectionTest {
>  private  synchronized Socket connect(String host, int port) throws 
Exception {
>  InetSocketAddress address = new InetSocketAddress(host, port);
>  Socket s = new Socket();
>  s.connect(address); // HERE: s.connect(address, T), with any 
T>0, would resolve the hang!
>  return s;
>  }
> 
>  private Socket connectToMain() throws Exception {
>  System.out.println("Connecting to main...");
>  return connect("www.google.com", 81);
>  }
> 
>  private Socket connectToAlternate() throws Exception {
>  System.out.println("Connecting to alternate...");
>  return connect("www.example.com", 80);
>  }
> 
>  public void test() throws Exception {
>  ExecutorService es = Executors.newFixedThreadPool(1);
>  Future f = es.submit(new Callable() {
>  public Socket call() throws Exception {
>  return connectToMain();
>  }
>  });
>  try {
>  f.get(2000, TimeUnit.MILLISECONDS);
>  System.out.println("Connected to main!");
>  return;
>  } catch (TimeoutException e) {
>  System.out.println("Connection to main timed out, cancelling 
the Future with mayInterruptIfRunning = true");
>  boolean ret = f.cancel(true);
>  System.out.println("Future.cancel(true) returned " + ret);
>  }
>  System.out.println("Is Future canceled? ..." + f.isCancelled());
>  if (f.isCancelled()) {
>  connectToAlternate();
>  System.out.println("Connected to alternate!");
>  }
>  }
> 
>  public static void main(String [] args) throws Exception {
>  new ConnectionTest().test();
>  }
> }
> --
> 
> -Pushkar
> 
> 
> 

Re: RFR: 8279508: Auto-vectorize Math.round API [v9]

[Joe Darcy]

On Fri, 25 Feb 2022 06:22:42 GMT, Jatin Bhateja  wrote:

>> Summary of changes:
>> - Intrinsify Math.round(float) and Math.round(double) APIs.
>> - Extend auto-vectorizer to infer vector operations on encountering scalar 
>> IR nodes for above intrinsics.
>> - Test creation using new IR testing framework.
>> 
>> Following are the performance number of a JMH micro included with the patch 
>> 
>> Test System: Intel(R) Xeon(R) Platinum 8380 CPU @ 2.30GHz (Icelake Server)
>> 
>> 
>> Benchmark | TESTSIZE | Baseline AVX3 (ops/ms) | Withopt AVX3 (ops/ms) | Gain 
>> ratio | Baseline AVX2 (ops/ms) | Withopt AVX2 (ops/ms) | Gain ratio
>> -- | -- | -- | -- | -- | -- | -- | --
>> FpRoundingBenchmark.test_round_double | 1024.00 | 504.15 | 2209.54 | 4.38 | 
>> 510.36 | 548.39 | 1.07
>> FpRoundingBenchmark.test_round_double | 2048.00 | 293.64 | 1271.98 | 4.33 | 
>> 293.48 | 274.01 | 0.93
>> FpRoundingBenchmark.test_round_float | 1024.00 | 825.99 | 4754.66 | 5.76 | 
>> 751.83 | 2274.13 | 3.02
>> FpRoundingBenchmark.test_round_float | 2048.00 | 412.22 | 2490.09 | 6.04 | 
>> 388.52 | 1334.18 | 3.43
>> 
>> 
>> Kindly review and share your feedback.
>> 
>> Best Regards,
>> Jatin
>
> Jatin Bhateja has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8279508: Adding descriptive comments.

test/jdk/java/lang/Math/RoundTests.java line 32:

> 30: public static void main(String... args) {
> 31: int failures = 0;
> 32: for (int i = 0; i < 10; i++) {

Is there an idiom to trigger the auto-vectorization, perhaps using command line 
arguments, that doesn't bloat the running time of this test?

-

PR: https://git.openjdk.java.net/jdk/pull/7094

Re: RFR: 8282143: Objects.requireNonNull should be ForceInline [v2]

[ExE Boss]

On Tue, 1 Mar 2022 02:22:49 GMT, Quan Anh Mai  wrote:

>> Hi,
>> 
>> `Objects.requireNonNull` may fail to be inlined. The call is expensive and 
>> may lead to objects escaping to the heap while the null check is cheap and 
>> is often elided. I have observed this when using the vector API when a call 
>> to `Objects.requireNonNull` leads to vectors being materialised in a hot 
>> loop.
>> 
>> Should the other `requireNonNull` be `ForceInline` as well?
>> 
>> Thank you very much.
>
> Quan Anh Mai has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   the other

I think `@ForceInline` should also be applied to the overload that takes the 
message supplier, and probably also `requireNonNullElse` and 
`requireNonNullElseGet`.

-

PR: https://git.openjdk.java.net/jdk/pull/7543

Integrated: 8282023: PropertiesStoreTest and StoreReproducibilityTest jtreg failures due to en_CA locale

[Jaikiran Pai]

On Mon, 21 Feb 2022 14:09:50 GMT, Jaikiran Pai  wrote:

> Can I please get a review of this test only change which fixes the issue 
> noted in https://bugs.openjdk.java.net/browse/JDK-8282023?
> 
> As noted in that JBS issue these tests fail when the default locale under 
> which those tests are run isn't `en_US`. Both these tests were introduced as 
> part of https://bugs.openjdk.java.net/browse/JDK-8231640. At a high level, 
> these test do the following:
> - Use Properties.store(...) APIs to write out a properties file. This 
> internally ends up writing a date comment via a call to 
> `java.util.Date.toString()` method.
> - The test then runs a few checks to make sure that the written out `Date` is 
> an expected one. To run these checks it uses the 
> `java.time.format.DateTimeFormatter` to parse the date comment out of the 
> properties file.
> 
> All this works fine when the default locale is (for example) `en_US`. 
> However, when the default locale is (for example `en_CA` or even `hi_IN`) the 
> tests fail with an exception in the latter step above while parsing the date 
> comment using the `DateTimeFormatter` instance. The exception looks like:
> 
> 
> Using locale: he for Properties#store(OutputStream) test
> test PropertiesStoreTest.testStoreOutputStreamDateComment(he): failure
> java.lang.AssertionError: Unexpected date comment: Mon Feb 21 19:10:31 IST 
> 2022
>   at org.testng.Assert.fail(Assert.java:87)
>   at PropertiesStoreTest.testDateComment(PropertiesStoreTest.java:255)
>   at 
> PropertiesStoreTest.testStoreOutputStreamDateComment(PropertiesStoreTest.java:223)
>   at 
> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
>   at java.base/java.lang.reflect.Method.invoke(Method.java:577)
>   at 
> org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
>   at org.testng.internal.TestInvoker.invokeMethod(TestInvoker.java:599)
>   at 
> org.testng.internal.TestInvoker.invokeTestMethod(TestInvoker.java:174)
>   at org.testng.internal.MethodRunner.runInSequence(MethodRunner.java:46)
>   at 
> org.testng.internal.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:822)
>   at 
> org.testng.internal.TestInvoker.invokeTestMethods(TestInvoker.java:147)
>   at 
> org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
>   at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:128)
>   at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
>   at org.testng.TestRunner.privateRun(TestRunner.java:764)
>   at org.testng.TestRunner.run(TestRunner.java:585)
>   at org.testng.SuiteRunner.runTest(SuiteRunner.java:384)
>   at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:378)
>   at org.testng.SuiteRunner.privateRun(SuiteRunner.java:337)
>   at org.testng.SuiteRunner.run(SuiteRunner.java:286)
>   at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:53)
>   at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:96)
>   at org.testng.TestNG.runSuitesSequentially(TestNG.java:1218)
>   at org.testng.TestNG.runSuitesLocally(TestNG.java:1140)
>   at org.testng.TestNG.runSuites(TestNG.java:1069)
>   at org.testng.TestNG.run(TestNG.java:1037)
>   at 
> com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:94)
>   at 
> com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:54)
>   at 
> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
>   at java.base/java.lang.reflect.Method.invoke(Method.java:577)
>   at 
> com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
>   at java.base/java.lang.Thread.run(Thread.java:828)
> Caused by: java.time.format.DateTimeParseException: Text 'Mon Feb 21 19:10:31 
> IST 2022' could not be parsed at index 0
>   at 
> java.base/java.time.format.DateTimeFormatter.parseResolved0(DateTimeFormatter.java:2106)
>   at 
> java.base/java.time.format.DateTimeFormatter.parse(DateTimeFormatter.java:1934)
>   at PropertiesStoreTest.testDateComment(PropertiesStoreTest.java:253)
>   ... 30 more
> 
> (in the exception above a locale with language `he` is being used)
> 
> The root cause of this failure lies (only) within the tests - the 
> `DateTimeFormatter` used in the tests is locale sensitive and uses the 
> current (`Locale.getDefault()`) locale by default for parsing the date text. 
> This parsing fails because, although `Date.toString()` javadoc states nothing 
> about locales, it does mention the exact characters that will be used to 
> write out the date comment. In other words, this date comment written out is 
> locale insensitive and as such when parsing using `DateTimeFormatter` a 
> neutral locale is appropriate on the `DateTimeFormatter` instance. This PR 
> thus changes the tests to 

Re: RFR: 8282023: PropertiesStoreTest and StoreReproducibilityTest jtreg failures due to en_CA locale [v5]

[Jaikiran Pai]

On Fri, 25 Feb 2022 08:44:42 GMT, Jaikiran Pai  wrote:

>> Can I please get a review of this test only change which fixes the issue 
>> noted in https://bugs.openjdk.java.net/browse/JDK-8282023?
>> 
>> As noted in that JBS issue these tests fail when the default locale under 
>> which those tests are run isn't `en_US`. Both these tests were introduced as 
>> part of https://bugs.openjdk.java.net/browse/JDK-8231640. At a high level, 
>> these test do the following:
>> - Use Properties.store(...) APIs to write out a properties file. This 
>> internally ends up writing a date comment via a call to 
>> `java.util.Date.toString()` method.
>> - The test then runs a few checks to make sure that the written out `Date` 
>> is an expected one. To run these checks it uses the 
>> `java.time.format.DateTimeFormatter` to parse the date comment out of the 
>> properties file.
>> 
>> All this works fine when the default locale is (for example) `en_US`. 
>> However, when the default locale is (for example `en_CA` or even `hi_IN`) 
>> the tests fail with an exception in the latter step above while parsing the 
>> date comment using the `DateTimeFormatter` instance. The exception looks 
>> like:
>> 
>> 
>> Using locale: he for Properties#store(OutputStream) test
>> test PropertiesStoreTest.testStoreOutputStreamDateComment(he): failure
>> java.lang.AssertionError: Unexpected date comment: Mon Feb 21 19:10:31 IST 
>> 2022
>>  at org.testng.Assert.fail(Assert.java:87)
>>  at PropertiesStoreTest.testDateComment(PropertiesStoreTest.java:255)
>>  at 
>> PropertiesStoreTest.testStoreOutputStreamDateComment(PropertiesStoreTest.java:223)
>>  at 
>> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
>>  at java.base/java.lang.reflect.Method.invoke(Method.java:577)
>>  at 
>> org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
>>  at org.testng.internal.TestInvoker.invokeMethod(TestInvoker.java:599)
>>  at 
>> org.testng.internal.TestInvoker.invokeTestMethod(TestInvoker.java:174)
>>  at org.testng.internal.MethodRunner.runInSequence(MethodRunner.java:46)
>>  at 
>> org.testng.internal.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:822)
>>  at 
>> org.testng.internal.TestInvoker.invokeTestMethods(TestInvoker.java:147)
>>  at 
>> org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
>>  at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:128)
>>  at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
>>  at org.testng.TestRunner.privateRun(TestRunner.java:764)
>>  at org.testng.TestRunner.run(TestRunner.java:585)
>>  at org.testng.SuiteRunner.runTest(SuiteRunner.java:384)
>>  at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:378)
>>  at org.testng.SuiteRunner.privateRun(SuiteRunner.java:337)
>>  at org.testng.SuiteRunner.run(SuiteRunner.java:286)
>>  at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:53)
>>  at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:96)
>>  at org.testng.TestNG.runSuitesSequentially(TestNG.java:1218)
>>  at org.testng.TestNG.runSuitesLocally(TestNG.java:1140)
>>  at org.testng.TestNG.runSuites(TestNG.java:1069)
>>  at org.testng.TestNG.run(TestNG.java:1037)
>>  at 
>> com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:94)
>>  at 
>> com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:54)
>>  at 
>> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
>>  at java.base/java.lang.reflect.Method.invoke(Method.java:577)
>>  at 
>> com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
>>  at java.base/java.lang.Thread.run(Thread.java:828)
>> Caused by: java.time.format.DateTimeParseException: Text 'Mon Feb 21 
>> 19:10:31 IST 2022' could not be parsed at index 0
>>  at 
>> java.base/java.time.format.DateTimeFormatter.parseResolved0(DateTimeFormatter.java:2106)
>>  at 
>> java.base/java.time.format.DateTimeFormatter.parse(DateTimeFormatter.java:1934)
>>  at PropertiesStoreTest.testDateComment(PropertiesStoreTest.java:253)
>>  ... 30 more
>> 
>> (in the exception above a locale with language `he` is being used)
>> 
>> The root cause of this failure lies (only) within the tests - the 
>> `DateTimeFormatter` used in the tests is locale sensitive and uses the 
>> current (`Locale.getDefault()`) locale by default for parsing the date text. 
>> This parsing fails because, although `Date.toString()` javadoc states 
>> nothing about locales, it does mention the exact characters that will be 
>> used to write out the date comment. In other words, this date comment 
>> written out is locale insensitive and as such when parsing using 
>> `DateTimeFormatter` a neutral locale is appropriate on the 
>> 

Re: Should System.exit be controlled by a Scope Local?

[Bernd Eckenfels]

Alternatively you can make this “first setter wins” (either globally or per 
thread), then you don’t have to care or check from where the call is coming. 
Could be even integrated with a system property similar to the 
securitymanager=allow.

Gruss
Bernd


--
http://bernd.eckenfels.net

Von: core-libs-dev  im Auftrag von Kasper 
Nielsen 
Gesendet: Monday, February 28, 2022 9:23:11 PM
An: Ethan McCue 
Cc: core-libs-dev 
Betreff: Re: Should System.exit be controlled by a Scope Local?

Is there really a need to make this so complicated?

In all the examples I've seen so far it would be fine to set
system-exit restrictions up from the program's main class.
So why not just restrict it to the main class by default?
I assume this class is under the control of the user or
an IDE/Application Server.

Add this method to java.lang.Runtime

void restrictExit(MethodHandles.Lookup lookup, IntConsumer interceptor) {
  if (lookup.lookupClass() != "JAVA_MAIN_CLASS" ||
!lookup.hasFullPrivilegeAccess())
{
 throw new IllegalArgumentException("Invalid Lookup class");
  }
  ...
  Register interceptor to be called before System.exit
  ...
}

People could then call it, for example, from a static initializer block in
the
Main class. And use scope locals or whatever they want.

static {
  Runtime.restrictExit(MethodHandles.lookup(), ...)
}

Ideally, everyone would be using the module system, And we would have some
kind
of "application module" concept, which would be the module containing the
program's entry point. And which could have these special permissions by
default.
It might even be possible to delegate permissions to other modules if
needed.

/Kasper

On Sat, 26 Feb 2022 at 22:27, Ethan McCue  wrote:

> I have a feeling this has been considered and I might just be articulating
> the obvious - but:
>
> As called out in JEP 411, one of the remaining legitimate uses of the
> Security Manager is to intercept calls to System.exit. This seems like a
> decent use case for the Scope Local mechanism.
>
>
>

Re: RFR: 8282081: java.time.DateTimeFormatter: wrong definition of symbol F

[Joe Wang]

On Mon, 28 Feb 2022 23:17:57 GMT, Naoto Sato  wrote:

> Fixing the definition and implementation of the pattern symbol `F`. Although 
> it is an incompatible change, I believe it is worth the fix. For that, a CSR 
> has been drafted.

Marked as reviewed by joehw (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/7640

Re: RFR: 8282143: Objects.requireNonNull should be ForceInline [v2]

[Quan Anh Mai]

On Tue, 1 Mar 2022 02:22:49 GMT, Quan Anh Mai  wrote:

>> Hi,
>> 
>> `Objects.requireNonNull` may fail to be inlined. The call is expensive and 
>> may lead to objects escaping to the heap while the null check is cheap and 
>> is often elided. I have observed this when using the vector API when a call 
>> to `Objects.requireNonNull` leads to vectors being materialised in a hot 
>> loop.
>> 
>> Should the other `requireNonNull` be `ForceInline` as well?
>> 
>> Thank you very much.
>
> Quan Anh Mai has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   the other

I have applied the change for the other one as well. Thanks a lot.

-

PR: https://git.openjdk.java.net/jdk/pull/7543

Re: RFR: 8282081: java.time.DateTimeFormatter: wrong definition of symbol F

[Naoto Sato]

On Tue, 1 Mar 2022 01:47:45 GMT, Joe Wang  wrote:

> Was the following assessment in the bug report correct? "the symbol F in 
> java.time.DateTimeFormatter is no use in any pattern. It just may cause an 
> exception."

No, not correct. It is currently incorrectly tied with the 
`ChronoField.ALIGNED_DAY_OF_WEEK_IN_MONTH` field, and works as such.

> If true, then it seems the compatibility risk would be low since pattern 
> letter "F" as is currently defined is of no use.

Thus, the risk should remain `medium`. 

> Also, the CSR summary needs to be a summary of the action to be taken, that 
> is, changing the pattern definition. The current statement is similar to the 
> problem statement.

Thanks, modified.

-

PR: https://git.openjdk.java.net/jdk/pull/7640

Re: Future.isCancelled and thread interruption

[David Holmes]

Hi,

On 1/03/2022 4:17 am, Pushkar N Kulkarni wrote:

"Future.cancel(true)" cancels the receiver Future and also attempts to interrupt the 
executor thread that is running this task. However, not all threads may be interrupted. An 
exception are threads that executing one of the "restart-able blocking system calls" from 
libnet.
Such threads will ignore the thread interrupt(EINTR) and restart the blocking 
system call that was interrupted. See the system calls wrapped in the 
BLOCKING_IO_RETURN_INT macro: 
https://github.com/openjdk/jdk/blob/master/src/java.base/linux/native/libnet/linux_close.c#L352


You are mixing up two completely different notions of "interrupt". 
Thread.interrupt does not "interrupt" system calls - that only happens 
with signals. Thread.interrupt will not unblock low-level blocking I/O 
calls - like a socket connect. Only InterruptibleChannels provide some 
support for Thread.interrupt to break out of the I/O operation.


David
-



The javadoc for "java.util.concurrent.Future.cancel(boolean mayInterruptIfRunning)" DOES clarify that this 
method is an "attempt" to cancel the Future, and that it has no effect if the "task is already completed 
or cancelled, or could not be cancelled for *some other reason*". It is also made clear that "the return 
value from this method does not necessarily indicate whether the task is now cancelled". This is good, in the 
context of this note.

The javadoc also presents "Future.isCancelled()" as a definitive way to test if a Future 
was cancelled. However, it does not comment on the thread interruption attempted by 
Future.cancel(true). This might lead users to assume that if "Future.isCancelled()" 
returns true, the related executor thread was also successfully interrupted. This assumption would 
be invalid if the related executor thread was blocked in one of libnet's restart-able system calls 
(connect() could block for a couple of minutes).

I am attaching a test program that reproduces the mentioned behaviour. The executor 
thread held a lock and it was assumed that when "Future.isCancelled()" returned 
true, the executor had been interrupted and the lock released. In reality, the lock was 
held for a longer time and it blocked the main thread where the invalid assumption was 
made.

I am curious to know what others think of this matter! Any 
help/corrections/opinions will be appreciated. Thank you!

--
import java.net.*;
import java.util.concurrent.*;

public class ConnectionTest {
 private  synchronized Socket connect(String host, int port) throws 
Exception {
 InetSocketAddress address = new InetSocketAddress(host, port);
 Socket s = new Socket();
 s.connect(address); // HERE: s.connect(address, T), with any T>0, 
would resolve the hang!
 return s;
 }

 private Socket connectToMain() throws Exception {
 System.out.println("Connecting to main...");
 return connect("www.google.com", 81);
 }

 private Socket connectToAlternate() throws Exception {
 System.out.println("Connecting to alternate...");
 return connect("www.example.com", 80);
 }

 public void test() throws Exception {
 ExecutorService es = Executors.newFixedThreadPool(1);
 Future f = es.submit(new Callable() {
 public Socket call() throws Exception {
 return connectToMain();
 }
 });
 try {
 f.get(2000, TimeUnit.MILLISECONDS);
 System.out.println("Connected to main!");
 return;
 } catch (TimeoutException e) {
 System.out.println("Connection to main timed out, cancelling the Future 
with mayInterruptIfRunning = true");
 boolean ret = f.cancel(true);
 System.out.println("Future.cancel(true) returned " + ret);
 }
 System.out.println("Is Future canceled? ..." + f.isCancelled());
 if (f.isCancelled()) {
 connectToAlternate();
 System.out.println("Connected to alternate!");
 }
 }

 public static void main(String [] args) throws Exception {
 new ConnectionTest().test();
 }
}
--

-Pushkar

Re: RFR: 8282143: Objects.requireNonNull should be ForceInline [v2]

[Quan Anh Mai]

> Hi,
> 
> `Objects.requireNonNull` may fail to be inlined. The call is expensive and 
> may lead to objects escaping to the heap while the null check is cheap and is 
> often elided. I have observed this when using the vector API when a call to 
> `Objects.requireNonNull` leads to vectors being materialised in a hot loop.
> 
> Should the other `requireNonNull` be `ForceInline` as well?
> 
> Thank you very much.

Quan Anh Mai has updated the pull request incrementally with one additional 
commit since the last revision:

  the other

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7543/files
  - new: https://git.openjdk.java.net/jdk/pull/7543/files/bcd22b9d..b362bdca

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7543=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7543=00-01

  Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7543.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7543/head:pull/7543

PR: https://git.openjdk.java.net/jdk/pull/7543

Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v16]

[Joe Darcy]

> This is an early review of changes to better model JVM access flags, that is 
> "modifiers" like public, protected, etc. but explicitly at a VM level.
> 
> Language level modifiers and JVM level access flags are closely related, but 
> distinct. There are concepts that overlap in the two domains (public, 
> private, etc.), others that only have a language-level modifier (sealed), and 
> still others that only have an access flag (synthetic).
> 
> The existing java.lang.reflect.Modifier class is inadequate to model these 
> subtleties. For example, the bit positions used by access flags on different 
> kinds of elements overlap (such as "volatile" for fields and "bridge" for 
> methods. Just having a raw integer does not provide sufficient context to 
> decode the corresponding language-level string. Methods like 
> Modifier.methodModifiers() were introduced to cope with this situation.
> 
> With additional modifiers and flags on the horizon with projects like 
> Valhalla, addressing the existent modeling deficiency now ahead of time is 
> reasonable before further strain is introduced.
> 
> This PR in its current form is meant to give the overall shape of the API. It 
> is missing implementations to map from, say, method modifiers to access 
> flags, taking into account overlaps in bit positions.
> 
> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in 
> once the API is further along.

Joe Darcy has updated the pull request with a new target base due to a merge or 
a rebase. The incremental webrev excludes the unrelated changes brought in by 
the merge/rebase. The pull request contains 24 additional commits since the 
last revision:

 - Make workding changes suggested in review feedback.
 - Merge branch 'master' into JDK-8266670
 - Typo fix; add implSpec to Executable.
 - Appease jcheck.
 - Fix some bugs found by inspection, docs cleanup.
 - Merge branch 'master' into JDK-8266670
 - Initial support for accessFlags methods
 - Add mask to access flag functionality.
 - Add test for location disjointness
 - Minor cleanup.
 - ... and 14 more: https://git.openjdk.java.net/jdk/compare/b096a4cb...e63fb13e

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7445/files
  - new: https://git.openjdk.java.net/jdk/pull/7445/files/aa2b5eed..e63fb13e

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7445=15
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7445=14-15

  Stats: 8741 lines in 190 files changed: 6567 ins; 683 del; 1491 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7445.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7445/head:pull/7445

PR: https://git.openjdk.java.net/jdk/pull/7445

Re: RFR: 8282081: java.time.DateTimeFormatter: wrong definition of symbol F

[Joe Wang]

On Mon, 28 Feb 2022 23:17:57 GMT, Naoto Sato  wrote:

> Fixing the definition and implementation of the pattern symbol `F`. Although 
> it is an incompatible change, I believe it is worth the fix. For that, a CSR 
> has been drafted.

Was the following assessment in the bug report correct?
"the symbol F in java.time.DateTimeFormatter is no use in any pattern. It 
just may cause an exception."

If true, then it seems the compatibility risk would be low since pattern letter 
"F" as is currently defined is of no use.

Also, the CSR summary needs to be a summary of the action to be taken, that is, 
changing the pattern definition. The current statement is similar to the 
problem statement.

-

PR: https://git.openjdk.java.net/jdk/pull/7640

Re: RFR: 8279598: Provide adapter from RandomGenerator to Random [v12]

[Yasser Bazzi]

> Hi, could i get a review on this implementation proposed by Stuart Marks, i 
> decided to use the 
> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/random/RandomGenerator.html
>  interface to create the default method `asRandom()` that wraps around the 
> newer algorithms to be used on classes that do not accept the new interface.
> 
> Some things to note as proposed by the bug report, the protected method 
> next(int bits) is not overrided and setSeed() method if left blank up to 
> discussion on what to do with it.
> 
> Small test done on 
> https://gist.github.com/YShow/da678561419cda8e32fccf3a27a649d4

Yasser Bazzi has updated the pull request incrementally with one additional 
commit since the last revision:

  fix wrong identation
  
  Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com>

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7001/files
  - new: https://git.openjdk.java.net/jdk/pull/7001/files/60c1e382..0e48edf6

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7001=11
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7001=10-11

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7001.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7001/head:pull/7001

PR: https://git.openjdk.java.net/jdk/pull/7001

Re: RFR: 8279598: Provide adapter from RandomGenerator to Random [v11]

[liach]

On Mon, 28 Feb 2022 18:49:59 GMT, ExE Boss  wrote:

>> Yasser Bazzi has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update javadoc
>
> src/java.base/share/classes/java/util/Random.java line 93:
> 
>> 91: 
>> 92: @SuppressWarnings("serial")
>> 93: private static class RandomWrapper extends Random implements 
>> RandomGenerator {
> 
> This class should also override all the default methods in `RandomGenerator` 
> and forward them to `this.generator`.

The method overridden in this class are exactly those present in 
`RandomGenerator` itself, which are also the API methods of `Random`.

-

PR: https://git.openjdk.java.net/jdk/pull/7001

Re: RFR: 8282143: Objects.requireNonNull should be ForceInline

[Remi Forax]

- Original Message -
> From: "Paul Sandoz" 
> To: "core-libs-dev" 
> Sent: Tuesday, March 1, 2022 1:48:02 AM
> Subject: Re: RFR: 8282143: Objects.requireNonNull should be ForceInline

> On Sat, 19 Feb 2022 05:51:52 GMT, Quan Anh Mai  wrote:
> 
>> Hi,
>> 
>> `Objects.requireNonNull` may fail to be inlined. The call is expensive and 
>> may
>> lead to objects escaping to the heap while the null check is cheap and is 
>> often
>> elided. I have observed this when using the vector API when a call to
>> `Objects.requireNonNull` leads to vectors being materialised in a hot loop.
>> 
>> Should the other `requireNonNull` be `ForceInline` as well?
>> 
>> Thank you very much.
> 
> `Objects.requireNonNull` is also used on the critical path of VarHandles, and
> other places in `j.l.invoke` so I think this is generally beneficial beyond 
> use
> by the Vector API.

It is also use by javac when the JLS requires to emit a nullcheck, like for 
example when creating a method reference.

> 
> -
> 
> PR: https://git.openjdk.java.net/jdk/pull/7543

Re: RFR: 8282143: Objects.requireNonNull should be ForceInline

[Paul Sandoz]

On Sat, 19 Feb 2022 05:51:52 GMT, Quan Anh Mai  wrote:

> Hi,
> 
> `Objects.requireNonNull` may fail to be inlined. The call is expensive and 
> may lead to objects escaping to the heap while the null check is cheap and is 
> often elided. I have observed this when using the vector API when a call to 
> `Objects.requireNonNull` leads to vectors being materialised in a hot loop.
> 
> Should the other `requireNonNull` be `ForceInline` as well?
> 
> Thank you very much.

`Objects.requireNonNull` is also used on the critical path of VarHandles, and 
other places in `j.l.invoke` so I think this is generally beneficial beyond use 
by the Vector API.

-

PR: https://git.openjdk.java.net/jdk/pull/7543

Re: RFR: 8282143: Objects.requireNonNull should be ForceInline

[John R Rose]

On Sat, 19 Feb 2022 05:51:52 GMT, Quan Anh Mai  wrote:

> Hi,
> 
> `Objects.requireNonNull` may fail to be inlined. The call is expensive and 
> may lead to objects escaping to the heap while the null check is cheap and is 
> often elided. I have observed this when using the vector API when a call to 
> `Objects.requireNonNull` leads to vectors being materialised in a hot loop.
> 
> Should the other `requireNonNull` be `ForceInline` as well?
> 
> Thank you very much.

Very simple methods like this are almost always inlined, but I suppose there 
are depth cutoffs that may cause inlining to fail.

Yes, it should always be inlined.  The optimizer works hard to track null-ness 
conditions, and making this call go out of line "breaks the thread of thought" 
in the optimizer's reasoning about null.

Yes, both versions of `O::cNN` should be marked this way; the only difference 
is in how the exception is constructed.

(A similar point goes for index-checking functions, which are also 
force-inlined so that simple index checks of the form `i*K+L https://git.openjdk.java.net/jdk/pull/7543

RFR: 8282081: java.time.DateTimeFormatter: wrong definition of symbol F

[Naoto Sato]

Fixing the definition and implementation of the pattern symbol `F`. Although it 
is an incompatible change, I believe it is worth the fix. For that, a CSR has 
been drafted.

-

Commit messages:
 - 8282081: java.time.DateTimeFormatter: wrong definition of symbol F

Changes: https://git.openjdk.java.net/jdk/pull/7640/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7640=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8282081
  Stats: 5 lines in 3 files changed: 0 ins; 0 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7640.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7640/head:pull/7640

PR: https://git.openjdk.java.net/jdk/pull/7640

Re: RFR: 8282008: Incorrect handling of quoted arguments in ProcessBuilder [v3]

[Roger Riggs]

On Fri, 18 Feb 2022 17:21:48 GMT, Olga Mikhaltsova  
wrote:

>> This fix made equal processing of strings such as ""C:\\Program 
>> Files\\Git\\"" before and after JDK-8250568.
>> 
>> For example, it's needed to execute the following command on Windows:
>> `C:\Windows\SysWOW64\WScript.exe "MyVB.vbs" "C:\Program Files\Git" "Test"`
>> it's equal to:
>> `new ProcessBuilder("C:\\Windows\\SysWOW64\\WScript.exe", "MyVB.vbs", 
>> ""C:\\Program Files\\Git\\"", "Test").start();`
>> 
>> While processing, the 3rd argument ""C:\\Program Files\\Git\\"" treated as 
>> unquoted due to the condition added in JDK-8250568.
>> 
>> private static String unQuote(String str) {
>> .. 
>>if (str.endsWith("\\"")) {
>> return str;// not properly quoted, treat as unquoted
>> }
>> ..
>> }
>> 
>> 
>> that leads to the additional surrounding by quotes in 
>> ProcessImpl::createCommandLine(..) because needsEscaping(..) returns true 
>> due to the space inside the string argument.
>> As a result the native function CreateProcessW 
>> (src/java.base/windows/native/libjava/ProcessImpl_md.c) gets the incorrectly 
>> quoted argument: 
>> 
>> pcmd = C:\Windows\SysWOW64\WScript.exe MyVB.vbs ""C:\Program Files\Git"" Test
>> (jdk.lang.Process.allowAmbiguousCommands = true)
>> pcmd = "C:\Windows\SysWOW64\WScript.exe" MyVB.vbs ""C:\Program Files\Git\\"" 
>> Test
>> (jdk.lang.Process.allowAmbiguousCommands = false)
>> 
>> 
>> Obviously, a string ending with `"\\""` must not be started with `"""` to 
>> treat as unquoted overwise it’s should be treated as properly quoted.
>
> Olga Mikhaltsova has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Add a new line to the end of test file for JDK-8282008

(I'm still working on a more nuanced fix that works with .exe, .cmd, and with 
allowAmbiguousCommands both true and false).

The suggested workaround was to remove the application quotes and let 
ProcessBuilder do the quoting.
That resulted in an extra backslash "\" at the end of a file path. In my 
investigation, the extra "\" doesn't prevent the 
string from being correctly used as a directory path in either VisualBasic or 
cmd.exe.
So I'm curious, in the original application that uncovered this problem, what 
is/was reported as the error?
Was the original application retested with the workaround?

The case of the backslash at the end of an argument occurs mainly in a 
directory path.
Yes, the argument is different, but does it make a difference that matters in 
the context in which it appears.

-

PR: https://git.openjdk.java.net/jdk/pull/7504

Integrated: 8275731: CDS archived enums objects are recreated at runtime

[Ioi Lam]

On Wed, 1 Dec 2021 20:47:20 GMT, Ioi Lam  wrote:

> **Background:**
> 
> In the Java Language, Enums can be tested for equality, so the constants in 
> an Enum type must be unique. Javac compiles an enum declaration like this:
> 
> 
> public enum Day {  SUNDAY, MONDAY ... } 
> 
> 
> to
> 
> 
> public class Day extends java.lang.Enum {
> public static final SUNDAY = new Day("SUNDAY");
> public static final MONDAY = new Day("MONDAY"); ...
> }
> 
> 
> With CDS archived heap objects, `Day::` is executed twice: once 
> during `java -Xshare:dump`, and once during normal JVM execution. If the 
> archived heap objects references one of the Enum constants created at dump 
> time, we will violate the uniqueness requirements of the Enum constants at 
> runtime. See the test case in the description of 
> [JDK-8275731](https://bugs.openjdk.java.net/browse/JDK-8275731)
> 
> **Fix:**
> 
> During -Xshare:dump, if we discovered that an Enum constant of type X is 
> archived, we archive all constants of type X. At Runtime, type X will skip 
> the normal execution of `X::`. Instead, we run 
> `HeapShared::initialize_enum_klass()` to retrieve all the constants of X that 
> were saved at dump time.
> 
> This is safe as we know that `X::` has no observable side effect -- 
> it only creates the constants of type X, as well as the synthetic value 
> `X::$VALUES`, which cannot be observed until X is fully initialized.
> 
> **Verification:**
> 
> To avoid future problems, I added a new tool, CDSHeapVerifier, to look for 
> similar problems where the archived heap objects reference a static field 
> that may be recreated at runtime. There are some manual steps involved, but I 
> analyzed the potential problems found by the tool are they are all safe 
> (after the current bug is fixed). See cdsHeapVerifier.cpp for gory details. 
> An example trace of this tool can be found at 
> https://bugs.openjdk.java.net/secure/attachment/97242/enum_warning.txt
> 
> **Testing:**
> 
> Passed Oracle CI tiers 1-4. WIll run tier 5 as well.

This pull request has now been integrated.

Changeset: d983d108
Author:Ioi Lam 
URL:   
https://git.openjdk.java.net/jdk/commit/d983d108c565654e717e2811d88aa94d982da2f5
Stats: 860 lines in 16 files changed: 807 ins; 4 del; 49 mod

8275731: CDS archived enums objects are recreated at runtime

Reviewed-by: coleenp, ccheung

-

PR: https://git.openjdk.java.net/jdk/pull/6653

Re: RFR: 8275731: CDS archived enums objects are recreated at runtime [v4]

[Ioi Lam]

On Thu, 17 Feb 2022 23:20:41 GMT, Calvin Cheung  wrote:

>> Ioi Lam has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Use InstanceKlass::do_local_static_fields for some field iterations
>
> Looks good. Minor comment below.
> Also, several files with copyright year 2021 need updating.

Thanks @calvinccheung and @coleenp for the review. Passed tiers 1-5.

-

PR: https://git.openjdk.java.net/jdk/pull/6653

Re: Should System.exit be controlled by a Scope Local?

[Kasper Nielsen]

Is there really a need to make this so complicated?

In all the examples I've seen so far it would be fine to set
system-exit restrictions up from the program's main class.
So why not just restrict it to the main class by default?
I assume this class is under the control of the user or
an IDE/Application Server.

Add this method to java.lang.Runtime

void restrictExit(MethodHandles.Lookup lookup, IntConsumer interceptor) {
  if (lookup.lookupClass() != "JAVA_MAIN_CLASS" ||
!lookup.hasFullPrivilegeAccess())
{
 throw new IllegalArgumentException("Invalid Lookup class");
  }
  ...
  Register interceptor to be called before System.exit
  ...
}

People could then call it, for example, from a static initializer block in
the
Main class. And use scope locals or whatever they want.

static {
  Runtime.restrictExit(MethodHandles.lookup(), ...)
}

Ideally, everyone would be using the module system, And we would have some
kind
of "application module" concept, which would be the module containing the
program's entry point. And which could have these special permissions by
default.
It might even be possible to delegate permissions to other modules if
needed.

/Kasper

On Sat, 26 Feb 2022 at 22:27, Ethan McCue  wrote:

> I have a feeling this has been considered and I might just be articulating
> the obvious - but:
>
> As called out in JEP 411, one of the remaining legitimate uses of the
> Security Manager is to intercept calls to System.exit. This seems like a
> decent use case for the Scope Local mechanism.
>
>
>

Re: RFR: 8279598: Provide adapter from RandomGenerator to Random [v6]

[ExE Boss]

On Fri, 25 Feb 2022 22:59:05 GMT, liach  wrote:

>> src/java.base/share/classes/java/util/Random.java line 298:
>> 
>>> 296:  */
>>> 297: public static Random from(RandomGenerator random) {
>>> 298: return RandomWrapper.wrap(random);
>> 
>> Might be good to check here or in the called methods / constructors for 
>> `null`. Currently `null` would not be noticed until the first method is 
>> called on the created `Random`, which makes it difficult for the user to 
>> track down bugs in their code.
>
> Suggestion:
> 
> Objects.requireNonNull(random);
> return RandomWrapper.wrap(random);
> 
> fyi this is the original change suggested by marcono1234. Notice that this 
> might need a csr update; maybe a javadoc update is needed too

Arguably, `RandomWrapper.wrap` should be inlined here, since it’s not used 
anywhere else:
Suggestion:

if (Objects.requireNonNull(random, "random") instanceof Random) {
return (Random) random;
}
return new RandomWrapper(random);

-

PR: https://git.openjdk.java.net/jdk/pull/7001

Re: RFR: 8279598: Provide adapter from RandomGenerator to Random [v11]

[ExE Boss]

On Fri, 25 Feb 2022 23:27:45 GMT, Yasser Bazzi  wrote:

>> Hi, could i get a review on this implementation proposed by Stuart Marks, i 
>> decided to use the 
>> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/random/RandomGenerator.html
>>  interface to create the default method `asRandom()` that wraps around the 
>> newer algorithms to be used on classes that do not accept the new interface.
>> 
>> Some things to note as proposed by the bug report, the protected method 
>> next(int bits) is not overrided and setSeed() method if left blank up to 
>> discussion on what to do with it.
>> 
>> Small test done on 
>> https://gist.github.com/YShow/da678561419cda8e32fccf3a27a649d4
>
> Yasser Bazzi has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update javadoc

src/java.base/share/classes/java/util/Random.java line 84:

> 82: i = 48, j = 0, k = 0,
> 83: equidistribution = 0
> 84: )

This should probably not be indented:
Suggestion:

)

src/java.base/share/classes/java/util/Random.java line 93:

> 91: 
> 92: @SuppressWarnings("serial")
> 93: private static class RandomWrapper extends Random implements 
> RandomGenerator {

This class should also override all the default methods in `RandomGenerator` 
and forward them to `this.generator`.

-

PR: https://git.openjdk.java.net/jdk/pull/7001

Re: RFR: 8282444: Module finder incorrectly assumes default file system path-separator character [v2]

[Alan Bateman]

On Mon, 28 Feb 2022 16:38:27 GMT, Chris Hegarty  wrote:

>> The module finder implementation incorrectly uses the path-separator
>> character from the default file system, when mapping the relative path
>> of an entry in an exploded module to a package name. This causes
>> problems on Windows [*] when using a module finder with a custom file
>> system that has a path-separator character that differs from that of the
>> default file system, e.g. the zip file system (which uses '/',
>> rather than '\' ).
>> 
>> Rather than adding a new test for this, I deciced to just modify an
>> existing one. The existing test exercises the module finder with a
>> custom file system, but does so with modules have trivial single-level
>> packages names. I just expanded the module to have a subpackage. This
>> is sufficient to reproduce the bug, and verify the fix.
>> 
>> [*]
>> 
>> java.lang.module.FindException: Error reading module: /m2
>> at 
>> java.base/jdk.internal.module.ModulePath.readModule(ModulePath.java:350)
>> at 
>> java.base/jdk.internal.module.ModulePath.scanDirectory(ModulePath.java:284)
>> at java.base/jdk.internal.module.ModulePath.scan(ModulePath.java:232)
>> at 
>> java.base/jdk.internal.module.ModulePath.scanNextEntry(ModulePath.java:190)
>> at 
>> java.base/jdk.internal.module.ModulePath.findAll(ModulePath.java:166)
>> at 
>> ModulesInCustomFileSystem.listAllModules(ModulesInCustomFileSystem.java:108)
>> at 
>> ModulesInCustomFileSystem.testZipFileSystem(ModulesInCustomFileSystem.java:97)
>> at 
>> ModulesInCustomFileSystem.testExplodedModulesInZipFileSystem(ModulesInCustomFileSystem.java:68)
>> at ...
>> Caused by: java.lang.module.InvalidModuleDescriptorException: Package q.r 
>> not found in module
>> at 
>> java.base/jdk.internal.module.ModuleInfo.invalidModuleDescriptor(ModuleInfo.java:1212)
>> at 
>> java.base/jdk.internal.module.ModuleInfo.doRead(ModuleInfo.java:330)
>> at java.base/jdk.internal.module.ModuleInfo.read(ModuleInfo.java:129)
>> at 
>> java.base/jdk.internal.module.ModulePath.readExplodedModule(ModulePath.java:689)
>> at 
>> java.base/jdk.internal.module.ModulePath.readModule(ModulePath.java:320)
>> ... 36 more
>
> Chris Hegarty has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - update copyright year
>  - add tag with OrigBugId 8178380, and bugFixId 8282444

Marked as reviewed by alanb (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/7632

Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v12]

[ExE Boss]

On Thu, 24 Feb 2022 01:17:23 GMT, Joe Darcy  wrote:

> It does because of the AccessFlags.MODULE constant.

But that’s exactly what the unqualified `MODULE` identifier refers to, and 
there’s no other bare `MODULE` identifier in scope that would shadow the 
`AccessFlag.MODULE` constant from the POV of `AccessFlag.LocationToFlags`.

-

PR: https://git.openjdk.java.net/jdk/pull/7445

Future.isCancelled and thread interruption

[Pushkar N Kulkarni]

"Future.cancel(true)" cancels the receiver Future and also attempts to 
interrupt the executor thread that is running this task. However, not all 
threads may be interrupted. An exception are threads that executing one of the 
"restart-able blocking system calls" from libnet.  
Such threads will ignore the thread interrupt(EINTR) and restart the blocking 
system call that was interrupted. See the system calls wrapped in the 
BLOCKING_IO_RETURN_INT macro: 
https://github.com/openjdk/jdk/blob/master/src/java.base/linux/native/libnet/linux_close.c#L352


The javadoc for "java.util.concurrent.Future.cancel(boolean 
mayInterruptIfRunning)" DOES clarify that this method is an "attempt" to cancel 
the Future, and that it has no effect if the "task is already completed or 
cancelled, or could not be cancelled for *some other reason*". It is also made 
clear that "the return value from this method does not necessarily indicate 
whether the task is now cancelled". This is good, in the context of this note.

The javadoc also presents "Future.isCancelled()" as a definitive way to test if 
a Future was cancelled. However, it does not comment on the thread interruption 
attempted by Future.cancel(true). This might lead users to assume that if 
"Future.isCancelled()" returns true, the related executor thread was also 
successfully interrupted. This assumption would be invalid if the related 
executor thread was blocked in one of libnet's restart-able system calls 
(connect() could block for a couple of minutes). 

I am attaching a test program that reproduces the mentioned behaviour. The 
executor thread held a lock and it was assumed that when "Future.isCancelled()" 
returned true, the executor had been interrupted and the lock released. In 
reality, the lock was held for a longer time and it blocked the main thread 
where the invalid assumption was made. 

I am curious to know what others think of this matter! Any 
help/corrections/opinions will be appreciated. Thank you!

--
import java.net.*;
import java.util.concurrent.*;

public class ConnectionTest {
private  synchronized Socket connect(String host, int port) throws 
Exception {
InetSocketAddress address = new InetSocketAddress(host, port);
Socket s = new Socket();
s.connect(address); // HERE: s.connect(address, T), with any T>0, would 
resolve the hang!
return s;
}

private Socket connectToMain() throws Exception {
System.out.println("Connecting to main...");
return connect("www.google.com", 81);
}

private Socket connectToAlternate() throws Exception {
System.out.println("Connecting to alternate...");
return connect("www.example.com", 80);
}

public void test() throws Exception {
ExecutorService es = Executors.newFixedThreadPool(1);
Future f = es.submit(new Callable() {
public Socket call() throws Exception {
return connectToMain();
}
}); 
try {
f.get(2000, TimeUnit.MILLISECONDS);
System.out.println("Connected to main!");
return;
} catch (TimeoutException e) {
System.out.println("Connection to main timed out, cancelling the 
Future with mayInterruptIfRunning = true");
boolean ret = f.cancel(true);
System.out.println("Future.cancel(true) returned " + ret);
}
System.out.println("Is Future canceled? ..." + f.isCancelled());
if (f.isCancelled()) {
connectToAlternate();
System.out.println("Connected to alternate!");
}
}

public static void main(String [] args) throws Exception {
new ConnectionTest().test();
}
}
--

-Pushkar

Re: RFR: 8282023: PropertiesStoreTest and StoreReproducibilityTest jtreg failures due to en_CA locale [v5]

[Roger Riggs]

On Fri, 25 Feb 2022 08:44:42 GMT, Jaikiran Pai  wrote:

>> Can I please get a review of this test only change which fixes the issue 
>> noted in https://bugs.openjdk.java.net/browse/JDK-8282023?
>> 
>> As noted in that JBS issue these tests fail when the default locale under 
>> which those tests are run isn't `en_US`. Both these tests were introduced as 
>> part of https://bugs.openjdk.java.net/browse/JDK-8231640. At a high level, 
>> these test do the following:
>> - Use Properties.store(...) APIs to write out a properties file. This 
>> internally ends up writing a date comment via a call to 
>> `java.util.Date.toString()` method.
>> - The test then runs a few checks to make sure that the written out `Date` 
>> is an expected one. To run these checks it uses the 
>> `java.time.format.DateTimeFormatter` to parse the date comment out of the 
>> properties file.
>> 
>> All this works fine when the default locale is (for example) `en_US`. 
>> However, when the default locale is (for example `en_CA` or even `hi_IN`) 
>> the tests fail with an exception in the latter step above while parsing the 
>> date comment using the `DateTimeFormatter` instance. The exception looks 
>> like:
>> 
>> 
>> Using locale: he for Properties#store(OutputStream) test
>> test PropertiesStoreTest.testStoreOutputStreamDateComment(he): failure
>> java.lang.AssertionError: Unexpected date comment: Mon Feb 21 19:10:31 IST 
>> 2022
>>  at org.testng.Assert.fail(Assert.java:87)
>>  at PropertiesStoreTest.testDateComment(PropertiesStoreTest.java:255)
>>  at 
>> PropertiesStoreTest.testStoreOutputStreamDateComment(PropertiesStoreTest.java:223)
>>  at 
>> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
>>  at java.base/java.lang.reflect.Method.invoke(Method.java:577)
>>  at 
>> org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
>>  at org.testng.internal.TestInvoker.invokeMethod(TestInvoker.java:599)
>>  at 
>> org.testng.internal.TestInvoker.invokeTestMethod(TestInvoker.java:174)
>>  at org.testng.internal.MethodRunner.runInSequence(MethodRunner.java:46)
>>  at 
>> org.testng.internal.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:822)
>>  at 
>> org.testng.internal.TestInvoker.invokeTestMethods(TestInvoker.java:147)
>>  at 
>> org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
>>  at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:128)
>>  at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
>>  at org.testng.TestRunner.privateRun(TestRunner.java:764)
>>  at org.testng.TestRunner.run(TestRunner.java:585)
>>  at org.testng.SuiteRunner.runTest(SuiteRunner.java:384)
>>  at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:378)
>>  at org.testng.SuiteRunner.privateRun(SuiteRunner.java:337)
>>  at org.testng.SuiteRunner.run(SuiteRunner.java:286)
>>  at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:53)
>>  at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:96)
>>  at org.testng.TestNG.runSuitesSequentially(TestNG.java:1218)
>>  at org.testng.TestNG.runSuitesLocally(TestNG.java:1140)
>>  at org.testng.TestNG.runSuites(TestNG.java:1069)
>>  at org.testng.TestNG.run(TestNG.java:1037)
>>  at 
>> com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:94)
>>  at 
>> com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:54)
>>  at 
>> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
>>  at java.base/java.lang.reflect.Method.invoke(Method.java:577)
>>  at 
>> com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
>>  at java.base/java.lang.Thread.run(Thread.java:828)
>> Caused by: java.time.format.DateTimeParseException: Text 'Mon Feb 21 
>> 19:10:31 IST 2022' could not be parsed at index 0
>>  at 
>> java.base/java.time.format.DateTimeFormatter.parseResolved0(DateTimeFormatter.java:2106)
>>  at 
>> java.base/java.time.format.DateTimeFormatter.parse(DateTimeFormatter.java:1934)
>>  at PropertiesStoreTest.testDateComment(PropertiesStoreTest.java:253)
>>  ... 30 more
>> 
>> (in the exception above a locale with language `he` is being used)
>> 
>> The root cause of this failure lies (only) within the tests - the 
>> `DateTimeFormatter` used in the tests is locale sensitive and uses the 
>> current (`Locale.getDefault()`) locale by default for parsing the date text. 
>> This parsing fails because, although `Date.toString()` javadoc states 
>> nothing about locales, it does mention the exact characters that will be 
>> used to write out the date comment. In other words, this date comment 
>> written out is locale insensitive and as such when parsing using 
>> `DateTimeFormatter` a neutral locale is appropriate on the 
>> 

Re: RFR: 8282444: Module finder incorrectly assumes default file system path-separator character

[Chris Hegarty]

On Mon, 28 Feb 2022 14:34:43 GMT, Alan Bateman  wrote:

> we should create another issue to create more tests for custom file systems.

Yeah, that's a good point. I'll take a closer look at this and see how best to 
expand the coverage (separately).

> I assume you'll update the date in the copyright header of the changed files.

Done.

>> Hi Chris, maybe you should add @bug 8282444 to 
>> ModulesInCustomFileSystem.java too?

> The downside is that it would make it look like the test was added fro this 
> issue. I think it only works if the original issue for the module system 
> implementation is there too.

I added a tag with both the original bugId that introduced the test, 8178380, 
and bug Fix Id for this issue, 8282444.

-

PR: https://git.openjdk.java.net/jdk/pull/7632

Re: RFR: 8282444: Module finder incorrectly assumes default file system path-separator character [v2]

[Chris Hegarty]

> The module finder implementation incorrectly uses the path-separator
> character from the default file system, when mapping the relative path
> of an entry in an exploded module to a package name. This causes
> problems on Windows [*] when using a module finder with a custom file
> system that has a path-separator character that differs from that of the
> default file system, e.g. the zip file system (which uses '/',
> rather than '\' ).
> 
> Rather than adding a new test for this, I deciced to just modify an
> existing one. The existing test exercises the module finder with a
> custom file system, but does so with modules have trivial single-level
> packages names. I just expanded the module to have a subpackage. This
> is sufficient to reproduce the bug, and verify the fix.
> 
> [*]
> 
> java.lang.module.FindException: Error reading module: /m2
> at 
> java.base/jdk.internal.module.ModulePath.readModule(ModulePath.java:350)
> at 
> java.base/jdk.internal.module.ModulePath.scanDirectory(ModulePath.java:284)
> at java.base/jdk.internal.module.ModulePath.scan(ModulePath.java:232)
> at 
> java.base/jdk.internal.module.ModulePath.scanNextEntry(ModulePath.java:190)
> at 
> java.base/jdk.internal.module.ModulePath.findAll(ModulePath.java:166)
> at 
> ModulesInCustomFileSystem.listAllModules(ModulesInCustomFileSystem.java:108)
> at 
> ModulesInCustomFileSystem.testZipFileSystem(ModulesInCustomFileSystem.java:97)
> at 
> ModulesInCustomFileSystem.testExplodedModulesInZipFileSystem(ModulesInCustomFileSystem.java:68)
> at ...
> Caused by: java.lang.module.InvalidModuleDescriptorException: Package q.r not 
> found in module
> at 
> java.base/jdk.internal.module.ModuleInfo.invalidModuleDescriptor(ModuleInfo.java:1212)
> at 
> java.base/jdk.internal.module.ModuleInfo.doRead(ModuleInfo.java:330)
> at java.base/jdk.internal.module.ModuleInfo.read(ModuleInfo.java:129)
> at 
> java.base/jdk.internal.module.ModulePath.readExplodedModule(ModulePath.java:689)
> at 
> java.base/jdk.internal.module.ModulePath.readModule(ModulePath.java:320)
> ... 36 more

Chris Hegarty has updated the pull request incrementally with two additional 
commits since the last revision:

 - update copyright year
 - add tag with OrigBugId 8178380, and bugFixId 8282444

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7632/files
  - new: https://git.openjdk.java.net/jdk/pull/7632/files/ca6cdf4c..220c43c2

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7632=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7632=00-01

  Stats: 3 lines in 2 files changed: 1 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7632.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7632/head:pull/7632

PR: https://git.openjdk.java.net/jdk/pull/7632

Re: RFR: 8282444: Module finder incorrectly assumes default file system path-separator character

[Alan Bateman]

On Mon, 28 Feb 2022 16:16:31 GMT, Daniel Fuchs  wrote:

> Hi Chris, maybe you should add `@bug 8282444` to 
> `ModulesInCustomFileSystem.java` too?

The downside is that it would make it look like the test was added fro this 
issue. I think it only works if the original issue for the module system 
implementation is there too.

-

PR: https://git.openjdk.java.net/jdk/pull/7632

Re: RFR: 8282444: Module finder incorrectly assumes default file system path-separator character

[Daniel Fuchs]

On Mon, 28 Feb 2022 11:12:17 GMT, Chris Hegarty  wrote:

> The module finder implementation incorrectly uses the path-separator
> character from the default file system, when mapping the relative path
> of an entry in an exploded module to a package name. This causes
> problems on Windows [*] when using a module finder with a custom file
> system that has a path-separator character that differs from that of the
> default file system, e.g. the zip file system (which uses '/',
> rather than '\' ).
> 
> Rather than adding a new test for this, I deciced to just modify an
> existing one. The existing test exercises the module finder with a
> custom file system, but does so with modules have trivial single-level
> packages names. I just expanded the module to have a subpackage. This
> is sufficient to reproduce the bug, and verify the fix.
> 
> [*]
> 
> java.lang.module.FindException: Error reading module: /m2
> at 
> java.base/jdk.internal.module.ModulePath.readModule(ModulePath.java:350)
> at 
> java.base/jdk.internal.module.ModulePath.scanDirectory(ModulePath.java:284)
> at java.base/jdk.internal.module.ModulePath.scan(ModulePath.java:232)
> at 
> java.base/jdk.internal.module.ModulePath.scanNextEntry(ModulePath.java:190)
> at 
> java.base/jdk.internal.module.ModulePath.findAll(ModulePath.java:166)
> at 
> ModulesInCustomFileSystem.listAllModules(ModulesInCustomFileSystem.java:108)
> at 
> ModulesInCustomFileSystem.testZipFileSystem(ModulesInCustomFileSystem.java:97)
> at 
> ModulesInCustomFileSystem.testExplodedModulesInZipFileSystem(ModulesInCustomFileSystem.java:68)
> at ...
> Caused by: java.lang.module.InvalidModuleDescriptorException: Package q.r not 
> found in module
> at 
> java.base/jdk.internal.module.ModuleInfo.invalidModuleDescriptor(ModuleInfo.java:1212)
> at 
> java.base/jdk.internal.module.ModuleInfo.doRead(ModuleInfo.java:330)
> at java.base/jdk.internal.module.ModuleInfo.read(ModuleInfo.java:129)
> at 
> java.base/jdk.internal.module.ModulePath.readExplodedModule(ModulePath.java:689)
> at 
> java.base/jdk.internal.module.ModulePath.readModule(ModulePath.java:320)
> ... 36 more

Hi Chris, maybe you should add `@bug 8282444` to 
`ModulesInCustomFileSystem.java` too?

-

PR: https://git.openjdk.java.net/jdk/pull/7632

Re: Should System.exit be controlled by a Scope Local?

[Andrew Haley]

On 2/28/22 15:12, Sean Mullan wrote:
>
> On 2/27/22 1:47 PM, Andrew Haley wrote:
>
>> I'd like to explore the use of scope locals as a lightweight means to
>> implement a system of permissions and capabilities for things such as
>> this.
>
> Now you have piqued my curiosity, as I have explored a capability based
> model for intercepting `System.exit`. Can you say any more about this yet?

I think all we'd need is a set of capabilities bound to a scope local
at thread startup, and I guess it'd default to "all capabilities".
Trusted code could then override any of those capabilities.

We'd have to make sure that capabilities were inherited by threads,
and we'd have to think very carefully about thread pools. The problem
there is that while it would (I guess) make sense to prevent all code
executing in thread pools from calling System.exit(), there's an
obvious compatibility problem if it can't.

--
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. 
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671

Re: Should System.exit be controlled by a Scope Local?

[Sean Mullan]

On 2/27/22 1:47 PM, Andrew Haley wrote:


I'd like to explore the use of scope locals as a lightweight means to
implement a system of permissions and capabilities for things such as
this.


Now you have piqued my curiosity, as I have explored a capability based 
model for intercepting `System.exit`. Can you say any more about this yet?


Thanks,
Sean

Re: Should System.exit be controlled by a Scope Local?

[Ethan McCue]

Where would be a good place to do that sort of surveying? The mechanism
does not seem to be that popular in open source software ( though that does
make a degree of sense ), or at least the software grep.app scans

https://grep.app/search?q=permission.getName%28%29.startsWith%28%22exitVM%22%29


On Mon, Feb 28, 2022 at 9:05 AM Alan Bateman 
wrote:

> On 26/02/2022 22:14, Ethan McCue wrote:
> > I have a feeling this has been considered and I might just be
> articulating
> > the obvious - but:
> >
> > As called out in JEP 411, one of the remaining legitimate uses of the
> > Security Manager is to intercept calls to System.exit. This seems like a
> > decent use case for the Scope Local mechanism.
> >
> I think it was mostly convenience to use the SM to intercept calls to
> System.exit as it's not really security when all other permissions are
> granted.
>
> There have been a few prototypes of APIs in this area but none made to
> the level of a good proposal. Using a SL or even TL set/remove is
> interesting but you might want to survey some of the existing usages to
> see if they are really stack confined. At least some of the uses have
> been container applications with plugins that accidentally call
> System.exit when running code not intended to run that way. I don't
> think there is any guarantee that they run completely in the same thread
> but some may do.
>
> -Alan
>

Re: RFR: 8282444: Module finder incorrectly assumes default file system path-separator character

[Alan Bateman]

On Mon, 28 Feb 2022 11:12:17 GMT, Chris Hegarty  wrote:

> The module finder implementation incorrectly uses the path-separator
> character from the default file system, when mapping the relative path
> of an entry in an exploded module to a package name. This causes
> problems on Windows [*] when using a module finder with a custom file
> system that has a path-separator character that differs from that of the
> default file system, e.g. the zip file system (which uses '/',
> rather than '\' ).
> 
> Rather than adding a new test for this, I deciced to just modify an
> existing one. The existing test exercises the module finder with a
> custom file system, but does so with modules have trivial single-level
> packages names. I just expanded the module to have a subpackage. This
> is sufficient to reproduce the bug, and verify the fix.
> 
> [*]
> 
> java.lang.module.FindException: Error reading module: /m2
> at 
> java.base/jdk.internal.module.ModulePath.readModule(ModulePath.java:350)
> at 
> java.base/jdk.internal.module.ModulePath.scanDirectory(ModulePath.java:284)
> at java.base/jdk.internal.module.ModulePath.scan(ModulePath.java:232)
> at 
> java.base/jdk.internal.module.ModulePath.scanNextEntry(ModulePath.java:190)
> at 
> java.base/jdk.internal.module.ModulePath.findAll(ModulePath.java:166)
> at 
> ModulesInCustomFileSystem.listAllModules(ModulesInCustomFileSystem.java:108)
> at 
> ModulesInCustomFileSystem.testZipFileSystem(ModulesInCustomFileSystem.java:97)
> at 
> ModulesInCustomFileSystem.testExplodedModulesInZipFileSystem(ModulesInCustomFileSystem.java:68)
> at ...
> Caused by: java.lang.module.InvalidModuleDescriptorException: Package q.r not 
> found in module
> at 
> java.base/jdk.internal.module.ModuleInfo.invalidModuleDescriptor(ModuleInfo.java:1212)
> at 
> java.base/jdk.internal.module.ModuleInfo.doRead(ModuleInfo.java:330)
> at java.base/jdk.internal.module.ModuleInfo.read(ModuleInfo.java:129)
> at 
> java.base/jdk.internal.module.ModulePath.readExplodedModule(ModulePath.java:689)
> at 
> java.base/jdk.internal.module.ModulePath.readModule(ModulePath.java:320)
> ... 36 more

The update to ModulePath looks okay. The tests for ModulePath with paths to 
locate files in custom file systems are somewhat minimal and probably should be 
expanded to ensure that there aren't any other issues. Updating the existing 
ModulesInCustomFileSystem test to use a deeper package structure is subtle but 
okay for now but we should create another issue to create more tests for custom 
file systems. I assume you'll update the date in the copyright header of the 
changed files.

-

PR: https://git.openjdk.java.net/jdk/pull/7632

Re: Should System.exit be controlled by a Scope Local?

[Alan Bateman]

On 26/02/2022 22:14, Ethan McCue wrote:

I have a feeling this has been considered and I might just be articulating
the obvious - but:

As called out in JEP 411, one of the remaining legitimate uses of the
Security Manager is to intercept calls to System.exit. This seems like a
decent use case for the Scope Local mechanism.

I think it was mostly convenience to use the SM to intercept calls to 
System.exit as it's not really security when all other permissions are 
granted.


There have been a few prototypes of APIs in this area but none made to 
the level of a good proposal. Using a SL or even TL set/remove is 
interesting but you might want to survey some of the existing usages to 
see if they are really stack confined. At least some of the uses have 
been container applications with plugins that accidentally call 
System.exit when running code not intended to run that way. I don't 
think there is any guarantee that they run completely in the same thread 
but some may do.


-Alan

Integrated: 8282131: java.time.ZoneId should be a sealed abstract class

[Naoto Sato]

On Fri, 25 Feb 2022 19:02:55 GMT, Naoto Sato  wrote:

> Refactoring `java.time.ZoneId` class to be a sealed class. A CSR has also 
> been drafted.

This pull request has now been integrated.

Changeset: 0ae3d1d5
Author:Naoto Sato 
URL:   
https://git.openjdk.java.net/jdk/commit/0ae3d1d59c44e966e13345b9197fcf067e63900e
Stats: 10 lines in 1 file changed: 0 ins; 4 del; 6 mod

8282131: java.time.ZoneId should be a sealed abstract class

Reviewed-by: iris, rriggs, bpb, lancea, mchung, scolebourne

-

PR: https://git.openjdk.java.net/jdk/pull/7625

Integrated: 8282219: jdk/java/lang/ProcessBuilder/Basic.java fails on AIX

[Ichiroh Takiguchi]

On Tue, 22 Feb 2022 12:17:59 GMT, Ichiroh Takiguchi  
wrote:

> Run jtreg:jdk/java/lang/ProcessBuilder/Basic.java on AIX.
> The test was failed by:
>   Incorrect handling of envstrings containing NULs
> 
> According to my investigation, this issue was happened after following change 
> was applied.
> JDK-8272600: (test) Use native "sleep" in Basic.java
> 
> test.nativepath value was added into AIX's LIBPATH during running this 
> testcase.
> On AIX, test.nativepath value should be removed from LIBPATH value before 
> comparing the values.

This pull request has now been integrated.

Changeset: c5c6058f
Author:Ichiroh Takiguchi 
URL:   
https://git.openjdk.java.net/jdk/commit/c5c6058fd57d4b594012035eaf18a57257f4ad85
Stats: 7 lines in 1 file changed: 5 ins; 0 del; 2 mod

8282219: jdk/java/lang/ProcessBuilder/Basic.java fails on AIX

Reviewed-by: rriggs

-

PR: https://git.openjdk.java.net/jdk/pull/7574

Re: RFR: 8279598: Provide adapter from RandomGenerator to Random [v10]

[Marcono1234]

On Fri, 25 Feb 2022 23:30:35 GMT, Yasser Bazzi  wrote:

>> src/java.base/share/classes/java/util/Random.java line 107:
>> 
>>> 105: return rand;
>>> 106: 
>>> 107: return (Random) new Random.RandomWrapper(random);
>> 
>> Isn't the `(Random)` cast redundant?
>
> Eclipse did not complain about it being redundant, also it does not impact 
> the reading of it and it shows the reader it is a Random instance that is 
> returning

It is unlikely that any IDE or code scanning tool will report _all_ cases of 
incorrect or redundant code. However, in this specific case Eclipse does 
support detecting that (to my knowledge) through its "Clean Up" action (you 
might to explicitly enable though). But you should always verify that the 
actions Eclipse (or any other IDE) performs are correct, because there are some 
reported corner cases where it removes casts which are actually needed.

-

PR: https://git.openjdk.java.net/jdk/pull/7001

RFR: 8282444: Module finder incorrectly assumes default file system path-separator character

[Chris Hegarty]

The module finder implementation incorrectly uses the path-separator
character from the default file system, when mapping the relative path
of an entry in an exploded module to a package name. This causes
problems on Windows [*] when using a module finder with a custom file
system that has a path-separator character that differs from that of the
default file system, e.g. the zip file system (which uses '/',
rather than '\' ).

Rather than adding a new test for this, I deciced to just modify an
existing one. The existing test exercises the module finder with a
custom file system, but does so with modules have trivial single-level
packages names. I just expanded the module to have a subpackage. This
is sufficient to reproduce the bug, and verify the fix.

[*]

java.lang.module.FindException: Error reading module: /m2
at 
java.base/jdk.internal.module.ModulePath.readModule(ModulePath.java:350)
at 
java.base/jdk.internal.module.ModulePath.scanDirectory(ModulePath.java:284)
at java.base/jdk.internal.module.ModulePath.scan(ModulePath.java:232)
at 
java.base/jdk.internal.module.ModulePath.scanNextEntry(ModulePath.java:190)
at java.base/jdk.internal.module.ModulePath.findAll(ModulePath.java:166)
at 
ModulesInCustomFileSystem.listAllModules(ModulesInCustomFileSystem.java:108)
at 
ModulesInCustomFileSystem.testZipFileSystem(ModulesInCustomFileSystem.java:97)
at 
ModulesInCustomFileSystem.testExplodedModulesInZipFileSystem(ModulesInCustomFileSystem.java:68)
at ...
Caused by: java.lang.module.InvalidModuleDescriptorException: Package q.r not 
found in module
at 
java.base/jdk.internal.module.ModuleInfo.invalidModuleDescriptor(ModuleInfo.java:1212)
at java.base/jdk.internal.module.ModuleInfo.doRead(ModuleInfo.java:330)
at java.base/jdk.internal.module.ModuleInfo.read(ModuleInfo.java:129)
at 
java.base/jdk.internal.module.ModulePath.readExplodedModule(ModulePath.java:689)
at 
java.base/jdk.internal.module.ModulePath.readModule(ModulePath.java:320)
... 36 more

-

Commit messages:
 - fix file separator in module finder with custom fs

Changes: https://git.openjdk.java.net/jdk/pull/7632/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7632=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8282444
  Stats: 7 lines in 4 files changed: 1 ins; 0 del; 6 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7632.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7632/head:pull/7632

PR: https://git.openjdk.java.net/jdk/pull/7632