This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 11.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit d97ff89a7e7c82519106ca89437f686fb6bfaefa Author: opencode <[email protected]> AuthorDate: Wed Nov 4 13:43:16 2026 +0100 Put the JMX helper's documentation in line with what it does - Log the composed name pieces and the object class when a registration fails: a malformed composed name is the one failure whose JDK message quotes nothing, so several pools sharing the platform server made a failure unattributable. - Document getObjectName's concatenation contract: the key property must carry its own leading comma and a value the ObjectName grammar accepts, and a null base is the deliberate, unlogged skip. - Scope the unregisterJmx contract claim: a throwing postDeregister() fails the call although the server already took the MBean down, so false only normally means still-published. Probe-verified on JDK 26. - Document the null-base skip on registerJmx and replace the misleading 'Constructs a new JmxUtil' javadoc on the never-constructible private constructor. - LocalStrings: parameterize jmxRegistrationFailed, rename the already-gone debug key to jmxAlreadyUnregistered with matching text, and say 'normally still registered' in the refusal text. - TestJmxRegistration: drop a comment clause claiming a failing postDeregister() produces the still-registered refusal state; the removal is committed before that hook runs (probe-verified). No functional change. Full jdbc-pool suite: 929 tests, 0 failures. --- .../org/apache/tomcat/jdbc/pool/jmx/JmxUtil.java | 45 +++++++++++++++------- .../tomcat/jdbc/pool/util/LocalStrings.properties | 6 +-- .../tomcat/jdbc/pool/jmx/TestJmxRegistration.java | 4 +- 3 files changed, 37 insertions(+), 18 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/jmx/JmxUtil.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/jmx/JmxUtil.java index b3dcc4c7c4..04eb606fac 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/jmx/JmxUtil.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/jmx/JmxUtil.java @@ -32,9 +32,7 @@ import org.apache.tomcat.jdbc.pool.util.MessageFactory; */ public class JmxUtil { - /** - * Constructs a new JmxUtil. - */ + // Not instantiable: this class only holds the static registration helpers private JmxUtil() { } @@ -43,13 +41,17 @@ public class JmxUtil { /** * Register an object as an MBean. * - * @param base the base object name + * @param base the base object name, or {@code null} to register nothing - + * the skip a component whose own registration is absent relies + * on, so it is deliberate and unlogged * @param keyprop the key property * @param obj the object to register * - * @return the registered object name, or {@code null} on failure. A registration the MBean server refused - because - * the name is already in use by another MBean, for instance - is such a failure, so a non-{@code null} - * name always means this call is the one that published the object under that name + * @return the registered object name, or {@code null} on failure - or on the + * absent-base skip. A registration the MBean server refused - because + * the name is already in use by another MBean, for instance - is such + * a failure, so a non-{@code null} name always means this call is the + * one that published the object under that name */ public static ObjectName registerJmx(ObjectName base, String keyprop, Object obj) { ObjectName oname = null; @@ -64,7 +66,12 @@ public class JmxUtil { oname = name; } } catch (Exception e) { - log.error(MessageFactory.get("jmxUtil.jmxRegistrationFailed"), e); + // The log is the only record this failure ever gets, and the JDK + // message of a malformed composed name does not quote that name: + // name the pieces, so a refused registration is attributable when + // several pools share the platform server. + log.error(MessageFactory.get("jmxUtil.jmxRegistrationFailed", base, keyprop, + obj == null ? null : obj.getClass().getName()), e); } return oname; } @@ -78,9 +85,12 @@ public class JmxUtil { * @return {@code true} if the name is no longer registered - either this * call took it down, or it was already gone, or there was * nothing to release; {@code false} if the MBean server refused - * to unregister it, which means the MBean is still published - * under that name and, since callers drop their handle - * regardless, held by nothing for the rest of the JVM's life + * to unregister it, which means the MBean is normally still + * published under that name and, since callers drop their + * handle regardless, held by nothing for the rest of the JVM's + * life (an MBean whose {@code postDeregister()} throws reports + * the same failure although the server already took it down; + * the pool's own MBeans never do) */ public static boolean unregisterJmx(ObjectName oname) { if (oname == null) { @@ -92,16 +102,25 @@ public class JmxUtil { } catch (InstanceNotFoundException e) { // The MBean is already down - the end state this call was asked to // bring about, not a failure, and no caller can act on it - log.debug(MessageFactory.get("jmxUtil.jmxUnregistrationFailed") + " " + oname); + log.debug(MessageFactory.get("jmxUtil.jmxAlreadyUnregistered") + " " + oname); return true; } catch (Exception e) { - // The MBean is still registered, and this name is now owned by + // Normally the MBean kept its name and that name is now owned by // nobody: say so, with the name, so an operator can take it down log.error(MessageFactory.get("jmxUtil.jmxUnregistrationRefused", oname), e); return false; } } + /* + * Concatenates base and keyprop into one object name, so the key + * property must carry its own leading comma and a value the ObjectName + * grammar accepts unquoted (or already quoted): a missing comma mis-parses + * into the base's last key or throws, and the concatenation validates + * neither - safe today because every caller passes an internal constant + * as the key property and a previously registered (so valid, non-pattern) + * name as the base. A null base is the documented skip: it returns null. + */ private static ObjectName getObjectName(ObjectName base, String keyprop) throws MalformedObjectNameException { if (base == null) { return null; diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/util/LocalStrings.properties b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/util/LocalStrings.properties index 8c80cdd60c..eb303a87bc 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/util/LocalStrings.properties +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/util/LocalStrings.properties @@ -153,9 +153,9 @@ genericNamingResourcesFactory.unableToSetProperty=Unable to set property [{0}] o jmxConnectionPool.droppingNotification=Dropping notification of type [{0}] for pool [{1}]: the notification delivery queue ({2} deliveries) is full -jmxUtil.jmxRegistrationFailed=Jmx registration failed. -jmxUtil.jmxUnregistrationFailed=Jmx unregistration failed. -jmxUtil.jmxUnregistrationRefused=Jmx unregistration of [{0}] was refused. The MBean is still registered under that name, and its caller releases the name regardless, so nothing will ever unregister it again: it keeps a live management surface and pins the component behind it for the life of the JVM. +jmxUtil.jmxRegistrationFailed=Jmx registration of [{2}] under base [{0}] and key property [{1}] failed. +jmxUtil.jmxAlreadyUnregistered=Jmx unregistration found no MBean under that name. +jmxUtil.jmxUnregistrationRefused=Jmx unregistration of [{0}] was refused. The MBean is normally still registered under that name, and its caller releases the name regardless, so nothing will ever unregister it again: it keeps a live management surface and pins the component behind it for the life of the JVM. poolProperties.invalidInterceptorProperty=Invalid interceptor property ''{0}'', missing ''='', in jdbcInterceptors value [{1}] poolProperties.credentialClearStripsStringEntry=Clearing the ''{0}'' credential also removed the entry the connectionProperties string contributes for that key. The string still names it, so every connection established from now on goes without that credential, and a connectionProperties reconfigure that re-names the key applies it again. Take the credential out of the connectionProperties string to retire it. diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxRegistration.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxRegistration.java index ffe85fa19b..e89eae76cd 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxRegistration.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxRegistration.java @@ -424,8 +424,8 @@ class TestJmxRegistration extends JdbcPoolBaseTest { server.registerMBean(mbean, name); try { Assertions.assertFalse(JmxUtil.unregisterJmx(name), - "an unregistration the MBean refused - a preDeregister() that throws, as a" - + " modeler wrapper's failing postDeregister() also produces - must" + "an unregistration the MBean refused - a preDeregister() that throws, the" + + " one failure besides the name simply being gone - must" + " come back as not done: the MBean is still registered under a" + " name its caller is about to forget, which is exactly the" + " failure the caller cannot see any other way, and it leaves a" --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
