fixup: pr feedback

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>

Author: toddbaert

openfeature/provider/_registry.py

@@ -17,9 +17,7 @@
 if typing.TYPE_CHECKING:
     from openfeature._event_support import EventSupport
 
-# spec 1.8.4: a provider should not be bound to more than one OpenFeature API
-# instance simultaneously. We track the owning registry per provider; rebinding
-# to a different registry raises. WeakKeyDictionary lets providers be GC'd.
+# spec 1.8.4: provider must not bind to more than one API; we track owning registry per provider, rebinding raises. WeakKeyDictionary lets providers be GC'd
 _binding_lock = threading.Lock()
 _provider_bindings: weakref.WeakKeyDictionary[FeatureProvider, ProviderRegistry] = (
     weakref.WeakKeyDictionary()
@@ -221,11 +219,8 @@ def _run_initialize(
     def _shutdown_if_unused(self, provider: FeatureProvider) -> None:
         # only shut down if no longer referenced. shutdown runs on a daemon
         # thread so a hanging shutdown() cannot block the caller.
-        with self._lock:
-            if provider is self._default_provider:
-                return
-            if provider in self._providers.values():
-                return
+        if self._is_active(provider):
+            return
 
         thread = threading.Thread(
             target=self._shutdown_provider,
@@ -235,20 +230,25 @@ def _shutdown_if_unused(self, provider: FeatureProvider) -> None:
         )
         thread.start()
 
+    def _is_active(self, provider: FeatureProvider) -> bool:
+        with self._lock:
+            return (
+                provider is self._default_provider
+                or provider in self._providers.values()
+            )
+
     def _shutdown_provider(
         self, provider: FeatureProvider, abort_if_re_registered: bool = False
     ) -> None:
         try:
+            # abort if re-registered before shutdown() to avoid tearing down the freshly-registered instance
+            if abort_if_re_registered and self._is_active(provider):
+                return
             if hasattr(provider, "shutdown"):
                 provider.shutdown()
-            # if provider is being re-registered, leave its status and event wiring intact
-            if abort_if_re_registered:
-                with self._lock:
-                    if (
-                        provider is self._default_provider
-                        or provider in self._providers.values()
-                    ):
-                        return
+            # abort if re-registered during shutdown(); leave status and event wiring intact
+            if abort_if_re_registered and self._is_active(provider):
+                return
             with self._lock:
                 self._provider_status.pop(provider, None)
         except Exception as err:

tests/provider/test_registry.py

@@ -433,3 +433,30 @@ def slow_shutdown():
         "stale shutdown of A clobbered the fresh registration's status"
     )
     provider_a.detach.assert_not_called()
+
+
+def test_stale_shutdown_skips_shutdown_if_re_registered_first():
+    """If a provider is re-registered before its background shutdown gets to
+    call shutdown() at all, shutdown() must not be invoked on the active
+    provider."""
+
+    registry = make_registry()
+
+    provider_a = Mock()
+    provider_b = Mock()
+
+    # step 1: register A, replace with B, then re-register A. queued background shutdown of A from the A->B swap is racing
+    registry.set_provider("domain", provider_a, wait_for_init=True)
+    registry.set_provider("domain", provider_b, wait_for_init=True)
+    registry.set_provider("domain", provider_a, wait_for_init=True)
+    # let the natural A->B background shutdown complete before we assert
+    time.sleep(0.2)
+    provider_a.shutdown.reset_mock()
+    provider_a.detach.reset_mock()
+
+    # step 2: simulate the late-arriving stale shutdown; abort check must short-circuit before shutdown() is called
+    registry._shutdown_provider(provider_a, abort_if_re_registered=True)
+
+    provider_a.shutdown.assert_not_called()
+    provider_a.detach.assert_not_called()
+    assert registry.get_provider_status(provider_a) == ProviderStatus.READY