[java-shib-shared] 01/02: JSSH-71 Remove the impact of the DestructableComponent Interface

Codeberg noreply at shibboleth.net
Tue Jul 7 15:51:21 UTC 2026


This is an automated email from the git hooks/post-receive script.

codeberg pushed a commit to branch dev/JSSH-71
in repository java-shib-shared.

View the commit online:
https://codeberg.org/Shibboleth/java-shib-shared/commit/5fe42d37a126dc0b932e9c1863f3dc1d1e5a225d

commit 5fe42d37a126dc0b932e9c1863f3dc1d1e5a225d
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Mon Jul 6 19:29:06 2026 +0100

    JSSH-71 Remove the impact of the DestructableComponent Interface
    
    https://shibboleth.atlassian.net/browse/JSSH-71
    
    Remove doDestroy methods that do something and replace with annotation.
    
    The exception is AbstractReloadableService.  See case notes for why.
---
 .../shared/security/impl/BasicKeystoreKeyStrategy.java   | 13 +++++++++----
 .../shared/security/impl/ScriptedKeyStrategy.java        | 13 +++++++++----
 .../shared/service/AbstractReloadableService.java        |  8 +++++++-
 .../spring/service/AbstractServiceableComponent.java     |  1 +
 .../shared/spring/service/ReloadableSpringService.java   | 16 +++++++++-------
 5 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/shib-security/src/main/java/net/shibboleth/shared/security/impl/BasicKeystoreKeyStrategy.java b/shib-security/src/main/java/net/shibboleth/shared/security/impl/BasicKeystoreKeyStrategy.java
index 7f8aea78..aba2a71b 100644
--- a/shib-security/src/main/java/net/shibboleth/shared/security/impl/BasicKeystoreKeyStrategy.java
+++ b/shib-security/src/main/java/net/shibboleth/shared/security/impl/BasicKeystoreKeyStrategy.java
@@ -36,6 +36,7 @@ import javax.crypto.SecretKey;
 
 import org.slf4j.Logger;
 
+import net.shibboleth.shared.annotation.OnTeardown;
 import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
 import net.shibboleth.shared.collection.Pair;
@@ -270,9 +271,11 @@ public class BasicKeystoreKeyStrategy extends AbstractInitializableComponent imp
         }
     }
 
-    /** {@inheritDoc} */
-    @Override
-    protected void doDestroy() {
+    /**
+     * Tear down the bean when the ServiceableComponent it is part of is unloaded.
+     */
+    @OnTeardown
+    public final void teardownBasicKeystoreKeyStrategy() {
         if (updateTask != null) {
             updateTask.cancel();
             updateTask = null;
@@ -281,7 +284,6 @@ public class BasicKeystoreKeyStrategy extends AbstractInitializableComponent imp
             }
             internalTaskTimer = null;
         }
-        super.doDestroy();
     }
 
     /** {@inheritDoc} */
