Review Request: JDK-8160698 java --dry-run should not cause main class be initialized

[Mandy Chung]

Kumar,

I move the call to CreateApplicationArgs before PostJVMInit and dryRun stops 
before PostJVMInit which shows the splash screen.

Webrev:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8160698/webrev.00/index.html

This also cleans up the LauncherHelper to use Class::forName instead of 
loadClass.

Mandy

os.name will be "macOS"? (was Re: RFR 9 : 8160370 : System.getProperty("os.version") returns "Unknown" on Mac)

[Wang Weijun]

I have an off-topic question:

Will os.name be macOS for 10.12? I have several places checking "if 
(!osname.contains("OS X"))", is there a helper method I can check for this in 
the future no matter if it's running on pre- or post-10.12?

Thanks
Max

> On Jul 1, 2016, at 2:23 AM, Brent Christian  
> wrote:
> 
> On 6/30/16 9:53 AM, Brent Christian wrote:
>> 
>> When the minimum Mac build platform is SDK 10.10, we'll be able to call
>> operatingSystemVersion directly without using msgSend.  We can also
>> consider removing this then.
> 
> FYI:
> https://bugs.openjdk.java.net/browse/JDK-8160676
> 
> -Brent

RFR: JDK-8160240 - javax/rmi/PortableRemoteObject/8146975/RmiIiopReturnValueTest.java failed with error "Address already in use: bind"

[Mark Sheppard]

Hi,
  please oblige and review the following change
http://cr.openjdk.java.net/~msheppar/8160240/webrev/

to address the issue raised in
https://bugs.openjdk.java.net/browse/JDK-8160240

it has been observed that, during continuous integration regression 
tests on some platforms,
there is an intermittent bind failure, when starting the orbd for the 
test. Thus, as the test is composed of
two run commands, one without security manager and one with security 
manager, it is
assumed that, the second run starts before the sockets in use in the 
first run have been fully released.
Therefore, to overcome the bind already in use port conflict, the test's 
second run with security manager
has been modified to use different ports, for cos nameservice and 
activator, to those of the first run.


regards
Mark

Re: RFR: jsr166 jdk9 integration wave 7

[Doug Lea]

On 06/30/2016 10:08 AM, Paul Sandoz wrote:

Hi Peter,

Thanks, that’s interesting. I am uncertain if our 166 fellows will be
reluctant or not to pull in a dependency on jdk.internal.misc.SharedSecrets.



Background: we are reluctant to rely on anything that makes sources impossible
to use in (usually, upcoming versions of) Android. Which probably
doesn't directly apply here.

But my main concern in this case is that we need better assurance
that there are no possible start-up circularities, since we've already
had some near-miss experiences with ThreadLocalRandom. Is there a
solid argument?

-Doug

JEP 290: Filter Incoming Serialization Data

[mark . reinhold]

New JEP Candidate: http://openjdk.java.net/jeps/290

- Mark

Re: RFR 9 : 8160370 : System.getProperty("os.version") returns "Unknown" on Mac

[Brent Christian]

On 6/30/16 9:53 AM, Brent Christian wrote:


When the minimum Mac build platform is SDK 10.10, we'll be able to call
operatingSystemVersion directly without using msgSend.  We can also
consider removing this then.


FYI:
https://bugs.openjdk.java.net/browse/JDK-8160676

-Brent

Re: RFR 8159245: Loggers created by system classes are not initialized correctly when configured programmatically from application code.

[Mandy Chung]

