Re: RFR 8200696 : Optimal initial capacity of java.lang.Class.enumConstantDirectory

2018-04-04 Thread Martin Buchholz
Hi Ivan,
I'm seeing

[2018-04-04 20:56:11,999] Agent[1]: stderr: WARNING: An illegal reflective
access operation has occurred
[2018-04-04 20:56:11,999] Agent[1]: stderr: WARNING: Illegal reflective
access by jdk.testlibrary.OptimalCapacity
(file:/tmp/jtr-BHhgGo/classes/lib/testlibrary/) to field
java.util.HashMap.table
[2018-04-04 20:56:11,999] Agent[1]: stderr: WARNING: Please consider
reporting this to the maintainers of jdk.testlibrary.OptimalCapacity

which seems likely a result of this change?

On Wed, Apr 4, 2018 at 4:54 PM, Ivan Gerasimov 
wrote:

> Thanks David and Claes!
>
> I changed the expression to (int)(universe.length / 0.75f) + 1 before
> pushing the fix and updated the Jira bug with more details.
>
> With kind regards,
>
> Ivan
>
>
>
> On 4/3/18 11:56 PM, Claes Redestad wrote:
>
>> Hi Ivan,
>>
>> looks good.
>>
>> Nit: maybe (int)(universe.length / 0.75f) + 1 to keep fp arithmetic to a
>> minimum.
>>
>> /Claes
>>
>> On 2018-04-04 01:22, Ivan Gerasimov wrote:
>>
>>> Hello!
>>>
>>> Yet another occurrence of not-optimally pre-sized HashMap.
>>>
>>> When java.lang.Class.enumConstantDirectory is created, the initial
>>> capacity is set to be (2 * universe.length), which is more than necessary
>>> in some cases.
>>>
>>> Choosing the capacity optimally will allow us to save a few bytes with
>>> some enum classes.
>>>
>>> Would you please help review this trivial fix?
>>>
>>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8200696
>>> WEBREV: http://cr.openjdk.java.net/~igerasim/8200696/00/webrev/
>>>
>>>
>>
>>
> --
> With kind regards,
> Ivan Gerasimov
>
>


Re: RFR: More cleanup patches

2018-04-04 Thread Martin Buchholz
On Fri, Mar 30, 2018 at 3:53 AM, Alan Bateman 
wrote:

>
> 8200125: Fix some classloader/module typos
>> http://cr.openjdk.java.net/~martin/webrevs/jdk/loader-typos/
>> https://bugs.openjdk.java.net/browse/JDK-8200125
>>
> Most of these are okay but you've dropped the text "defined to this
> ClassLoader" from the findClassInModuleOrNull methods. Can we leave that
> text as they really mean "this" class loader, these methods do not delegate.


OK, I'm dropping that part of the change.


Re: RFR: 8200888: typo in name of exception in @throws

2018-04-04 Thread Jonathan Gibbons

Thanks, Joe,

The use of @exception is pervasive in this file, so I think consistency 
wins over localized stylistic changes. But yes, maybe we should do more 
global updates at some point.


-- Jon

On 04/04/2018 05:24 PM, Joseph D. Darcy wrote:

Looks fine; even better if @exception is replaced with @throws :-)

-Joe

On 4/4/2018 5:21 PM, Jonathan Gibbons wrote:
Please review this tiny fix to a typo in the name of an exception in 
@throws:


No webrev; here's the change: NullPointerExcpetion -> 
NullPointerException


diff -r 3930c4d4f805 
src/java.base/share/classes/java/text/ChoiceFormat.java
--- a/src/java.base/share/classes/java/text/ChoiceFormat.java Wed Apr 
04 14:42:53 2018 -0700
+++ b/src/java.base/share/classes/java/text/ChoiceFormat.java Wed Apr 
04 17:12:53 2018 -0700

@@ -312,7 +312,7 @@
  * Constructs with limits and corresponding formats based on the 
pattern.

  *
  * @param newPattern the new pattern string
- * @exception NullPointerExcpetion if {@code newPattern} is
+ * @exception NullPointerException if {@code newPattern} is
  * {@code null}
  * @see #applyPattern
  */

JBS: https://bugs.openjdk.java.net/browse/JDK-8200888

-- Jon






Re: RFR: 8200888: typo in name of exception in @throws

2018-04-04 Thread Joseph D. Darcy

Looks fine; even better if @exception is replaced with @throws :-)

-Joe

On 4/4/2018 5:21 PM, Jonathan Gibbons wrote:
Please review this tiny fix to a typo in the name of an exception in 
@throws:


No webrev; here's the change: NullPointerExcpetion -> 
NullPointerException


diff -r 3930c4d4f805 
src/java.base/share/classes/java/text/ChoiceFormat.java
--- a/src/java.base/share/classes/java/text/ChoiceFormat.java Wed Apr 
04 14:42:53 2018 -0700
+++ b/src/java.base/share/classes/java/text/ChoiceFormat.java Wed Apr 
04 17:12:53 2018 -0700

@@ -312,7 +312,7 @@
  * Constructs with limits and corresponding formats based on the 
pattern.

  *
  * @param newPattern the new pattern string
- * @exception NullPointerExcpetion if {@code newPattern} is
+ * @exception NullPointerException if {@code newPattern} is
  * {@code null}
  * @see #applyPattern
  */

JBS: https://bugs.openjdk.java.net/browse/JDK-8200888

-- Jon




Re: RFR: 8200888: typo in name of exception in @throws

2018-04-04 Thread Brian Burkhalter
+1

