ramanathan1504 opened a new issue, #4241: URL: https://github.com/apache/logging-log4j2/issues/4241
## Description `AbstractDatabaseManager` consults `isRunning()` on the shutdown path but not on the write path. The consequence is that a manager whose `startupInternal()` failed does the opposite of the safe thing in both directions: - **It still accepts writes.** `write()` has no `isRunning()` check, so it proceeds to `writeThrough()` → `writeInternal()` regardless. Subclass state that `startupInternal()` never got to assign is therefore dereferenced on every event, typically as an NPE that names an internal field rather than the failure that caused it. - **It is never shut down.** `shutdown()` *is* guarded by `if (this.isRunning())`, so `shutdownInternal()` never runs. Anything `startupInternal()` managed to construct before it threw is leaked, and `shutdown()` still returns `true`. `startup()` catches everything and logs `"Could not perform database startup operations"` once, leaving `running = false`: https://github.com/apache/logging-log4j2/blob/2.x/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/AbstractDatabaseManager.java ```java public final synchronized void startup() { if (!this.isRunning()) { try { this.startupInternal(); this.running = true; } catch (final Exception e) { logError("Could not perform database startup operations", e); // running stays false } } } public final synchronized void write(final LogEvent event, final Serializable serializable) { if (isBuffered()) { // no isRunning() check buffer(event); } else { writeThrough(event, serializable); } } public final synchronized boolean shutdown() { boolean closed = true; this.flush(); if (this.isRunning()) { // guarded, so a failed-startup manager is never cleaned up try { closed &= this.shutdownInternal(); ... } return closed; // true, having done nothing } ``` The practical effect is that the one status line naming the real cause is emitted once and then buried under one exception per logged event, while whatever the manager half-built stays open for the life of the JVM. This is not specific to any one subclass — it is the shape of every `AbstractDatabaseManager` whose startup fails, so it applies to the JDBC, JPA and NoSQL managers alike. `CassandraManager` is where I hit it, and there the leak is severe enough to prevent JVM exit; I have filed that separately as the concrete case. ## Configuration **Version:** 2.x @ `04c93c1d33` (also present on `main` — `startup()`, `write()` and `shutdown()` are unchanged in that respect, so 3.x is affected too) **Operating system:** macOS 15 (Darwin 25.5.0) **JDK:** Temurin 21 ## Logs The real cause, logged exactly once: ``` main ERROR CassandraManager Cassandra Could not perform database startup operations: com.datastax.driver.core.exceptions.InvalidQueryException: Keyspace 'log4j' does not exist at com.datastax.driver.core.Cluster.connect(Cluster.java:340) at org.apache.logging.log4j.cassandra.CassandraManager.startupInternal(CassandraManager.java:78) at org.apache.logging.log4j.core.appender.db.AbstractDatabaseManager.startup(AbstractDatabaseManager.java:259) at org.apache.logging.log4j.core.appender.db.AbstractDatabaseAppender.start(AbstractDatabaseAppender.java:183) ``` and then, for every subsequent event, an exception that names none of it: ``` main ERROR Unable to write to database [Cassandra] for appender [Cassandra]. java.lang.NullPointerException: Cannot invoke "com.datastax.driver.core.PreparedStatement.bind(Object[])" because "this.preparedStatement" is null main ERROR An exception occurred processing Appender Cassandra org.apache.logging.log4j.core.appender.AppenderLoggingException: Unable to write to database in appender: ... ``` ## Reproduction Any `AbstractDatabaseManager` subclass whose `startupInternal()` throws will show it; no database is needed to demonstrate the control flow: ```java final class FailingManager extends AbstractDatabaseManager { private Object resource; // assigned by startupInternal, so null on failure FailingManager() { super("test", 0); } @Override protected void startupInternal() throws Exception { resource = new Object(); throw new IllegalStateException("boom"); // startup() swallows this } @Override protected boolean shutdownInternal() { resource = null; return true; } @Override protected void connectAndStart() { } @Override protected void writeInternal(LogEvent e, Serializable s) { resource.toString(); // reached despite running == false } @Override protected boolean commitAndClose() { return true; } } manager.startup(); // logs the real cause, running == false manager.write(event, null); // NPE per event, naming `resource` manager.shutdown(); // returns true, shutdownInternal() never called ``` ## Suggested fix Move the guard to the method that needs it, on both paths: - `write()` (and `flush()`/`buffer()`) should return early when `!isRunning()`, ideally logging once rather than per event, so the startup failure remains the visible error. - `shutdown()` should attempt `shutdownInternal()` regardless of `running`, since `startupInternal()` may have acquired resources before throwing. Subclasses would need to tolerate partially-initialised state, which is a smaller change than it sounds — most already null-check. Happy to put a PR together if the approach looks right. -- 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]
