jvz commented on code in PR #1136:
URL: https://github.com/apache/logging-log4j2/pull/1136#discussion_r1015853873


##########
log4j-api/src/main/java/org/apache/logging/log4j/util/Activator.java:
##########
@@ -101,64 +100,52 @@ private String toStateString(final int state) {
     }
 
     private void loadProvider(final BundleContext bundleContext, final 
BundleWiring bundleWiring) {
-        final String filter = "(APIVersion>=2.6.0)";
-        try {
-            final Collection<ServiceReference<Provider>> serviceReferences = 
bundleContext.getServiceReferences(Provider.class, filter);
-            Provider maxProvider = null;
-            for (final ServiceReference<Provider> serviceReference : 
serviceReferences) {
-                final Provider provider = 
bundleContext.getService(serviceReference);
-                if (maxProvider == null || provider.getPriority() > 
maxProvider.getPriority()) {
-                    maxProvider = provider;
-                }
-            }
-            if (maxProvider != null) {
-                ProviderUtil.addProvider(maxProvider);
+        final List<Provider> providers = loadProviders(bundleWiring);
+        if (!providers.isEmpty()) {
+            
ServiceRegistry.getInstance().registerBundleServices(Provider.class, 
bundleContext.getBundle().getBundleId(), providers);
+            if (hasLoggingSystemInitializationLock) {
+                LoggingSystem.getInstance().releaseInitializationLock();
+                hasLoggingSystemInitializationLock = false;
             }
-        } catch (final InvalidSyntaxException ex) {
-            LOGGER.error("Invalid service filter: " + filter, ex);
         }
-        final List<URL> urls = bundleWiring.findEntries("META-INF", 
"log4j-provider.properties", 0);
-        for (final URL url : urls) {
-            ProviderUtil.loadProvider(url, bundleWiring.getClassLoader());
+    }
+
+    private List<Provider> loadProviders(final BundleWiring bundleWiring) {
+        final List<Provider> providers = new ArrayList<>();
+        final ClassLoader classLoader = bundleWiring.getClassLoader();
+        final Iterator<Provider> iterator = ServiceLoader.load(Provider.class, 
classLoader).iterator();
+        while (iterator.hasNext()) {
+            try {
+                providers.add(iterator.next());
+            } catch (ServiceConfigurationError e) {
+                LOGGER.error("Unable to load Log4j Provider", e);
+            }
         }
+        return providers;
     }
 
     @Override
     public void start(final BundleContext bundleContext) throws Exception {
-        ProviderUtil.STARTUP_LOCK.lock();
-        lockingProviderUtil = true;
-        final BundleWiring self = 
bundleContext.getBundle().adapt(BundleWiring.class);
-        final List<BundleWire> required = 
self.getRequiredWires(LoggerContextFactory.class.getName());
-        for (final BundleWire wire : required) {
-            loadProvider(bundleContext, wire.getProviderWiring());
-        }
+        final LoggingSystem system = LoggingSystem.getInstance();
+        system.acquireInitializationLock();
+        hasLoggingSystemInitializationLock = true;
         bundleContext.addBundleListener(this);
-        final Bundle[] bundles = bundleContext.getBundles();
-        for (final Bundle bundle : bundles) {
-            loadProvider(bundle);
-        }
-        unlockIfReady();
-    }
-
-    private void unlockIfReady() {
-        if (lockingProviderUtil && !ProviderUtil.PROVIDERS.isEmpty()) {
-            ProviderUtil.STARTUP_LOCK.unlock();
-            lockingProviderUtil = false;
-        }

Review Comment:
   Turns out that all this logic was redundant since it's impossible (?) for a 
Log4j Provider to have been installed before installing `log4j-api` due to 
dependencies. Thus, we don't need to scan bundles for Log4j Provider services 
until after the API has been installed.
   
   In theory, you could try to upgrade the API or provider bundles at runtime, 
but that likely wouldn't work anyways, so there's not much point in scanning 
what's already installed here.
   
   Note that the unlocking logic has also been simplified to unlock once we 
actually register a `Provider` with `ServiceRegistry`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to