Brian

On Apr 4, 2018, at 5:21 PM, Jonathan Gibbons  
wrote:

> Please review this tiny fix to a typo in the name of an exception in @throws:
> 
> […]
> 
> JBS: https://bugs.openjdk.java.net/browse/JDK-8200888



RFR: 8200888: typo in name of exception in @throws

2018-04-04 Thread Jonathan Gibbons
Please review this tiny fix to a typo in the name of an exception in 
@throws:


No webrev; here's the change: NullPointerExcpetion -> NullPointerException

diff -r 3930c4d4f805 
src/java.base/share/classes/java/text/ChoiceFormat.java
--- a/src/java.base/share/classes/java/text/ChoiceFormat.java Wed Apr 04 
14:42:53 2018 -0700
+++ b/src/java.base/share/classes/java/text/ChoiceFormat.java Wed Apr 04 
17:12:53 2018 -0700

@@ -312,7 +312,7 @@
  * Constructs with limits and corresponding formats based on the 
pattern.

  *
  * @param newPattern the new pattern string
- * @exception NullPointerExcpetion if {@code newPattern} is
+ * @exception NullPointerException if {@code newPattern} is
  * {@code null}
  * @see #applyPattern
  */

JBS: https://bugs.openjdk.java.net/browse/JDK-8200888

-- Jon


Re: RFR: 8196750 [Testbug] tools/launcher tests need to tolerate unrelated warnings

2018-04-04 Thread Andrey Nazarov


> On 22 Mar 2018, at 07:18, Kumar Srinivasan  
> wrote:
> 
> David,
> 
> Why would the VM emit these warnings if the deprecated vm flag
> is not being used ?
> 
> Andrey,
> As for the reviews I am ok with InfoStreams, wrt. ToolOpts it is not at all 
> apparent,
> maybe more comments explaining what is going on ?
Added more comments.
see http://cr.openjdk.java.net/~anazarov/JDK-8196750/webrev.02/
> 
> Kumar
> 
> 
>> Hi Andrei,
>> 
>> On 22/03/2018 11:12 AM, Andrey Nazarov wrote:
>>> Hi,
>>> 
>>> Please review fix in launcher tests.
>>> 
>>> Review: http://cr.openjdk.java.net/~anazarov/JDK-8196750/webrev/
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8196750?filter=-1
>> 
>> test/jdk/tools/launcher/ToolsOpts.java
>> 
>> I don't understand how the change fixes the problem. IIUC the problem is 
>> that the test expects the output to consist of the command-line passed to 
>> the tool. Instead if it contains a Warning from the VM, it won't match and 
>> so we fail. The new code no longer uses a String[] and no longer assumes 
>> that output[i] == args[i], but it still searches the current "output" value 
>> for a match with one of the args. But that will still fail if what we 
>> actually have is a Warning. ?? I would have expected to see the Warning 
>> strings filtered out more directly.
>> 
>> The other test change seems reasonable.
>> 
>> Thanks,
>> David
>> 
>>> —Thanks,
>>>  Andrei
>>> 
> 



Re: RFR 8200696 : Optimal initial capacity of java.lang.Class.enumConstantDirectory

2018-04-04 Thread Ivan Gerasimov

Thanks David and Claes!

I changed the expression to (int)(universe.length / 0.75f) + 1 before 
pushing the fix and updated the Jira bug with more details.


With kind regards,

Ivan


On 4/3/18 11:56 PM, Claes Redestad wrote:

Hi Ivan,

looks good.

Nit: maybe (int)(universe.length / 0.75f) + 1 to keep fp arithmetic to 
a minimum.


/Claes

On 2018-04-04 01:22, Ivan Gerasimov wrote:

Hello!

Yet another occurrence of not-optimally pre-sized HashMap.

When java.lang.Class.enumConstantDirectory is created, the initial 
capacity is set to be (2 * universe.length), which is more than 
necessary in some cases.


Choosing the capacity optimally will allow us to save a few bytes 
with some enum classes.


Would you please help review this trivial fix?

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






--
With kind regards,
Ivan Gerasimov



RFR: 8184692: add Pattern.asMatchPredicate

2018-04-04 Thread Vivek Theeyarath
Hi All,

   Please review.

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

Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184692/webrev.00/ 

 

The related jtreg test was run and the test passed .

 

Regards

Vivek


Re: RFR: 8164781: Pattern.asPredicate specification is incomplete

2018-04-04 Thread Paul Sandoz


> On Apr 3, 2018, at 11:54 PM, Vivek Theeyarath  
> wrote:
> 
> Hi,
>   I have incorporated the changes as per the feedback and here is the 
> updated webrev .
> http://cr.openjdk.java.net/~rraghavan/8164781/webrev.02/ .
> Bug: https://bugs.openjdk.java.net/browse/JDK-8164781 
> 

+1 

I know it’s picky, but would you mind sticking closer to the existing line 
length in the source file
(no need for another review)

Did you run the jtreg test to verify it passes? I missed the problem initially, 
glad Stuart caught it, but i presume the test would of reported a failure? if 
not there is something wrong with the test itself that should be investigated.


> Here is the related csr https://bugs.openjdk.java.net/browse/JDK-8200603 
> 

Ok, i tweaked some of the information (after creating a CSR one often needs to 
edit it to fill in the gaps).

Can you blockquote the markdown for the embedded patch since the formatting is 
all messed up?

Thanks,
Paul.

