gnodet-bot commented on code in PR #12694: URL: https://github.com/apache/maven/pull/12694#discussion_r4079537516
########## impl/maven-logging/src/main/java/org/apache/maven/slf4j/MavenJulHandler.java: ########## @@ -0,0 +1,318 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.slf4j; + +import java.text.MessageFormat; +import java.util.MissingResourceException; +import java.util.ResourceBundle; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.logging.Handler; +import java.util.logging.Level; +import java.util.logging.LogManager; +import java.util.logging.LogRecord; +import java.util.logging.Logger; + +import org.slf4j.LoggerFactory; +import org.slf4j.spi.LocationAwareLogger; + +/** + * A JUL {@link Handler} that routes {@code java.util.logging} events into + * Maven's structured logging pipeline, preserving the rich {@link LogRecord} + * metadata that the standard {@code SLF4JBridgeHandler} silently drops + * (source class name, source method name, thread ID). + * <p> + * All JUL events are routed through SLF4J so that {@link MavenSimpleLogger} + * produces a consistent {@code formattedMessage} (with timestamp, logger name, + * and ANSI styling) regardless of the event's origin. The JUL metadata is + * stashed in a thread-local <em>before</em> the SLF4J call so that downstream + * consumers (e.g. {@code ProjectBuildLogAppender}) can read it when + * constructing a structured {@code LogEvent}. + * <p> + * Usage — replace the standard SLF4J bridge in {@code LookupInvoker}: + * <pre> + * MavenJulHandler.install(); + * </pre> + * + * @since 4.1.0 + * @see #install() + * @see #getJulMetadata() + */ +public class MavenJulHandler extends Handler { + + /** + * JUL metadata captured from a {@link LogRecord} that would otherwise + * be lost when bridging to SLF4J. + * + * @param sourceClassName the source class, or {@code null} + * @param sourceMethodName the source method, or {@code null} + * @param threadId the originating thread ID + */ + public record JulMetadata(String sourceClassName, String sourceMethodName, long threadId) {} + + private static final ThreadLocal<JulMetadata> METADATA = new ThreadLocal<>(); + + /** + * Private SLF4J logger cache using {@link ConcurrentMap#putIfAbsent} + * instead of {@link ConcurrentMap#computeIfAbsent}. This avoids the + * {@code ConcurrentHashMap.computeIfAbsent} reentrancy bug + * ({@code IllegalStateException("Recursive update")}) that occurs + * when a JUL event fires during SLF4J logger initialization: the + * handler's {@code publish()} calls {@code LoggerFactory.getLogger()}, + * which internally uses {@code computeIfAbsent}, and if that triggers + * another JUL event whose logger name hashes to the same bucket, + * {@code ConcurrentHashMap} throws. {@code putIfAbsent} is safe + * against reentrancy — worst case, two threads create the same + * logger and one is discarded. + */ + private static final ConcurrentMap<String, org.slf4j.Logger> LOGGER_CACHE = new ConcurrentHashMap<>(); + Review Comment: **[low] `LOGGER_CACHE` grows unbounded — potential memory pressure in long-running daemon processes.** `LOGGER_CACHE` is a `static final ConcurrentHashMap` with no eviction. In a Maven daemon that runs hundreds of builds, JUL-emitting frameworks with many named loggers (e.g. Netty, gRPC, Hibernate in complex builds) will populate this cache indefinitely. Each entry is small (string key + SLF4J logger wrapper), but it can accumulate. Not blocking for an initial implementation, but worth a bounded `LinkedHashMap`-based LRU or a note in the Javadoc that the cache is intentionally unbounded (size bounded by distinct JUL logger names seen across all builds in the daemon process). Post-MVP cleanup candidate. -- 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]
