Re: JDK 16 RFR of JDK-8250240: Address use of default constructors in the java.util.concurrent
Hi Martin, Okay; I'll push with "public", but would be fine if the 166-alpha crew want to revisit for "protected". Thanks for the review, -Joe On 7/23/2020 7:49 PM, Martin Buchholz wrote: I'm happy with this change whether or not the slightly more evocative "protected" is used. On Thu, Jul 23, 2020 at 5:43 PM Joe Darcy wrote:
Re: JDK 16 RFR of JDK-8250240: Address use of default constructors in the java.util.concurrent
I'm happy with this change whether or not the slightly more evocative "protected" is used. On Thu, Jul 23, 2020 at 5:43 PM Joe Darcy wrote: > > Hi Martin, > > On 7/23/2020 5:24 PM, Martin Buchholz wrote: > > So these are all abstract classes where the constructor can only be > > called via super() ? > > Yep. > > > In which case one would expect the constructors to be protected, not public. > > But I'm probably missing some reason why "protected" would not be 100% > > compatible. > > It would be compatible (AFAICT), but the current (implicit) default > constructors are public, since the classes are public, so I made the new > explicit constructors public. > > On the assumption the upstream JDK 166-alpha repo would want to take in > this change for as many releases as possible, the public constructors > could be used for earlier release trains too. > > > Historically, we've preferred to put changes in via CVS, but in 2020, > > we might prefer you make the change directly in openjdk. > > Doug? > > > I'm happy to make the changes in OpenJDK for JDK 16. > > Thanks, > > -Joe >
Re: JDK 16 RFR of JDK-8250240: Address use of default constructors in the java.util.concurrent
Hi Martin, On 7/23/2020 5:24 PM, Martin Buchholz wrote: So these are all abstract classes where the constructor can only be called via super() ? Yep. In which case one would expect the constructors to be protected, not public. But I'm probably missing some reason why "protected" would not be 100% compatible. It would be compatible (AFAICT), but the current (implicit) default constructors are public, since the classes are public, so I made the new explicit constructors public. On the assumption the upstream JDK 166-alpha repo would want to take in this change for as many releases as possible, the public constructors could be used for earlier release trains too. Historically, we've preferred to put changes in via CVS, but in 2020, we might prefer you make the change directly in openjdk. Doug? I'm happy to make the changes in OpenJDK for JDK 16. Thanks, -Joe
Re: JDK 16 RFR of JDK-8250240: Address use of default constructors in the java.util.concurrent
So these are all abstract classes where the constructor can only be
called via super() ?
In which case one would expect the constructors to be protected, not public.
But I'm probably missing some reason why "protected" would not be 100%
compatible.
Historically, we've preferred to put changes in via CVS, but in 2020,
we might prefer you make the change directly in openjdk.
Doug?
On Thu, Jul 23, 2020 at 3:04 PM Joe Darcy wrote:
>
> Hello,
>
> Martin, how would you prefer these changes, or equivalent ones, to get
> into the java.util.concurrent upstream sources?
>
> Several classes in java.util.concurrent rely on default constructors,
> which is not recommended. Please review the patch below which removes
> them along with the corresponding CSR
> (https://bugs.openjdk.java.net/browse/JDK-8250241); webrev at
> http://cr.openjdk.java.net/~darcy/8250240.0/.
>
> Thanks,
>
> -Joe
>
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/AbstractExecutorService.java
> ---
> a/src/java.base/share/classes/java/util/concurrent/AbstractExecutorService.java
> Thu Jul 23 20:25:41 2020 +0100
> +++
> b/src/java.base/share/classes/java/util/concurrent/AbstractExecutorService.java
> Thu Jul 23 15:01:27 2020 -0700
> @@ -77,6 +77,11 @@
> public abstract class AbstractExecutorService implements ExecutorService {
>
> /**
> + * Constructor for subclasses to call.
> + */
> +public AbstractExecutorService() {}
> +
> +/**
>* Returns a {@code RunnableFuture} for the given runnable and default
>* value.
>*
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/ForkJoinTask.java
> --- a/src/java.base/share/classes/java/util/concurrent/ForkJoinTask.java
> Thu Jul 23 20:25:41 2020 +0100
> +++ b/src/java.base/share/classes/java/util/concurrent/ForkJoinTask.java
> Thu Jul 23 15:01:27 2020 -0700
> @@ -242,6 +242,11 @@
> private static final int SIGNAL = 1 << 16; // true if joiner waiting
> private static final int SMASK= 0x; // short bits for tags
>
> +/**
> + * Constructor for subclasses to call.
> + */
> +public ForkJoinTask() {}
> +
> static boolean isExceptionalStatus(int s) { // needed by subclasses
> return (s & THROWN) != 0;
> }
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/RecursiveAction.java
> ---
> a/src/java.base/share/classes/java/util/concurrent/RecursiveAction.java
> Thu Jul 23 20:25:41 2020 +0100
> +++
> b/src/java.base/share/classes/java/util/concurrent/RecursiveAction.java
> Thu Jul 23 15:01:27 2020 -0700
> @@ -166,6 +166,11 @@
> private static final long serialVersionUID = 5232453952276485070L;
>
> /**
> + * Constructor for subclasses to call.
> + */
> +public RecursiveAction() {}
> +
> +/**
>* The main computation performed by this task.
>*/
> protected abstract void compute();
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/RecursiveTask.java
> ---
> a/src/java.base/share/classes/java/util/concurrent/RecursiveTask.java
> Thu Jul 23 20:25:41 2020 +0100
> +++
> b/src/java.base/share/classes/java/util/concurrent/RecursiveTask.java
> Thu Jul 23 15:01:27 2020 -0700
> @@ -69,6 +69,11 @@
> private static final long serialVersionUID = 5232453952276485270L;
>
> /**
> + * Constructor for subclasses to call.
> + */
> +public RecursiveTask() {}
> +
> +/**
>* The result of the computation.
>*/
> @SuppressWarnings("serial") // Conditionally serializable
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
> ---
> a/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
> Thu Jul 23 20:25:41 2020 +0100
> +++
> b/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
> Thu Jul 23 15:01:27 2020 -0700
> @@ -65,6 +65,11 @@
>
> private static final long serialVersionUID = 7373984972572414692L;
>
> +/**
> + * Constructor for subclasses to call.
> + */
> +public AbstractQueuedLongSynchronizer() {}
> +
> /*
>* To keep sources in sync, the remainder of this source file is
>* exactly cloned from AbstractQueuedSynchronizer, replacing class
>
Re: JDK 16 RFR of JDK-8250240: Address use of default constructors in the java.util.concurrent
+1
--
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
Sent from my iPhone
> On Jul 23, 2020, at 6:04 PM, Joe Darcy wrote:
>
> Hello,
>
> Martin, how would you prefer these changes, or equivalent ones, to get into
> the java.util.concurrent upstream sources?
>
> Several classes in java.util.concurrent rely on default constructors, which
> is not recommended. Please review the patch below which removes them along
> with the corresponding CSR
> (https://bugs.openjdk.java.net/browse/JDK-8250241); webrev at
> http://cr.openjdk.java.net/~darcy/8250240.0/.
>
> Thanks,
>
> -Joe
>
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/AbstractExecutorService.java
> ---
> a/src/java.base/share/classes/java/util/concurrent/AbstractExecutorService.java
> Thu Jul 23 20:25:41 2020 +0100
> +++
> b/src/java.base/share/classes/java/util/concurrent/AbstractExecutorService.java
> Thu Jul 23 15:01:27 2020 -0700
> @@ -77,6 +77,11 @@
> public abstract class AbstractExecutorService implements ExecutorService {
>
> /**
> + * Constructor for subclasses to call.
> + */
> +public AbstractExecutorService() {}
> +
> +/**
> * Returns a {@code RunnableFuture} for the given runnable and default
> * value.
> *
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/ForkJoinTask.java
> --- a/src/java.base/share/classes/java/util/concurrent/ForkJoinTask.java Thu
> Jul 23 20:25:41 2020 +0100
> +++ b/src/java.base/share/classes/java/util/concurrent/ForkJoinTask.java Thu
> Jul 23 15:01:27 2020 -0700
> @@ -242,6 +242,11 @@
> private static final int SIGNAL = 1 << 16; // true if joiner waiting
> private static final int SMASK= 0x; // short bits for tags
>
> +/**
> + * Constructor for subclasses to call.
> + */
> +public ForkJoinTask() {}
> +
> static boolean isExceptionalStatus(int s) { // needed by subclasses
> return (s & THROWN) != 0;
> }
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/RecursiveAction.java
> --- a/src/java.base/share/classes/java/util/concurrent/RecursiveAction.java
> Thu Jul 23 20:25:41 2020 +0100
> +++ b/src/java.base/share/classes/java/util/concurrent/RecursiveAction.java
> Thu Jul 23 15:01:27 2020 -0700
> @@ -166,6 +166,11 @@
> private static final long serialVersionUID = 5232453952276485070L;
>
> /**
> + * Constructor for subclasses to call.
> + */
> +public RecursiveAction() {}
> +
> +/**
> * The main computation performed by this task.
> */
> protected abstract void compute();
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/RecursiveTask.java
> --- a/src/java.base/share/classes/java/util/concurrent/RecursiveTask.java Thu
> Jul 23 20:25:41 2020 +0100
> +++ b/src/java.base/share/classes/java/util/concurrent/RecursiveTask.java Thu
> Jul 23 15:01:27 2020 -0700
> @@ -69,6 +69,11 @@
> private static final long serialVersionUID = 5232453952276485270L;
>
> /**
> + * Constructor for subclasses to call.
> + */
> +public RecursiveTask() {}
> +
> +/**
> * The result of the computation.
> */
> @SuppressWarnings("serial") // Conditionally serializable
> diff -r d62da6fc4074
> src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
> ---
> a/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
> Thu Jul 23 20:25:41 2020 +0100
> +++
> b/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
> Thu Jul 23 15:01:27 2020 -0700
> @@ -65,6 +65,11 @@
>
> private static final long serialVersionUID = 7373984972572414692L;
>
> +/**
> + * Constructor for subclasses to call.
> + */
> +public AbstractQueuedLongSynchronizer() {}
> +
> /*
> * To keep sources in sync, the remainder of this source file is
> * exactly cloned from AbstractQueuedSynchronizer, replacing class
>
JDK 16 RFR of JDK-8250240: Address use of default constructors in the java.util.concurrent
Hello,
Martin, how would you prefer these changes, or equivalent ones, to get
into the java.util.concurrent upstream sources?
Several classes in java.util.concurrent rely on default constructors,
which is not recommended. Please review the patch below which removes
them along with the corresponding CSR
(https://bugs.openjdk.java.net/browse/JDK-8250241); webrev at
http://cr.openjdk.java.net/~darcy/8250240.0/.
Thanks,
-Joe
diff -r d62da6fc4074
src/java.base/share/classes/java/util/concurrent/AbstractExecutorService.java
---
a/src/java.base/share/classes/java/util/concurrent/AbstractExecutorService.java
Thu Jul 23 20:25:41 2020 +0100
+++
b/src/java.base/share/classes/java/util/concurrent/AbstractExecutorService.java
Thu Jul 23 15:01:27 2020 -0700
@@ -77,6 +77,11 @@
public abstract class AbstractExecutorService implements ExecutorService {
/**
+ * Constructor for subclasses to call.
+ */
+ public AbstractExecutorService() {}
+
+ /**
* Returns a {@code RunnableFuture} for the given runnable and default
* value.
*
diff -r d62da6fc4074
src/java.base/share/classes/java/util/concurrent/ForkJoinTask.java
--- a/src/java.base/share/classes/java/util/concurrent/ForkJoinTask.java
Thu Jul 23 20:25:41 2020 +0100
+++ b/src/java.base/share/classes/java/util/concurrent/ForkJoinTask.java
Thu Jul 23 15:01:27 2020 -0700
@@ -242,6 +242,11 @@
private static final int SIGNAL = 1 << 16; // true if joiner waiting
private static final int SMASK = 0x; // short bits for tags
+ /**
+ * Constructor for subclasses to call.
+ */
+ public ForkJoinTask() {}
+
static boolean isExceptionalStatus(int s) { // needed by subclasses
return (s & THROWN) != 0;
}
diff -r d62da6fc4074
src/java.base/share/classes/java/util/concurrent/RecursiveAction.java
---
a/src/java.base/share/classes/java/util/concurrent/RecursiveAction.java
Thu Jul 23 20:25:41 2020 +0100
+++
b/src/java.base/share/classes/java/util/concurrent/RecursiveAction.java
Thu Jul 23 15:01:27 2020 -0700
@@ -166,6 +166,11 @@
private static final long serialVersionUID = 5232453952276485070L;
/**
+ * Constructor for subclasses to call.
+ */
+ public RecursiveAction() {}
+
+ /**
* The main computation performed by this task.
*/
protected abstract void compute();
diff -r d62da6fc4074
src/java.base/share/classes/java/util/concurrent/RecursiveTask.java
---
a/src/java.base/share/classes/java/util/concurrent/RecursiveTask.java
Thu Jul 23 20:25:41 2020 +0100
+++
b/src/java.base/share/classes/java/util/concurrent/RecursiveTask.java
Thu Jul 23 15:01:27 2020 -0700
@@ -69,6 +69,11 @@
private static final long serialVersionUID = 5232453952276485270L;
/**
+ * Constructor for subclasses to call.
+ */
+ public RecursiveTask() {}
+
+ /**
* The result of the computation.
*/
@SuppressWarnings("serial") // Conditionally serializable
diff -r d62da6fc4074
src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
---
a/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
Thu Jul 23 20:25:41 2020 +0100
+++
b/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
Thu Jul 23 15:01:27 2020 -0700
@@ -65,6 +65,11 @@
private static final long serialVersionUID = 7373984972572414692L;
+ /**
+ * Constructor for subclasses to call.
+ */
+ public AbstractQueuedLongSynchronizer() {}
+
/*
* To keep sources in sync, the remainder of this source file is
* exactly cloned from AbstractQueuedSynchronizer, replacing class