> I will try to address Uwe's point with a fix separately.
> 
> Regards
> Vivek
> -Original Message-
> From: Stuart Marks 
> Sent: Wednesday, April 04, 2018 6:13 AM
> To: Vivek Theeyarath 
> Cc: Paul Sandoz ; Core-Libs-Dev 
> 
> Subject: Re: RFR: 8164781: Pattern.asPredicate specification is incomplete
> 
> Hi Vivek,
> 
> Thanks for taking on this task.
> 
> In case it wasn't clear from Paul's mail, what I think you should do is 
> continue with this fix as a doc-only (and test-only) change, and not modify 
> the behavior of this method. Doing that would be an incompatible change.
> 
> Uwe's point is a reasonable one, which is that you can't tell from the method 
> name "asPredicate" whether it uses find() or match() semantics. Oh well, I 
> think we just have to live with this, and document it clearly.
> 
> Adding a method to create a Predicate that has match() semantics would be a 
> fine task to consider separately.
> 
> Also, in RegExTest.java,
> 
> 4686 if (p.test("word1234")) {
> 4687 failCount++;
> 4688 }
> 
> I think the logic should be negated, as the predicate should properly find 
> the pattern in this string.
> 
> Thanks,
> 
> s'marks
> 
> On 4/2/18 10:56 AM, Paul Sandoz wrote:
>> Hi,
>> 
>> Looks good, expect for:
>> 
>> 5823  * @return  The predicate which can be used for finding on a string
>> 
>> “finding on a… ” is a little awkward to parse . I recommend to either change 
>> it back, since the first sentence of the method doc says what it means by 
>> matches, or being a little more verbose:
>> 
>>   The predicate which can be used for finding a match on a 
>> subsequence of a string
>> 
>> You will need a CSR to document the clarification in specification behavior.
>> 
>> —
>> 
>> To Uwe’s point, we could have chosen a more descriptive method name, e.g. 
>> asFinding/Predicate, leaving logical space for say any future 
>> asMatching/Predicate if we chose to add it.
>> 
>> Paul.
>> 
>> 
>>> On Apr 1, 2018, at 1:11 AM, Vivek Theeyarath  
>>> wrote:
>>> 
>>> Hi all,
>>> 
>>>   Please review.
>>> 
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8164781
>>> 
>>> Webrev: http://cr.openjdk.java.net/~rraghavan/8164781/webrev.01/
>>> 
>>> 
>>> 
>>> Regards
>>> 
>>> Vivek
>> 



Re: RFR: [PATCH] 8176553 Fix LDAP referral loop

2018-04-04 Thread Jan Kalina
Just note the LDIF expect the LDAP server running at:
ldap://localhost:10389/dc=example,dc=com

The "ref" needs to reference the same server to create referrals loop.

If you are unable to reproduce, does it produce LimitExceededException for you?

On Wed, Apr 4, 2018 at 6:05 PM, Jan Kalina  wrote:
> On Fedora 27, using latest Apache Directory Studio
> (ApacheDirectoryStudio-2.0.0.v20170904-M13-linux.gtk.x86_64.tar.gz)
> created new Apache DS 2.0.0, imported referrals.ldif
> and running JI9048012.java:
>
> export JAVA_HOME=/opt/jdk-9_linux-x64_bin/
> $JAVA_HOME/bin/java JI9048012
ou=test

