Hold read lock when reading LoggerContextTargetMap

There was one place the map was read without a read lock.
This change also removes one map lookup when getting a logger
with a logger class type.

bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/debug/Debug.java

@@ -220,6 +220,13 @@ public static void println(String x) {
 	public void trace(String topic, String message) {
 		LogService current = logService;
 		if (current != null) {
+			// Note that the logger is not cached.
+			// Attempts to cache the logger have not
+			// shown significant performance improvement.
+			// The LogService does cache instances with a
+			// non-blocking read lock. Any caching here
+			// will need to be proven to be faster than
+			// the LogService logger cache.
 			current.getLogger(topic).trace(message);
 		} else {
 			out.println(message);

bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/log/ExtendedLogServiceFactory.java

@@ -209,9 +209,14 @@ public Map<String, LogLevel> getLogLevels() {
 		}
 
 		Map<String, LogLevel> getEffectiveLogLevels() {
-			Map<String, LogLevel> effectiveLogLevels = loggerContextTargetMap.getRootLoggerContext().getLogLevels();
-			effectiveLogLevels.putAll(getLogLevels());
-			return effectiveLogLevels;
+			contextsLock.readLock().lock();
+			try {
+				Map<String, LogLevel> effectiveLogLevels = loggerContextTargetMap.getRootLoggerContext().getLogLevels();
+				effectiveLogLevels.putAll(getLogLevels());
+				return effectiveLogLevels;
+			} finally {
+				contextsLock.readLock().unlock();
+			}
 		}
 
 		@Override
@@ -229,6 +234,7 @@ public void setLogLevels(Map<String, LogLevel> logLevels) {
 				} finally {
 					contextsLock.writeLock().unlock();
 				}
+				// Note that the readlock is still held here
 				systemBundleLoggerContext = loggerContextTargetMap.applyLogLevels(this);
 			} finally {
 				if (readLocked) {

bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/log/ExtendedLogServiceImpl.java

@@ -25,14 +25,13 @@ public class ExtendedLogServiceImpl implements ExtendedLogService {
 
 	private final ExtendedLogServiceFactory factory;
 	private volatile Bundle bundle;
-	private final Map<Class<? extends org.osgi.service.log.Logger>, Map<String, LoggerImpl>> loggerCache = new HashMap<>();
+	private final Map<String, LoggerImpl> loggerClassLoggers = new HashMap<>();
+	private final Map<String, LoggerImpl> formatterLoggerClassLoggers = new HashMap<>();
 	private static final String LOG_SERVICE = "LogService"; //$NON-NLS-1$
 
 	public ExtendedLogServiceImpl(ExtendedLogServiceFactory factory, Bundle bundle) {
 		this.factory = factory;
 		this.bundle = bundle;
-		loggerCache.put(org.osgi.service.log.Logger.class, new HashMap<>());
-		loggerCache.put(org.osgi.service.log.FormatterLogger.class, new HashMap<>());
 	}
 
 	@SuppressWarnings("deprecation")
@@ -141,18 +140,25 @@ public <L extends org.osgi.service.log.Logger> L getLogger(String name, Class<L>
 		}
 		LoggerImpl logger = null;
 		Map<String, LoggerImpl> loggers = null;
+		LoggerContext loggerContext = null;
 		factory.contextsLock.readLock().lock();
 		try {
-			loggers = loggerCache.get(loggerType);
-			if (loggers == null) {
+			if (org.osgi.service.log.Logger.class.equals(loggerType)) {
+				loggers = loggerClassLoggers;
+			} else if (org.osgi.service.log.FormatterLogger.class.equals(loggerType)) {
+				loggers = formatterLoggerClassLoggers;
+			} else {
 				throw new IllegalArgumentException(loggerType.getName());
 			}
 			logger = loggers.get(name);
+			if (logger == null) {
+				// get the loggerContext with the read lock
+				loggerContext = factory.loggerContextTargetMap.getEffectiveLoggerContext(bundle);
+			}
 		} finally {
 			factory.contextsLock.readLock().unlock();
 		}
 		if (logger == null) {
-			LoggerContext loggerContext = factory.loggerContextTargetMap.getEffectiveLoggerContext(bundle);
 			if (loggerType == FormatterLogger.class) {
 				logger = new FormatterLoggerImpl(this, name, loggerContext);
 			} else if (loggerType == org.osgi.service.log.Logger.class) {
@@ -351,10 +357,7 @@ public <E extends Exception> void error(LoggerConsumer<E> consumer) throws E {
 	}
 
 	void applyLogLevels(EquinoxLoggerContext effectiveLoggerContext) {
-		for (Map<String, LoggerImpl> loggers : loggerCache.values()) {
-			for (LoggerImpl logger : loggers.values()) {
-				logger.applyLoggerContext(effectiveLoggerContext);
-			}
-		}
+		loggerClassLoggers.forEach((s, l) -> l.applyLoggerContext(effectiveLoggerContext));
+		formatterLoggerClassLoggers.forEach((s, l) -> l.applyLoggerContext(effectiveLoggerContext));
 	}
 }