Re: RFR 8200696 : Optimal initial capacity of java.lang.Class.enumConstantDirectory
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 Gerasimovwrote: > 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
On Fri, Mar 30, 2018 at 3:53 AM, Alan Batemanwrote: > > 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
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
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
+1 Brian On Apr 4, 2018, at 5:21 PM, Jonathan Gibbonswrote: > 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
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
> 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
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
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
> 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
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 Kalinawrote: > 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
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 Tewariwrote: > > > 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
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
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 Kalinawrote: 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
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 Kalinawrote: > 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
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
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
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
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
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 Lloydwrote: > 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
Is there anyone who would be willing to sponsor this change? On Tue, Jan 23, 2018 at 4:58 PM, Jan Kalinawrote: > 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
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
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
[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
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
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
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 TheeyarathCc: 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 >