ldap://localhost:10389/ou=test,dc=example,dc=com
ldap://localhost:10389/dc=example,dc=com
ldap://localhost:10389/ou=test,dc=example,dc=com
ldap://localhost:10389/dc=example,dc=com
ldap://localhost:10389/ou=test,dc=example,dc=com
ldap://localhost:10389/dc=example,dc=com
ldap://localhost:10389/ou=test,dc=example,dc=com
ldap://localhost:10389/dc=example,dc=com
ldap://localhost:10389/ou=test,dc=example,dc=com
ldap://localhost:10389/dc=example,dc=com
>
> Output continues indefinitely, while LimitExceededException is EXPECTED,
> but it does not occur.
>
> On Wed, Apr 4, 2018 at 5:35 PM, Vyom Tewari  wrote:
>>
>>
>> On 4/4/2018 8:59 PM, Jan Kalina wrote:
>>>
>>> Note: Test is not included, as it would require running LDAP server.
>>> (existing ldap tests in JDK use only mocks of the server)
>>>
>>> The patch was manually verified using reproducer attached to issue.
>>
>> Hi Jan,
>>
>> I ran the reproducer long back on my Linux box(Ubuntu 1604) on apacheDS 2
>> and it was not reproducible at my end. can you please let us know your
>> environment detail.
>> Thanks,
>> Vyom
>>
>>> On Wed, Apr 4, 2018 at 4:12 PM, Jan Kalina  wrote:

 Hi,
 I has prepared trivial patch for bug JDK-8176553,
 which I would like to get reviewed and sponsored.

 I am covered by Red Hat OCA.

 The bug affects upstream, JDK9 and JDK8 too.

 The reproducer is available in issue tracker:
 https://bugs.openjdk.java.net/browse/JDK-8176553

 PATCH:
 --
 diff --git
 a/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java

 b/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
 ---
 a/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
 +++
 b/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
 @@ -312,7 +312,8 @@

   if ((refEx != null) &&
   (refEx.hasMoreReferrals() ||
 - refEx.hasMoreReferralExceptions())) {
 + refEx.hasMoreReferralExceptions()) &&
 + ! (errEx instanceof LimitExceededException)) {

   if (homeCtx.handleReferrals == LdapClient.LDAP_REF_THROW) {
   throw (NamingException)(refEx.fillInStackTrace());
 --

 Thanks,
 Jan Kalina
>>
>>


Re: RFR: [PATCH] 8176553 Fix LDAP referral loop

2018-04-04 Thread Jan Kalina
On Fedora 27, using latest Apache Directory Studio
(ApacheDirectoryStudio-2.0.0.v20170904-M13-linux.gtk.x86_64.tar.gz)
created new Apache DS 2.0.0, imported referrals.ldif
and running JI9048012.java:

export JAVA_HOME=/opt/jdk-9_linux-x64_bin/
$JAVA_HOME/bin/java JI9048012
>>>ou=test
>>>
>>>ldap://localhost:10389/ou=test,dc=example,dc=com
>>>ldap://localhost:10389/dc=example,dc=com
>>>ldap://localhost:10389/ou=test,dc=example,dc=com
>>>ldap://localhost:10389/dc=example,dc=com
>>>ldap://localhost:10389/ou=test,dc=example,dc=com
>>>ldap://localhost:10389/dc=example,dc=com
>>>ldap://localhost:10389/ou=test,dc=example,dc=com
>>>ldap://localhost:10389/dc=example,dc=com
>>>ldap://localhost:10389/ou=test,dc=example,dc=com
>>>ldap://localhost:10389/dc=example,dc=com

Output continues indefinitely, while LimitExceededException is EXPECTED,
but it does not occur.

On Wed, Apr 4, 2018 at 5:35 PM, Vyom Tewari  wrote:
>
>
> On 4/4/2018 8:59 PM, Jan Kalina wrote:
>>
>> Note: Test is not included, as it would require running LDAP server.
>> (existing ldap tests in JDK use only mocks of the server)
>>
>> The patch was manually verified using reproducer attached to issue.
>
> Hi Jan,
>
> I ran the reproducer long back on my Linux box(Ubuntu 1604) on apacheDS 2
> and it was not reproducible at my end. can you please let us know your
> environment detail.
> Thanks,
> Vyom
>
>> On Wed, Apr 4, 2018 at 4:12 PM, Jan Kalina  wrote:
>>>
>>> Hi,
>>> I has prepared trivial patch for bug JDK-8176553,
>>> which I would like to get reviewed and sponsored.
>>>
>>> I am covered by Red Hat OCA.
>>>
>>> The bug affects upstream, JDK9 and JDK8 too.
>>>
>>> The reproducer is available in issue tracker:
>>> https://bugs.openjdk.java.net/browse/JDK-8176553
>>>
>>> PATCH:
>>> --
>>> diff --git
>>> a/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
>>>
>>> b/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
>>> ---
>>> a/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
>>> +++
>>> b/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
>>> @@ -312,7 +312,8 @@
>>>
>>>   if ((refEx != null) &&
>>>   (refEx.hasMoreReferrals() ||
>>> - refEx.hasMoreReferralExceptions())) {
>>> + refEx.hasMoreReferralExceptions()) &&
>>> + ! (errEx instanceof LimitExceededException)) {
>>>
>>>   if (homeCtx.handleReferrals == LdapClient.LDAP_REF_THROW) {
>>>   throw (NamingException)(refEx.fillInStackTrace());
>>> --
>>>
>>> Thanks,
>>> Jan Kalina
>
>


RFR: JDK-8190187: C++ code calling JNI_CreateJavaVM can be killed by Java

2018-04-04 Thread Adam Farley8
Hi All, 

I've attached the code to resolve JDK-8190187

Could a committer please take the fix (amended code attached, and 
available on request) and: 

1) Complete the CSR process for the new JNI Return code. 
2) Commit the changes that contain the Return code.

And, optionally, 3) Perform 1 and 2 for the jdwp agent changes as well, so 
it can use this new functionality.



Best Regards

Adam Farleydiff --git a/src/hotspot/share/prims/jni.cpp b/src/hotspot/share/prims/jni.cpp
--- a/src/hotspot/share/prims/jni.cpp
+++ b/src/hotspot/share/prims/jni.cpp
@@ -3909,7 +3909,7 @@
   bool can_try_again = true;
 
   result = Threads::create_vm((JavaVMInitArgs*) args, _try_again);