@@ -294,6 +296,9 @@ public class BasicKeystoreKeyStrategy extends AbstractInitializableComponent imp
     /** {@inheritDoc} */
     @Nonnull public NamedKey getDefaultKeyRecord() throws KeyException {
         checkComponentActive();
+        if (updateTask == null) {
+            log.warn("Called getDefaultKeyRecord in a torn down KeyStoreStrategy");
+        }
         
         synchronized(this) {
             if (defaultKey != null) {
diff --git a/shib-security/src/main/java/net/shibboleth/shared/security/impl/ScriptedKeyStrategy.java b/shib-security/src/main/java/net/shibboleth/shared/security/impl/ScriptedKeyStrategy.java
index 127dbfc7..f6a2d5ce 100644
--- a/shib-security/src/main/java/net/shibboleth/shared/security/impl/ScriptedKeyStrategy.java
+++ b/shib-security/src/main/java/net/shibboleth/shared/security/impl/ScriptedKeyStrategy.java
@@ -30,6 +30,7 @@ import javax.script.SimpleScriptContext;
 
 import org.slf4j.Logger;
 
+import net.shibboleth.shared.annotation.OnTeardown;
 import net.shibboleth.shared.annotation.constraint.NonNegative;
 import net.shibboleth.shared.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.shared.annotation.constraint.NotEmpty;
@@ -193,9 +194,11 @@ public class ScriptedKeyStrategy extends AbstractInitializableComponent implemen
         }
     }
 
-    /** {@inheritDoc} */
-    @Override
-    protected void doDestroy() {
+    /**
+     * Tear down the bean when the ServiceableComponent it is part of is unloaded.
+     */
+    @OnTeardown
+    public final void teardownScriptedKeyStrategy() {
         if (updateTask != null) {
             updateTask.cancel();
             updateTask = null;
@@ -204,7 +207,6 @@ public class ScriptedKeyStrategy extends AbstractInitializableComponent implemen
             }
             internalTaskTimer = null;
         }
-        super.doDestroy();
     }
 
 
@@ -217,6 +219,9 @@ public class ScriptedKeyStrategy extends AbstractInitializableComponent implemen
     /** {@inheritDoc} */
     @Nonnull public NamedKey getDefaultKeyRecord() throws KeyException {
         checkComponentActive();
+        if (updateTask == null) {
+            log.warn("Called getDefaultKeyRecord in a torn down KeyStoreStrategy");
+        }
 
         synchronized(this) {
             if (defaultKey != null) {
diff --git a/shib-service/src/main/java/net/shibboleth/shared/service/AbstractReloadableService.java b/shib-service/src/main/java/net/shibboleth/shared/service/AbstractReloadableService.java
index 0fe1b2d0..50ca7bb1 100644
--- a/shib-service/src/main/java/net/shibboleth/shared/service/AbstractReloadableService.java
+++ b/shib-service/src/main/java/net/shibboleth/shared/service/AbstractReloadableService.java
@@ -29,6 +29,7 @@ import net.shibboleth.shared.component.AbstractIdentifiableInitializableComponen
 import net.shibboleth.shared.component.ComponentInitializationException;
 import net.shibboleth.shared.component.UnmodifiableComponent;
 import net.shibboleth.shared.logic.Constraint;
+import net.shibboleth.shared.primitive.AnnotationsSupport;
 import net.shibboleth.shared.primitive.LoggerFactory;
 import net.shibboleth.shared.primitive.TimerSupport;
 
@@ -203,8 +204,13 @@ public abstract class AbstractReloadableService<T> extends AbstractIdentifiableI
     }
 
     /** {@inheritDoc} */
-    @Override protected void doDestroy() {
+    @Override @Deprecated protected void doDestroy() {
+        /* V6 code note.  This is a "top level object so the annotation serves no purpose
+         * (it wont be inside another service).  The V6 implementation could implement
+         * Disposable bean (and override destroy).
+         */
         log.info("{} Starting shutdown", getLogPrefix());
+        AnnotationsSupport.callOnTeardownAnnotations(this);
         if (reloadTask != null) {
             reloadTask.cancel();
             reloadTask = null;
diff --git a/shib-service/src/main/java/net/shibboleth/shared/spring/service/AbstractServiceableComponent.java b/shib-service/src/main/java/net/shibboleth/shared/spring/service/AbstractServiceableComponent.java
index 1c5ca2e8..ea96897a 100644
--- a/shib-service/src/main/java/net/shibboleth/shared/spring/service/AbstractServiceableComponent.java
+++ b/shib-service/src/main/java/net/shibboleth/shared/spring/service/AbstractServiceableComponent.java
@@ -154,6 +154,7 @@ public abstract class AbstractServiceableComponent<T> extends AbstractIdentifiab
      * unloaded, but we do the unload here so that error cases also clean up.
      */
     @Override protected void doDestroy() {
+        /* V6 code note.  This is  "belt and suspenders" and can be removed. */
         unloadComponent();
         super.doDestroy();
     }
diff --git a/shib-service/src/main/java/net/shibboleth/shared/spring/service/ReloadableSpringService.java b/shib-service/src/main/java/net/shibboleth/shared/spring/service/ReloadableSpringService.java
index 88c934ec..df851948 100644
--- a/shib-service/src/main/java/net/shibboleth/shared/spring/service/ReloadableSpringService.java
+++ b/shib-service/src/main/java/net/shibboleth/shared/spring/service/ReloadableSpringService.java
@@ -36,6 +36,7 @@ import org.springframework.context.support.GenericApplicationContext;
 import org.springframework.core.convert.ConversionService;
 import org.springframework.core.io.Resource;
 
+import net.shibboleth.shared.annotation.OnTeardown;
 import net.shibboleth.shared.annotation.ParameterName;
 import net.shibboleth.shared.collection.CollectionSupport;
 import net.shibboleth.shared.component.ComponentInitializationException;
@@ -402,17 +403,18 @@ public class ReloadableSpringService<T> extends AbstractReloadableService<T> imp
         log.info("{} Reload complete", getLogPrefix());
     }
 
-    /** {@inheritDoc} */
-    @Override protected void doDestroy() {
+    /**
+     * Tear down (for the last time) the component we are reloading.
+     *
+     * This is called when the parent is torn down (by some tbd mechanism).
+     */
+    @OnTeardown
+    public final synchronized void teardownReloadableSpringService() {
         final AbstractServiceableComponent<T> oldComponent = cachedComponent;
         cachedComponent = null;
-        // And tear down. Note that we are synchronized on this right now
-        // and this will grab the lock - but that is OK because the ranking
-        // is to lock this object, then the ServicableComponent.
-        if (null != oldComponent) {
+        if (oldComponent != null) {
             oldComponent.unloadComponent();
         }
-        super.doDestroy();
     }
 
     /**

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list