> On Jun 30, 2016, at 9:33 AM, Daniel Fuchs  wrote:
> 
> Indeed, good catch! I should have seen that :-(
> 
> Here is a patch that should take care of the issue:
> 
> http://cr.openjdk.java.net/~dfuchs/webrev_8159245/webrev.05
> 
> Thanks for your offline suggestions on how to deal with that
> scenario.
> 

Looks pretty good. What if the application logger is a custom Logger subclass?  
Would this mechanism support that?

One small comment:

 577 if (logger != sysLogger && !logger.isConfigMerged(sysLogger)) {
 578 // if logger already exists we will merge the two 
configurations.
 :
 587 }

I suggest to hide the body and isConfigMerged check in Logger in a single 
method:
   if (logger != sysLogger) {
   logger.mergeWithSystemLogger(sysLogger);
   }

Mandy

Re: JDK 9 RFR of JDK-6226715: (ann) java.lang.annotation.AnnotationTypeMismatchException could not be serialized

[Roger Riggs]

Joe,

Caveats and all, that seems like a good solution.
+1

Roger


On 6/30/2016 1:18 PM, joe darcy wrote:

Hello,

Please review the changes to address

JDK-6226715: (ann) 
java.lang.annotation.AnnotationTypeMismatchException could not be 
serialized

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

Patch below.

The analysis of why is patch is valid requires a bit of explanation.

Like all other exceptions, AnnotationTypeMismatchException is 
serializable. However, its state contains a non-serializable element 
field of type Method. Therefore, previously an 
AnnotationTypeMismatchException with a non-null element field threw an 
exception when serialized. Therefore, the only persisted 
AnnotationTypeMismatchException objects have null element fields in 
the serial output.


The patch changes element to be a transient field, removing it from 
the serial output, *without* changing the serialVersionUID. This would 
not usually be a valid transformation, but limits on what can be 
serialized make it acceptable in this case.


If an old serialized form of an AnnotationTypeMismatchException object 
with a null value for element is deserialized after the patch, the now 
extraneous element information is ignored, yielding a semantically 
correct object (with a null value for element).


If a new serialized form is deserialized against the previous version 
of AnnotationTypeMismatchException, the information for the element 
field is missing, but it gets defaulted to be null, again giving the 
correct semantics.


I've verified this cross-version behavior.

While subtle, I think this change is preferable to introducing 
readObject/writeObject methods to do something like explicit write out 
a null value for the element field in the serial form even if element 
is non-null, etc.


The specification updates make the possibility of nulls explicit.

Thanks,

-Joe


--- 
old/src/java.base/share/classes/java/lang/annotation/AnnotationTypeMismatchException.java 
2016-06-30 09:53:08.335033457 -0700
+++ 
new/src/java.base/share/classes/java/lang/annotation/AnnotationTypeMismatchException.java 
2016-06-30 09:53:08.223033453 -0700

@@ -44,7 +44,7 @@
 /**
  * The {@code Method} object for the annotation element.
  */
-private final Method element;
+private final transient Method element;

 /**
  * The (erroneous) type of data found in the annotation. This string
@@ -57,10 +57,12 @@
  * Constructs an AnnotationTypeMismatchException for the specified
  * annotation type element and found data type.
  *
- * @param element the {@code Method} object for the annotation 
element

+ * @param element the {@code Method} object for the annotation
+ * element, may be {@code null}
  * @param foundType the (erroneous) type of data found in the 
annotation.

  *This string may, but is not required to, contain the value
- *as well.  The exact format of the string is unspecified.
+ *as well.  The exact format of the string is unspecified,
+ *may be {@code null}.
  */
 public AnnotationTypeMismatchException(Method element, String 
foundType) {
 super("Incorrectly typed data found for annotation element " 
+ element

@@ -71,8 +73,11 @@

 /**
  * Returns the {@code Method} object for the incorrectly typed 
element.

+ * The value may be unavailable if this exception has been
+ * serialized and then read back in.
  *
- * @return the {@code Method} object for the incorrectly typed 
element

+ * @return the {@code Method} object for the incorrectly typed
+ * element, or {@code null} if unavailable
  */
 public Method element() {
 return this.element;
@@ -81,7 +86,8 @@
 /**
  * Returns the type of data found in the incorrectly typed element.
  * The returned string may, but is not required to, contain the 
value

- * as well.  The exact format of the string is unspecified.
+ * as well.  The exact format of the string is unspecified and 
the string

+ * may be {@code null}.
  *
  * @return the type of data found in the incorrectly typed element
  */

JDK 9 RFR of JDK-6226715: (ann) java.lang.annotation.AnnotationTypeMismatchException could not be serialized

[joe darcy]

Hello,

Please review the changes to address

JDK-6226715: (ann) 
java.lang.annotation.AnnotationTypeMismatchException could not be serialized

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

Patch below.

The analysis of why is patch is valid requires a bit of explanation.

Like all other exceptions, AnnotationTypeMismatchException is 
serializable. However, its state contains a non-serializable element 
field of type Method. Therefore, previously an 
AnnotationTypeMismatchException with a non-null element field threw an 
exception when serialized. Therefore, the only persisted 
AnnotationTypeMismatchException objects have null element fields in the 
serial output.


The patch changes element to be a transient field, removing it from the 
serial output, *without* changing the serialVersionUID. This would not 
usually be a valid transformation, but limits on what can be serialized 
make it acceptable in this case.


If an old serialized form of an AnnotationTypeMismatchException object 
with a null value for element is deserialized after the patch, the now 
extraneous element information is ignored, yielding a semantically 
correct object (with a null value for element).


If a new serialized form is deserialized against the previous version of 
AnnotationTypeMismatchException, the information for the element field 
is missing, but it gets defaulted to be null, again giving the correct 
semantics.


I've verified this cross-version behavior.

While subtle, I think this change is preferable to introducing 
readObject/writeObject methods to do something like explicit write out a 
null value for the element field in the serial form even if element is 
non-null, etc.


The specification updates make the possibility of nulls explicit.

Thanks,

-Joe


--- 
old/src/java.base/share/classes/java/lang/annotation/AnnotationTypeMismatchException.java 
2016-06-30 09:53:08.335033457 -0700
+++ 
new/src/java.base/share/classes/java/lang/annotation/AnnotationTypeMismatchException.java 
2016-06-30 09:53:08.223033453 -0700

@@ -44,7 +44,7 @@
 /**
  * The {@code Method} object for the annotation element.
  */
-private final Method element;
+private final transient Method element;

 /**
  * The (erroneous) type of data found in the annotation. This string
@@ -57,10 +57,12 @@
  * Constructs an AnnotationTypeMismatchException for the specified
  * annotation type element and found data type.
  *
- * @param element the {@code Method} object for the annotation element
+ * @param element the {@code Method} object for the annotation
+ * element, may be {@code null}
  * @param foundType the (erroneous) type of data found in the 
annotation.

  *This string may, but is not required to, contain the value
- *as well.  The exact format of the string is unspecified.
+ *as well.  The exact format of the string is unspecified,
+ *may be {@code null}.
  */
 public AnnotationTypeMismatchException(Method element, String 
foundType) {
 super("Incorrectly typed data found for annotation element " + 
element

@@ -71,8 +73,11 @@

 /**
  * Returns the {@code Method} object for the incorrectly typed 
element.

+ * The value may be unavailable if this exception has been
+ * serialized and then read back in.
  *
- * @return the {@code Method} object for the incorrectly typed element
+ * @return the {@code Method} object for the incorrectly typed
+ * element, or {@code null} if unavailable
  */
 public Method element() {
 return this.element;
@@ -81,7 +86,8 @@
 /**
  * Returns the type of data found in the incorrectly typed element.
  * The returned string may, but is not required to, contain the value
- * as well.  The exact format of the string is unspecified.
+ * as well.  The exact format of the string is unspecified and the 
string

+ * may be {@code null}.
  *
  * @return the type of data found in the incorrectly typed element
  */

Re: RFR 9 : 8160370 : System.getProperty("os.version") returns "Unknown" on Mac

[Mandy Chung]

> On Jun 30, 2016, at 7:33 AM, Gerard Ziemski  wrote:
> 
> 
>> On Jun 29, 2016, at 6:47 PM, Mandy Chung  wrote:
>> 
>> I think we should move away from testing on unsupported platforms.  Is this 
>> just temporary?  when will this be removed?
> 
> I think we should leave the workaround code in forever - it will never 
> execute on newer/supported OS X and will do the right thing on earlier ones.

I disagree to leave dead code in our source forever (imagine the amount of code 
that we removed in the past were left in the source base).

It may be fine to add this temporarily until our infrastructure completes 
migrating to supported platforms.  We should track this and remove it in the 
future.

Mandy

Re: RFR 9 : 8160370 : System.getProperty("os.version") returns "Unknown" on Mac

[Brent Christian]

On 6/29/16 4:47 PM, Mandy Chung wrote:

On Jun 29, 2016, at 12:36 PM, Brent Christian  
wrote:

The code to restore behavior on older Mac systems is only a few
lines, so that seems like a good way to get testing going again.


I think we should move away from testing on unsupported platforms.
Is this just temporary?  when will this be removed?


When the minimum Mac build platform is SDK 10.10, we'll be able to call 
operatingSystemVersion directly without using msgSend.  We can also 
consider removing this then.


Thanks,
-Brent

Re: RFR 9 : 8160370 : System.getProperty("os.version") returns "Unknown" on Mac

[Brent Christian]

Thanks, Brian

On 6/29/16 4:35 PM, Brian Burkhalter wrote:

Approved.

Brian

On Jun 29, 2016, at 4:33 PM, Brent Christian > wrote:


Thank you, Dave and Gerard.
I believe I still need to hear from a JDK9 Reviewer.

Re: RFR 8054213: Class name repeated in output of Type.toString()

[Svetlana Nikandrova]

Hello Joe,

thank you for your advice! GenericStringTest looks really good with 
annotations. I refactored my test, please see updated webrev:
http://cr.openjdk.java.net/~snikandrova/8054213/webrev.01/ 



As for "." vs "$" let me provide an example:
E.g. we have class

public classTest {

publicNested1.Nested2 foo()
{
return null;
}

public classNested1
{
public classNested2{}
}
}

The output of the

System.out.println(Test.class.getMethod("foo",null).getReturnType());
Output:
class Test$Nested1$Nested2 (nested classes divided by "$")

while

System.out.println(Test.class.getMethod("foo",null).getGenericReturnType());
Output:
Test$Nested1.Nested2(nested classes divided by "." if enclosed by 
parametrized type and "$" in other cases).

I think it's a little bit strange to have different separators for inner 
classes depended on is it nested by parametrized or raw type.


Thank you,
Svetlana

On 28.06.2016 22:02, joe darcy wrote:

Hello Svetlana,

I'm not convinced '$' should be replaced with '.' in this context as 
'.' is what the separator used in source code.


In any case, the test should be restructured. I recommend declaring an 
annotation type to hold the expected to string output. You can them 
loop over the methods from the class object and for the methods with 
the annotation verifying the toString output is as expected. For a 
guide see


test/java/lang/reflect/Constructor/GenericStringTest.java

Thanks,

-Joe


On 6/28/2016 11:25 AM, Svetlana Nikandrova wrote:

May be someone can find a minute?

On 27.06.2016 21:25, Svetlana Nikandrova wrote:

Kindly reminder

On 24.06.2016 14:58, Svetlana Nikandrova wrote:

Hello,

let me try again with updated version of webrev:
Webrev:
http://cr.openjdk.java.net/~snikandrova/8054213/webrev.01/ 


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

Many thanks goes to Sergey Ustimenko for his valuable advises. I 
believe fix looks nicer now.

JPRT tested.

Thank you,
Svetlana

On 17.06.2016 21:45, Svetlana Nikandrova wrote:

Hello,

could you please review this fix for toString() method of 
ParameterizedTypeImpl.
The problem is that when we obtain simple name of nested type 
shared prefix is only removed for ParameterizedType owner. We need 
to remove it for other cases too.


Please note that I also changed delimiter between outer and inner 
class from "." to "$". I believe as "$" is usually used to 
separate nested class name "." looks inconsistent and confusing.

Let me know if you have any objections.

Webrev:
http://cr.openjdk.java.net/~msolovie/8054213/webrev.00/ 


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

Thank you,
Svetlana

Re: RFR 9 : 8160370 : System.getProperty("os.version") returns "Unknown" on Mac

[Brent Christian]

Will do - thanks, Roger.

-Brent

On 6/30/16 6:46 AM, Roger Riggs wrote:

+1

Can you wrap a couple of very long lines to make the diff easier to read.
- 187
- 202

Thanks, Roger


On 6/29/2016 7:47 PM, Mandy Chung wrote:

On Jun 29, 2016, at 12:36 PM, Brent Christian
 wrote:

Hi,

Please review the following change for JDK 9:

Bug:
https://bugs.openjdk.java.net/browse/JDK-8160370
Webrev:
http://cr.openjdk.java.net/~bchristi/8160370/webrev.00/

The fix for 7131356 fills in the "os.version" system property on Mac
using [NSProcessInfo operatingSystemVersion], available starting in
Mac OS 10.9.

While JDK 9 will not support versions of Mac OS prior to 10.9 [1],
not all testing infrastructure has been updated, and we've seen
failures of

java/lang/System/OsVersionTest.java

The code to restore behavior on older Mac systems is only a few
lines, so that seems like a good way to get testing going again.

I think we should move away from testing on unsupported platforms.  Is
this just temporary?  when will this be removed?

Mandy

Re: RFR 8159245: Loggers created by system classes are not initialized correctly when configured programmatically from application code.

[Daniel Fuchs]

Hi Mandy,

On 21/06/16 17:01, Mandy Chung wrote:



On Jun 21, 2016, at 8:39 AM, Daniel Fuchs  wrote:

I don't understand this scenario.
ConfigurationData should remain as simple as possible.
Logger.getLogger() / LogManager.demandSystemLogger() will never return
a logger before it has been configured.
When we're merging the configuration data the system logger has
not been configured yet. Level etc are already volatile in Logger and
we should not introduce any new synchronization there.


This is the scenario I was thinking about - would this ever happen?

T1 is creating a system logger named “foo” and calling this.importConfg(other) 
at line 462 after setLevel(otherLevel) is done.

this is the system logger “foo” and other is the user-created logger “foo”.

T2 is calling other.setLevel(newLevel) on user-created logger “foo”.


Indeed, good catch! I should have seen that :-(

Here is a patch that should take care of the issue:

http://cr.openjdk.java.net/~dfuchs/webrev_8159245/webrev.05

Thanks for your offline suggestions on how to deal with that
scenario.

best regards,

-- daniel



Mandy

Re: [jdk9] RFR: 8159822: Non-synchronized access to shared members of com.sun.jndi.ldap.pool.Pool

[Ivan Gerasimov]

Thanks Seán!

I added an example of stacktrace to the description.

With kind regards,
Ivan


On 30.06.2016 17:31, Seán Coffey wrote:

Looks fine to me Ivan.

If you have a stacktrace of an example ConcurrentModificationException 
issue, please paste it into the bug report so that it's documented for 
others to find.


Regards,
Sean.

On 24/06/2016 15:56, Ivan Gerasimov wrote:

Hi Aleksey!

I've double-checked the Pool class and found no other code that has 
to be additionally synchronized.
Please note that the method expungeStaleConnections() is thread-safe 
and can be called from outside the synchronized blocks.
The method Pool.getConnections(Object) accesses the `map` field, but 
it is only called from synchronized blocks, so it's fine.


Please let me know, if I'm missing something obvious.

With kind regards,
Ivan


On 21.06.2016 19:25, Ivan Gerasimov wrote:



On 21.06.2016 18:43, Aleksey Shipilev wrote:

On 06/21/2016 06:14 PM, Ivan Gerasimov wrote:

Hello!

The Pool has a member `map`, which is accessed from different 
threads,

thus the access is synchronized.
However, in some code paths (mostly in debug printing) it is accessed
without proper synchronization, which results in intermediate
ConcurrentModificationException when the debug output is turned on.

Would you please help review the fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8159822
WEBREV: http://cr.openjdk.java.net/~igerasim/8159822/00/webrev/

Missed synchronized-s:

  154 private Connections getConnections(Object id) {
  155 ConnectionsRef ref = map.get(id);
  156 return (ref != null) ? ref.getConnections() : null;
  157 }

...gets called via:

  168 public void expire(long threshold) {
  169 synchronized (map) {
   ...
  179 }
  180 expungeStaleConnections();
  181 }

...and also via:

  188 private static void expungeStaleConnections() {
...
  192 Connections conns = releaseRef.getConnections();
...
  203  }
  204 }
But this is ConnectionsWeakRef.getConnections() not 
Pool.getConnections(Object).
As far as I can see, an instance of ConnectionsWeakRef cannot be 
accessed concurrently.




...

  117 public PooledConnection getPooledConnection(Object id, long
timeout,
  118 PooledConnectionFactory factory) throws 
NamingException {

...
 // no synchronized prior here
  127 expungeStaleConnections();
...

expungeStaleConnections() should be safe to call concurrently as 
long as ReferenceQueue.poll() is thread-safe.


With kind regards,
Ivan

Re: Semantics of VarHandle CAS methods

[Martin Buchholz]

On Thu, Jun 30, 2016 at 4:19 AM, Doug Lea  wrote:

> On 06/30/2016 06:38 AM, Martin Buchholz wrote:
>
>> It's not only about naming.
>>
>> So yes, I'd like the name weakCompareAndSet to be the sequentially
>> consistent version, BUT I'd also expect the next more relaxed version to
>> be
>> memory_order_acq_rel which we don't provide.
>>
>
> There are a few flavors of C++ CAS/weakCAS that aren't explicitly supported
> (also including those with different memory orders on success and failure),
> because you can get the effects with combinations of the supplied versions
> and
> explicit fences. Supporting all of them would have added lots of
> seldom-used
> methods.
>
>
One concrete proposal that removes a method:

Replace
weakCompareAndSetAcquire and weakCompareAndSetRelease with
weakCompareAndSetAcquireRelease


> I don't have a good intuition about how useful non-sequentially-consistent
>> CASes are.
>>
>
> Among other uses, fallible performance counters, and the bases for
> combining with fences as above.


We can keep the fully relaxed flavor for use with fences, so you can still
get the effect of  weakCompareAndSetAcquire if you really want.

In C++ we have sequential consistency of the single memory location holding
an atomic (cache coherence).  Should we say something about locations
updated via a VarHandle?  If you call weakCompareAndSetAcquire, then the
spec says the write is a "plain write", but some kind of cache coherence
seems inherent in the idea of a compareAndSet, so the write itself has to
be complete and visible.

Re: RFR: jsr166 jdk9 integration wave 7

[Daniel Fuchs]

On 30/06/16 01:04, Doug Lea wrote:

On Wed, Jun 29, 2016 at 8:38 AM, Daniel Fuchs > wrote:



I mean something like: "The maximum number of spare threads used by the
common pool is 256: to arrange the same value as is used by default
for the
common pool, use {@code 256} plus the parallelism level for {@code
maximumPoolSize}."



Thanks! I added a variant of this sentence.


On 30/06/16 12:20, Martin Buchholz wrote:

Webrev regenerated with updates.
Lots of rework for the atomic VarHandle specs.
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/jsr166-jdk9-integration/



Thanks Doug, Martin,

I find the new text is indeed more clear :-)

best regards,

-- daniel

Re: RFR 9 : 8160370 : System.getProperty("os.version") returns "Unknown" on Mac

[Gerard Ziemski]

> On Jun 29, 2016, at 6:47 PM, Mandy Chung  wrote:
> 
>> 
>> On Jun 29, 2016, at 12:36 PM, Brent Christian  
>> wrote:
>> 
>> Hi,
>> 
>> Please review the following change for JDK 9:
>> 
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8160370
>> Webrev:
>> http://cr.openjdk.java.net/~bchristi/8160370/webrev.00/
>> 
>> The fix for 7131356 fills in the "os.version" system property on Mac using 
>> [NSProcessInfo operatingSystemVersion], available starting in Mac OS 10.9.
>> 
>> While JDK 9 will not support versions of Mac OS prior to 10.9 [1], not all 
>> testing infrastructure has been updated, and we've seen failures of
>> 
>> java/lang/System/OsVersionTest.java
>> 
>> The code to restore behavior on older Mac systems is only a few lines, so 
>> that seems like a good way to get testing going again.
> 
> I think we should move away from testing on unsupported platforms.  Is this 
> just temporary?  when will this be removed?

I think we should leave the workaround code in forever - it will never execute 
on newer/supported OS X and will do the right thing on earlier ones.


cheers

Re: [jdk9] RFR: 8159822: Non-synchronized access to shared members of com.sun.jndi.ldap.pool.Pool

[Seán Coffey]

Looks fine to me Ivan.

If you have a stacktrace of an example ConcurrentModificationException 
issue, please paste it into the bug report so that it's documented for 
others to find.


Regards,
Sean.

On 24/06/2016 15:56, Ivan Gerasimov wrote:

Hi Aleksey!

I've double-checked the Pool class and found no other code that has to 
be additionally synchronized.
Please note that the method expungeStaleConnections() is thread-safe 
and can be called from outside the synchronized blocks.
The method Pool.getConnections(Object) accesses the `map` field, but 
it is only called from synchronized blocks, so it's fine.


Please let me know, if I'm missing something obvious.

With kind regards,
Ivan


On 21.06.2016 19:25, Ivan Gerasimov wrote:



On 21.06.2016 18:43, Aleksey Shipilev wrote:

On 06/21/2016 06:14 PM, Ivan Gerasimov wrote:

Hello!

The Pool has a member `map`, which is accessed from different threads,
thus the access is synchronized.
However, in some code paths (mostly in debug printing) it is accessed
without proper synchronization, which results in intermediate
ConcurrentModificationException when the debug output is turned on.

Would you please help review the fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8159822
WEBREV: http://cr.openjdk.java.net/~igerasim/8159822/00/webrev/

Missed synchronized-s:

  154 private Connections getConnections(Object id) {
  155 ConnectionsRef ref = map.get(id);
  156 return (ref != null) ? ref.getConnections() : null;
  157 }

...gets called via:

  168 public void expire(long threshold) {
  169 synchronized (map) {
   ...
  179 }
  180 expungeStaleConnections();
  181 }

...and also via:

  188 private static void expungeStaleConnections() {
...
  192 Connections conns = releaseRef.getConnections();
...
  203  }
  204 }
But this is ConnectionsWeakRef.getConnections() not 
Pool.getConnections(Object).
As far as I can see, an instance of ConnectionsWeakRef cannot be 
accessed concurrently.




...

  117 public PooledConnection getPooledConnection(Object id, long
timeout,
  118 PooledConnectionFactory factory) throws NamingException {
...
 // no synchronized prior here
  127 expungeStaleConnections();
...

expungeStaleConnections() should be safe to call concurrently as long 
as ReferenceQueue.poll() is thread-safe.


With kind regards,
Ivan

Re: RFR: jsr166 jdk9 integration wave 7

[Paul Sandoz]

Hi Peter,

Thanks, that’s interesting. I am uncertain if our 166 fellows will be reluctant 
or not to pull in a dependency on jdk.internal.misc.SharedSecrets. But i note 
the A*FUs already have a dependency on 
jdk.internal.reflect.Reflection.getCallerClass().

I like the laziness aspect, but i have a preference for Thread to grant it’s 
lookup to TLR and Striped64, a white list if you like. The pro being Thread’s 
fields are not exposed to all within java.base the con being that TLR and 
Striped64 must guard that lookup as they would their own.

There is also another use-case here for the A*FUs, which probably less of a 
priority. What we need is to somehow get the lookup of the caller, perhaps 
injected as an argument into the caller sensitive method newUpdater methods.

Paul.

> On 30 Jun 2016, at 14:41, Peter Levart  wrote:
> 
> Hi,
> 
> I noticed that in ThreadLocalRandom and Striped64, Unsafe remains for the 
> sole purpose of accessing 3 fields in java.lang.Thread class. Now that there 
> are VarHandle(s), they could be combined with SharedSecrets to get rid of 
> Unsafe access in those two classes. For example, in Striped64:
> 
> 
> http://cr.openjdk.java.net/~plevart/misc/JavaLangThreadAccess/webrev.Striped64/
> 
> This has the added benefit that IDE(s) can search for usages of 
> JavaLangThreadAccess.xxx methods to quickly find all the usages. Note that 
> even though Thread is one of primordial classes, obtaining VarHandle(s) for 
> its fields is lazy and works without problems...
> 
> Regards, Peter
> 
> On 06/30/2016 01:20 PM, Martin Buchholz wrote:
>> Webrev regenerated with updates.
>> Lots of rework for the atomic VarHandle specs.
>> http://cr.openjdk.java.net/~martin/webrevs/openjdk9/jsr166-jdk9-integration/
>> 
>> In the unlikely case you want the old webrev, it's at
>> http://cr.openjdk.java.net/~martin/webrevs/openjdk9/jsr166-jdk9-integration.2016-06-29/
> 

Re: RFR 9 : 8160370 : System.getProperty("os.version") returns "Unknown" on Mac

[Roger Riggs]

+1

Can you wrap a couple of very long lines to make the diff easier to read.
- 187
- 202

Thanks, Roger


On 6/29/2016 7:47 PM, Mandy Chung wrote:

On Jun 29, 2016, at 12:36 PM, Brent Christian  
wrote:

Hi,

Please review the following change for JDK 9:

Bug:
https://bugs.openjdk.java.net/browse/JDK-8160370
Webrev:
http://cr.openjdk.java.net/~bchristi/8160370/webrev.00/

The fix for 7131356 fills in the "os.version" system property on Mac using 
[NSProcessInfo operatingSystemVersion], available starting in Mac OS 10.9.

While JDK 9 will not support versions of Mac OS prior to 10.9 [1], not all 
testing infrastructure has been updated, and we've seen failures of

java/lang/System/OsVersionTest.java

The code to restore behavior on older Mac systems is only a few lines, so that 
seems like a good way to get testing going again.

I think we should move away from testing on unsupported platforms.  Is this 
just temporary?  when will this be removed?

Mandy

Re: RFR: jsr166 jdk9 integration wave 7

[Peter Levart]

Hi,

I noticed that in ThreadLocalRandom and Striped64, Unsafe remains for 
the sole purpose of accessing 3 fields in java.lang.Thread class. Now 
that there are VarHandle(s), they could be combined with SharedSecrets 
to get rid of Unsafe access in those two classes. For example, in Striped64:


http://cr.openjdk.java.net/~plevart/misc/JavaLangThreadAccess/webrev.Striped64/

This has the added benefit that IDE(s) can search for usages of 
JavaLangThreadAccess.xxx methods to quickly find all the usages. Note 
that even though Thread is one of primordial classes, obtaining 
VarHandle(s) for its fields is lazy and works without problems...


Regards, Peter

On 06/30/2016 01:20 PM, Martin Buchholz wrote:

Webrev regenerated with updates.
Lots of rework for the atomic VarHandle specs.
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/jsr166-jdk9-integration/ 



In the unlikely case you want the old webrev, it's at
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/jsr166-jdk9-integration.2016-06-29/ 

Re: RFR: jsr166 jdk9 integration wave 7

[Martin Buchholz]

Webrev regenerated with updates.
Lots of rework for the atomic VarHandle specs.
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/jsr166-jdk9-integration/

In the unlikely case you want the old webrev, it's at
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/jsr166-jdk9-integration.2016-06-29/

Re: Semantics of VarHandle CAS methods

[Doug Lea]

On 06/30/2016 06:38 AM, Martin Buchholz wrote:

It's not only about naming.

So yes, I'd like the name weakCompareAndSet to be the sequentially
consistent version, BUT I'd also expect the next more relaxed version to be
memory_order_acq_rel which we don't provide.


There are a few flavors of C++ CAS/weakCAS that aren't explicitly supported
(also including those with different memory orders on success and failure),
because you can get the effects with combinations of the supplied versions and
explicit fences. Supporting all of them would have added lots of seldom-used
methods.


I don't have a good intuition about how useful non-sequentially-consistent
CASes are.


Among other uses, fallible performance counters, and the bases for
combining with fences as above.

-Doug

Re: Semantics of VarHandle CAS methods

[Martin Buchholz]

It's not only about naming.

So yes, I'd like the name weakCompareAndSet to be the sequentially
consistent version, BUT I'd also expect the next more relaxed version to be
memory_order_acq_rel which we don't provide.

I was surprised that weakCompareAndSetAcquire actually does relaxed writes
http://download.java.net/java/jdk9/docs/api/java/lang/invoke/VarHandle.html#weakCompareAndSetAcquire-java.lang.Object...-

I don't have a good intuition about how useful non-sequentially-consistent
CASes are.


On Thu, Jun 30, 2016 at 1:00 AM, Paul Sandoz  wrote:

>
> > On 30 Jun 2016, at 09:37, Andrew Haley  wrote:
> >
> > On 30/06/16 02:20, Martin Buchholz wrote:
> >> VarHandle.compareAndSet is strong in two ways - non-spurious and
> >> sequentially consistent.
> >>
> >> VarHandle.weakCompareAndSet is weak in both these ways.
> >> (That seems like a mistake to me.
> >> The fact that j.u.c. Atomic classes are a precedent for this seems
> >> unfortunate.)
> >
> > Is your disagreement purely about the name?
>
> I was also wondering if that was the source of disgreement.
>
> Paul.
>
> >  We have all of the variants we need.
> >
>

Re: Semantics of VarHandle CAS methods

[Paul Sandoz]

> On 30 Jun 2016, at 09:37, Andrew Haley  wrote:
> 
> On 30/06/16 02:20, Martin Buchholz wrote:
>> VarHandle.compareAndSet is strong in two ways - non-spurious and
>> sequentially consistent.
>> 
>> VarHandle.weakCompareAndSet is weak in both these ways.
>> (That seems like a mistake to me.
>> The fact that j.u.c. Atomic classes are a precedent for this seems
>> unfortunate.)
> 
> Is your disagreement purely about the name?

I was also wondering if that was the source of disgreement.

Paul.

>  We have all of the variants we need.
> 

Re: Semantics of VarHandle CAS methods

[Andrew Haley]

On 30/06/16 02:20, Martin Buchholz wrote:
> VarHandle.compareAndSet is strong in two ways - non-spurious and
> sequentially consistent.
> 
> VarHandle.weakCompareAndSet is weak in both these ways.
> (That seems like a mistake to me.
> The fact that j.u.c. Atomic classes are a precedent for this seems
> unfortunate.)

Is your disagreement purely about the name?  We have all of the
variants we need.

Andrew.