-  if (result == JNI_OK) {
+  if ((result == JNI_OK) || (result == JNI_SILENT_EXIT)) {
 JavaThread *thread = JavaThread::current();
 assert(!thread->has_pending_exception(), "should have returned not OK");
 /* thread is thread_in_vm here */
diff --git a/src/hotspot/share/prims/jvmti.xml 
b/src/hotspot/share/prims/jvmti.xml
--- a/src/hotspot/share/prims/jvmti.xml
+++ b/src/hotspot/share/prims/jvmti.xml
@@ -631,8 +631,10 @@
 
 
 The return value from Agent_OnLoad or
-Agent_OnLoad_agent-lib-name is used to indicate an 
error.
-Any value other than zero indicates an error and causes termination of the 
VM.
+Agent_OnLoad_agent-lib-name is used to indicate 
whether
+the agent successfully initialised or not. JNI_OK indicates success, 
JNI_SILENT_EXIT
+indicates the agent did not fully initialize but that no error occurred, 
and any
+other value indicates an error. Non-JNI_OK return codes cause termination 
of the VM.
   
 
   
@@ -14811,6 +14813,12 @@
- The function may return NULL in the start phase if the
  can_generate_early_vmstart capability is enabled.
   
+  
+  Added JNI_SILENT_EXIT return code for early exits without errors.
+  java.c's ParseArguments function now sets pret value to 2 for a NULL 
pwhat. Addendums:
+ - This allows us to clearly identify when no class was set, but no 
other error has occurred.
+ - This is undone in java.c's ContinueInNewThread method, so the 
surface behaviour does not change.  
+  
 
 
 
diff --git a/src/hotspot/share/runtime/thread.cpp 
b/src/hotspot/share/runtime/thread.cpp
--- a/src/hotspot/share/runtime/thread.cpp
+++ b/src/hotspot/share/runtime/thread.cpp
@@ -3637,6 +3637,9 @@
 }
 
 jint Threads::create_vm(JavaVMInitArgs* args, bool* canTryAgain) {
+  /* This gets returned at the end of the method. */
+  /* It allows us to complete vm initialisation and still return an error code 
if we want. */
+  jint jniReturnCode = JNI_OK;
   extern void JDK_Version_init();
 
   // Preinitialize version info.
@@ -3725,7 +3728,9 @@
 
   // Launch -agentlib/-agentpath and converted -Xrun agents
   if (Arguments::init_agents_at_startup()) {
-create_vm_init_agents();
+if (create_vm_init_agents() == JNI_SILENT_EXIT) {
+  jniReturnCode = JNI_SILENT_EXIT;
+}
   }
 
   // Initialize Threads state
@@ -3991,7 +3996,7 @@
 ShouldNotReachHere();
   }
 
-  return JNI_OK;
+  return jniReturnCode;
 }
 
 // type for the Agent_OnLoad and JVM_OnLoad entry points
@@ -4110,9 +4115,10 @@
 // Create agents for -agentlib:  -agentpath:  and converted -Xrun
 // Invokes Agent_OnLoad
 // Called very early -- before JavaThreads exist
-void Threads::create_vm_init_agents() {
+jint Threads::create_vm_init_agents() {
   extern struct JavaVM_ main_vm;
   AgentLibrary* agent;
+  jint jniReturnCode = JNI_OK;
 
   JvmtiExport::enter_onload_phase();
 
@@ -4123,13 +4129,18 @@
   // Invoke the Agent_OnLoad function
   jint err = (*on_load_entry)(_vm, agent->options(), NULL);
   if (err != JNI_OK) {
-vm_exit_during_initialization("agent library failed to init", 
agent->name());
+if (err == JNI_SILENT_EXIT) {
+  jniReturnCode = JNI_SILENT_EXIT;
+} else {
+  vm_exit_during_initialization("agent library failed to init", 
agent->name());
+}
   }
 } else {
   vm_exit_during_initialization("Could not find Agent_OnLoad function in 
the agent library", agent->name());
 }
   }
   JvmtiExport::enter_primordial_phase();
+  return jniReturnCode;
 }
 
 extern "C" {
diff --git a/src/hotspot/share/runtime/thread.hpp 
b/src/hotspot/share/runtime/thread.hpp
--- a/src/hotspot/share/runtime/thread.hpp
+++ b/src/hotspot/share/runtime/thread.hpp
@@ -2153,7 +2153,7 @@
   static jint create_vm(JavaVMInitArgs* args, bool* canTryAgain);
   static void convert_vm_init_libraries_to_agents();
   static void create_vm_init_libraries();
-  static void create_vm_init_agents();
+  static jint create_vm_init_agents();
   static void shutdown_vm_agents();
   static bool destroy_vm();
   // Supported VM versions via JNI
diff --git a/src/java.base/share/native/include/jni.h 
b/src/java.base/share/native/include/jni.h
--- a/src/java.base/share/native/include/jni.h
+++ 

Re: RFR: [PATCH] 8176553 Fix LDAP referral loop

2018-04-04 Thread Vyom Tewari



On 4/4/2018 8:59 PM, Jan Kalina wrote:

Note: Test is not included, as it would require running LDAP server.
(existing ldap tests in JDK use only mocks of the server)

The patch was manually verified using reproducer attached to issue.

Hi Jan,

I ran the reproducer long back on my Linux box(Ubuntu 1604) on apacheDS 
2 and it was not reproducible at my end. can you please let us know your 
environment detail.

Thanks,
Vyom

On Wed, Apr 4, 2018 at 4:12 PM, Jan Kalina  wrote:

Hi,
I has prepared trivial patch for bug JDK-8176553,
which I would like to get reviewed and sponsored.

I am covered by Red Hat OCA.

The bug affects upstream, JDK9 and JDK8 too.

The reproducer is available in issue tracker:
https://bugs.openjdk.java.net/browse/JDK-8176553

PATCH:
--
diff --git 
a/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
b/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
--- 
a/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
+++ 
b/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
@@ -312,7 +312,8 @@

  if ((refEx != null) &&
  (refEx.hasMoreReferrals() ||
- refEx.hasMoreReferralExceptions())) {
+ refEx.hasMoreReferralExceptions()) &&
+ ! (errEx instanceof LimitExceededException)) {

  if (homeCtx.handleReferrals == LdapClient.LDAP_REF_THROW) {
  throw (NamingException)(refEx.fillInStackTrace());
--

Thanks,
Jan Kalina




Re: RFR: [PATCH] 8176553 Fix LDAP referral loop

2018-04-04 Thread Jan Kalina
Note: Test is not included, as it would require running LDAP server.
(existing ldap tests in JDK use only mocks of the server)

The patch was manually verified using reproducer attached to issue.

On Wed, Apr 4, 2018 at 4:12 PM, Jan Kalina  wrote:
> Hi,
> I has prepared trivial patch for bug JDK-8176553,
> which I would like to get reviewed and sponsored.
>
> I am covered by Red Hat OCA.
>
> The bug affects upstream, JDK9 and JDK8 too.
>
> The reproducer is available in issue tracker:
> https://bugs.openjdk.java.net/browse/JDK-8176553
>
> PATCH:
> --
> diff --git 
> a/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
> b/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
> --- 
> a/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
> +++ 
> b/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
> @@ -312,7 +312,8 @@
>
>  if ((refEx != null) &&
>  (refEx.hasMoreReferrals() ||
> - refEx.hasMoreReferralExceptions())) {
> + refEx.hasMoreReferralExceptions()) &&
> + ! (errEx instanceof LimitExceededException)) {
>
>  if (homeCtx.handleReferrals == LdapClient.LDAP_REF_THROW) {
>  throw (NamingException)(refEx.fillInStackTrace());
> --
>
> Thanks,
> Jan Kalina


Re: RFR: Export InitializeEncoding for JVM access

2018-04-04 Thread Roger Riggs

Hi Andrew,

The function itself seems straightforward enough.

Can you clarify the boundary that you are replacing?  The initialization 
has a lot of

intermingled responsibilities and is quite sensitive.

What else needs to be said about when it can be called and who should be 
calling it?


I don't see the similarity to Canonicalize, which is not a supported 
interface, just

exported for use within the implementation.

Thanks, Roger


On 4/4/2018 8:17 AM, Andrew Leonard wrote:

Hi,
I would like to propose and find a sponsor for this change please, which
is is export from libjava the JNU_InitializeEncoding() method, so that a
JVM can initialize the platform encoding. Currently
Java_java_lang_System_initProperties initializes the encoding, however for

a generic JVM that will naturally provide it's own System implementation,
the JVM needs a way to initialize this encoding directly. We can thus
simply export it as per the change below.. There is precendence for doing
this sort of export with Canonicalize().
Thanks
Andrew

diff --git a/src/java.base/share/native/libjava/jni_util.c
b/src/java.base/share/native/libjava/jni_util.c
--- a/src/java.base/share/native/libjava/jni_util.c
+++ b/src/java.base/share/native/libjava/jni_util.c
@@ -840,6 +840,15 @@
  String_value_ID = (*env)->GetFieldID(env, strClazz, "value", "[B");
  }
  
+/*

+ * Export this method to allow JVM to initialize platform encoding
explicitly.
+ */
+JNIEXPORT void JNICALL
+JNU_InitializeEncoding(JNIEnv *env, const char *encname)
+{
+   InitializeEncoding(env, encname);
+}
+
  JNIEXPORT jstring
  NewStringPlatform(JNIEnv *env, const char *str)
  {




Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leon...@uk.ibm.com


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




Re: RFR of JDK-8188897: java/rmi/registry/reexport/Reexport.java failed with Port already in use

2018-04-04 Thread Roger Riggs

Hi Hamlin,

Reexport.java:

I think this change to use a separate process for the 2nd registry 
changes the test so that it does not
address the original test case.  The original problem was the incorrect 
retention of an object in

the object table when the create of a registry in the same process failed.
Finally being able to create the registry in the same process assured 
that the object was not

retained in the object table.

Go back to creating the 2nd registry in the test process.


RegistryVM.java: 2, the copyright update should be  "2017, 2018," range.

(I'm  really not a fan of all the RegistryVM methods with the same name 
and minimal and implicit
differences in their functions.  When reading the test, you have to go 
and read the RegistryVM method
to see what it does.  I would have preferred that the full 
createRegistryVM (out, err, options, port) method
was used directly in the tests.  In the case of a method used once, it 
is an inconvenience method, not a convenience).


The new terminate() method is quite similar to the existing cleanup() 
method which does not wait.

It would be a good cleanup to figure out if another method is really needed.
The override is going to change the behavior of the existing uses of 
terminate().

It should be checked that it does not break any existing uses.

178: 187: The method comments are not consistent, one says forcibly and 
the other gracefully

but both call requestExit and both call destroy().

Thanks, Roger


On 4/3/2018 11:35 PM, Hamlin Li wrote:

Hi Joe, Roger,

Thank you for reviewing, I have refactored the test more and fix as 
you suggested.


please review http://cr.openjdk.java.net/~mli/8188897/webrev.01/

Thank you

-Hamlin


On 04/04/2018 10:42 AM, Joseph D. Darcy wrote:

Hello,

Some general comments on the coding for tests like this:

* It is preferable to avoid sleep in tests to avoid increasing the 
minimum amount of time a test takes to run. This helps limit the 
overall time it takes the test suite to run.


* If timeouts are used, it is recommend to factor the maximum time 
waited with the jtreg timeout scaling factor; I don't recall its 
exact name. In other words, many of our tests are run on heavily 
loaded systems and the tests take longer than run than on typical 
laptops and workstations so jtreg is invoked with an timeout scaling 
factor. Individual tests should be sensitive to this scaling factor 
for any internal timeout that need to be used.


HTH,

-Joe

On 4/3/2018 7:48 AM, Roger Riggs wrote:

Hi Hamlin,

Instead of a simple time delay, it would be useful to wait for the 
RegistryVM to terminate.

In killRegistry: 149,  adding subreg.waitFor() should be sufficient.

58: If using a 'for' loop it would be easier to understand if it 
included the usual start, increment and termination.

Instead of burying it in the exception handler.

59, 102, 104: the introduction of the kill boolean makes the test 
harder to understand and seems to be unnecessary.
 the killRegistry() method already will only kill the subprocess if 
it still is alive.


Roger

On 4/2/2018 6:33 AM, Hamlin Li wrote:

would you please review the following patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8188897

webrev: http://cr.openjdk.java.net/~mli/8188897/webrev.00/


Thank you

-Hamlin











Re: Patch fixing JDK-8176553

2018-04-04 Thread Alan Bateman



On 04/04/2018 15:06, David Lloyd wrote:

Actually I've talked to Jan and we're going to try again with the more
correct subject line & RFR.  Thanks.


It would be good to see if a test can be developed too.

-Alan


RFR: [PATCH] 8176553 Fix LDAP referral loop

2018-04-04 Thread Jan Kalina
Hi,
I has prepared trivial patch for bug JDK-8176553,
which I would like to get reviewed and sponsored.

I am covered by Red Hat OCA.

The bug affects upstream, JDK9 and JDK8 too.

The reproducer is available in issue tracker:
https://bugs.openjdk.java.net/browse/JDK-8176553

PATCH:
--
diff --git 
a/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
b/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
--- 
a/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
+++ 
b/src/java.naming/share/classes/com/sun/jndi/ldap/AbstractLdapNamingEnumeration.java
@@ -312,7 +312,8 @@

 if ((refEx != null) &&
 (refEx.hasMoreReferrals() ||
- refEx.hasMoreReferralExceptions())) {
+ refEx.hasMoreReferralExceptions()) &&
+ ! (errEx instanceof LimitExceededException)) {

 if (homeCtx.handleReferrals == LdapClient.LDAP_REF_THROW) {
 throw (NamingException)(refEx.fillInStackTrace());
--

Thanks,
Jan Kalina


Re: Patch fixing JDK-8176553

2018-04-04 Thread David Lloyd
Actually I've talked to Jan and we're going to try again with the more
correct subject line & RFR.  Thanks.

On Wed, Apr 4, 2018 at 8:51 AM, David Lloyd  wrote:
> Is there anyone who would be willing to sponsor this change?
>
> On Tue, Jan 23, 2018 at 4:58 PM, Jan Kalina  wrote:
>> Hi,
>> I has prepared trivial patch for bug JDK-8176553, which I has reported.
>>
>> I will welcome if it could be merged into JDK.
>> (The bug is present in JDK9 and JDK8 too.)
>> I am covered by Red Hat OCA.
>>
>> The patch is attached, bug reproducer is already in JIRA:
>> https://bugs.openjdk.java.net/browse/JDK-8176553
>>
>> Thanks,
>> Jan Kalina
>
>
>
> --
> - DML



-- 
- DML


Re: Patch fixing JDK-8176553

2018-04-04 Thread David Lloyd
Is there anyone who would be willing to sponsor this change?

On Tue, Jan 23, 2018 at 4:58 PM, Jan Kalina  wrote:
> Hi,
> I has prepared trivial patch for bug JDK-8176553, which I has reported.
>
> I will welcome if it could be merged into JDK.
> (The bug is present in JDK9 and JDK8 too.)
> I am covered by Red Hat OCA.
>
> The patch is attached, bug reproducer is already in JIRA:
> https://bugs.openjdk.java.net/browse/JDK-8176553
>
> Thanks,
> Jan Kalina



-- 
- DML


RFR: Export InitializeEncoding for JVM access

2018-04-04 Thread Andrew Leonard
Hi,
I would like to propose and find a sponsor for this change please, which 
is is export from libjava the JNU_InitializeEncoding() method, so that a 
JVM can initialize the platform encoding. Currently 
Java_java_lang_System_initProperties initializes the encoding, however for 

a generic JVM that will naturally provide it's own System implementation, 
the JVM needs a way to initialize this encoding directly. We can thus 
simply export it as per the change below.. There is precendence for doing 
this sort of export with Canonicalize().
Thanks
Andrew 

diff --git a/src/java.base/share/native/libjava/jni_util.c 
b/src/java.base/share/native/libjava/jni_util.c
--- a/src/java.base/share/native/libjava/jni_util.c
+++ b/src/java.base/share/native/libjava/jni_util.c
@@ -840,6 +840,15 @@
 String_value_ID = (*env)->GetFieldID(env, strClazz, "value", "[B");
 }
 
+/*
+ * Export this method to allow JVM to initialize platform encoding 
explicitly.
+ */
+JNIEXPORT void JNICALL
+JNU_InitializeEncoding(JNIEnv *env, const char *encname)
+{
+   InitializeEncoding(env, encname);
+}
+
 JNIEXPORT jstring
 NewStringPlatform(JNIEnv *env, const char *str)
 {




Andrew Leonard
Java Runtimes Development
IBM Hursley
IBM United Kingdom Ltd
Phone internal: 245913, external: 01962 815913
internet email: andrew_m_leon...@uk.ibm.com 


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


Re: [11] RFR 8200703: Problem list jdk/jshell/ExceptionsTest.java fails on windows

2018-04-04 Thread Amy Lu

Thank you Jan.

Thanks,
Amy

On 04/04/2018 4:37 PM, Jan Lahoda wrote:

Hi,

Ok to problem list.

Jan

On 4.4.2018 05:26, Amy Lu wrote:

jdk/jshell/ExceptionsTest.java

This test has been failing on Windows (JDK-8200701) since the push for
JDK-8198801. The test needs to be problem listed on Windows until a full
fix is developed.

Please review the patch below.

Thanks,
Amy


--- old/test/langtools/ProblemList.txt    2018-04-04 11:17:19.0
+0800
+++ new/test/langtools/ProblemList.txt    2018-04-04 11:17:19.0
+0800
@@ -38,6 +38,7 @@

  jdk/jshell/UserJdiUserRemoteTest.java 8173079    linux-all
  jdk/jshell/UserInputTest.java 8169536    generic-all
+jdk/jshell/ExceptionsTest.java 8200701    windows-all

###
  #





Re: [11] RFR 8200703: Problem list jdk/jshell/ExceptionsTest.java fails on windows

2018-04-04 Thread Amy Lu

[Including core-libs-dev]

Quick review needed.

bug: https://bugs.openjdk.java.net/browse/JDK-8200703
webrev: http://cr.openjdk.java.net/~amlu/8200703/webrev.00/

Thanks,
Amy

On 04/04/2018 11:26 AM, Amy Lu wrote:

jdk/jshell/ExceptionsTest.java

This test has been failing on Windows (JDK-8200701) since the push for 
JDK-8198801. The test needs to be problem listed on Windows until a 
full fix is developed.


Please review the patch below.

Thanks,
Amy


--- old/test/langtools/ProblemList.txt    2018-04-04 
11:17:19.0 +0800
+++ new/test/langtools/ProblemList.txt    2018-04-04 
11:17:19.0 +0800

@@ -38,6 +38,7 @@

 jdk/jshell/UserJdiUserRemoteTest.java 8173079    linux-all
 jdk/jshell/UserInputTest.java 8169536    generic-all
+jdk/jshell/ExceptionsTest.java 8200701    windows-all

 ### 


 #





Re: RFR 8200696 : Optimal initial capacity of java.lang.Class.enumConstantDirectory

2018-04-04 Thread Claes Redestad

Hi Ivan,

looks good.

Nit: maybe (int)(universe.length / 0.75f) + 1 to keep fp arithmetic to a 
minimum.


/Claes

On 2018-04-04 01:22, Ivan Gerasimov wrote:

Hello!

Yet another occurrence of not-optimally pre-sized HashMap.

When java.lang.Class.enumConstantDirectory is created, the initial 
capacity is set to be (2 * universe.length), which is more than 
necessary in some cases.


Choosing the capacity optimally will allow us to save a few bytes with 
some enum classes.


Would you please help review this trivial fix?

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





Re: RFR: 8200664: fix broken links in java.base docs

2018-04-04 Thread Alan Bateman

On 03/04/2018 22:17, Jonathan Gibbons wrote:
Please review a small update to fix some broken links in the java.base 
API docs.


The change is necessary because when generating HTML 5 output, javadoc
no longer has to encode method signatures into the restricted set of 
characters

available in HTML 4 names.

JBS: https://bugs.openjdk.java.net/browse/JDK-8200664
Webrev: http://cr.openjdk.java.net/~jjg/8200664/webrev.00/
This looks good to me. These dashes have always been confused in links 
so good that they will no longer be needed.


-Alan


RE: RFR: 8164781: Pattern.asPredicate specification is incomplete

2018-04-04 Thread Vivek Theeyarath
Hi,
I have incorporated the changes as per the feedback and here is the 
updated webrev .
http://cr.openjdk.java.net/~rraghavan/8164781/webrev.02/ .
Bug: https://bugs.openjdk.java.net/browse/JDK-8164781 

Here is the related csr https://bugs.openjdk.java.net/browse/JDK-8200603 

I will try to address Uwe's point with a fix separately.

Regards
Vivek
-Original Message-
From: Stuart Marks 
Sent: Wednesday, April 04, 2018 6:13 AM
To: Vivek Theeyarath 
Cc: Paul Sandoz ; Core-Libs-Dev 

Subject: Re: RFR: 8164781: Pattern.asPredicate specification is incomplete

Hi Vivek,

Thanks for taking on this task.

In case it wasn't clear from Paul's mail, what I think you should do is 
continue with this fix as a doc-only (and test-only) change, and not modify the 
behavior of this method. Doing that would be an incompatible change.

Uwe's point is a reasonable one, which is that you can't tell from the method 
name "asPredicate" whether it uses find() or match() semantics. Oh well, I 
think we just have to live with this, and document it clearly.

Adding a method to create a Predicate that has match() semantics would be a 
fine task to consider separately.

Also, in RegExTest.java,

4686 if (p.test("word1234")) {
4687 failCount++;
4688 }

I think the logic should be negated, as the predicate should properly find the 
pattern in this string.

Thanks,

s'marks

On 4/2/18 10:56 AM, Paul Sandoz wrote:
> Hi,
> 
> Looks good, expect for:
> 
> 5823  * @return  The predicate which can be used for finding on a string
> 
> “finding on a… ” is a little awkward to parse . I recommend to either change 
> it back, since the first sentence of the method doc says what it means by 
> matches, or being a little more verbose:
> 
>The predicate which can be used for finding a match on a 
> subsequence of a string
> 
> You will need a CSR to document the clarification in specification behavior.
> 
> —
> 
> To Uwe’s point, we could have chosen a more descriptive method name, e.g. 
> asFinding/Predicate, leaving logical space for say any future 
> asMatching/Predicate if we chose to add it.
> 
> Paul.
> 
> 
>> On Apr 1, 2018, at 1:11 AM, Vivek Theeyarath  
>> wrote:
>>
>> Hi all,
>>
>>Please review.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8164781
>>
>> Webrev: http://cr.openjdk.java.net/~rraghavan/8164781/webrev.01/
>>
>>
>>
>> Regards
>>
>> Vivek
>