You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by cs...@apache.org on 2026/09/26 12:17:21 UTC
(maven-resolver) branch master updated: Use structured logging in the IPC server (#2157)
This is an automated email from the ASF dual-hosted git repository.
cstamas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git
The following commit(s) were added to refs/heads/master by this push:
new 26b4de33b Use structured logging in the IPC server (#2157)
26b4de33b is described below
commit 26b4de33b03cab608ce970b741b928537f649ab2
Author: Efe <go...@gmail.com>
AuthorDate: Sat Sep 26 15:17:17 2026 +0300
Use structured logging in the IPC server (#2157)
## Summary
Fixes #1989 by routing `IpcServer` diagnostics through SLF4J instead of writing directly to `System.out`.
## Changes
- Add an SLF4J logger to `IpcServer`.
- Route debug, informational, and error messages through the logger.
- Preserve throwable details for error diagnostics.
## Testing
- `mvn -pl maven-resolver-named-locks-ipc -am -DskipITs -DskipTests=false -Dspotless.apply.skip=true -Dspotless.check.skip=true test` — PASS (reactor build; all tests passed).
- `git diff --check` — PASS.
- Spotless formatting was not run because the repository configured Palantir formatter is incompatible with the available JDK 27 runtime; the source follows existing formatting and the build Checkstyle and RAT checks passed.
## Notes
- No overlapping open PR was found for issue #1989 or the IpcServer logging change during the final duplicate check.
- The commit is signed and includes the required DCO sign-off.
- AI assistance was used to prepare this change; the submitter has reviewed the implementation and test results.
Signed-off-by: Efe Gökdemir <ef...@rexcode.co.uk>
* Preserve forked IPC diagnostics
* test: bound forked IPC diagnostics test
* fix: clean up IPC server shutdown state
* test: always close forked IPC client
---------
Signed-off-by: Efe Gökdemir <ef...@rexcode.co.uk>
---
.../org/eclipse/aether/named/ipc/IpcClient.java | 9 ++-
.../org/eclipse/aether/named/ipc/IpcServer.java | 43 ++++++++++---
.../aether/named/ipc/IpcClientForkedTest.java | 72 ++++++++++++++++++++++
3 files changed, 111 insertions(+), 13 deletions(-)
diff --git a/maven-resolver-named-locks-ipc/src/main/java/org/eclipse/aether/named/ipc/IpcClient.java b/maven-resolver-named-locks-ipc/src/main/java/org/eclipse/aether/named/ipc/IpcClient.java
index 191a723f5..bc2bee449 100644
--- a/maven-resolver-named-locks-ipc/src/main/java/org/eclipse/aether/named/ipc/IpcClient.java
+++ b/maven-resolver-named-locks-ipc/src/main/java/org/eclipse/aether/named/ipc/IpcClient.java
@@ -375,10 +375,13 @@ public class IpcClient {
}
void close() {
- if (noFork) {
- stopServer();
+ try {
+ if (noFork) {
+ stopServer();
+ }
+ } finally {
+ close(new IOException("Closing"));
}
- close(new IOException("Closing"));
}
synchronized void close(Throwable e) {
diff --git a/maven-resolver-named-locks-ipc/src/main/java/org/eclipse/aether/named/ipc/IpcServer.java b/maven-resolver-named-locks-ipc/src/main/java/org/eclipse/aether/named/ipc/IpcServer.java
index ef8a9c2b5..603173966 100644
--- a/maven-resolver-named-locks-ipc/src/main/java/org/eclipse/aether/named/ipc/IpcServer.java
+++ b/maven-resolver-named-locks-ipc/src/main/java/org/eclipse/aether/named/ipc/IpcServer.java
@@ -39,6 +39,8 @@ import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
+import org.slf4j.LoggerFactory;
+
/**
* Implementation of the server side.
* The server instance is bound to a given maven repository.
@@ -46,6 +48,8 @@ import java.util.concurrent.atomic.AtomicInteger;
* @since 2.0.1
*/
public class IpcServer {
+ static volatile boolean forkedProcess;
+
/**
* Should the IPC server not fork? (i.e. for testing purposes)
*
@@ -162,6 +166,7 @@ public class IpcServer {
}
public static void main(String[] args) throws Exception {
+ forkedProcess = true;
// When spawning a new process, the child process is create within
// the same process group. This means that a few signals are sent
// to the whole group. This is the case for SIGINT (Ctrl-C) and
@@ -213,17 +218,33 @@ public class IpcServer {
private static void debug(String msg, Object... args) {
if (DEBUG) {
- System.out.printf("[ipc] [debug] " + msg + "\n", args);
+ if (forkedProcess) {
+ System.out.println("[ipc] [debug] " + format(msg, args));
+ } else {
+ LoggerFactory.getLogger(IpcServer.class).debug(msg, args);
+ }
}
}
private static void info(String msg, Object... args) {
- System.out.printf("[ipc] [info] " + msg + "\n", args);
+ if (forkedProcess) {
+ System.out.println("[ipc] [info] " + format(msg, args));
+ } else {
+ LoggerFactory.getLogger(IpcServer.class).info(msg, args);
+ }
}
private static void error(String msg, Throwable t) {
- System.out.println("[ipc] [error] " + msg);
- t.printStackTrace(System.out);
+ if (forkedProcess) {
+ System.out.println("[ipc] [error] " + msg);
+ t.printStackTrace(System.out);
+ } else {
+ LoggerFactory.getLogger(IpcServer.class).error(msg, t);
+ }
+ }
+
+ private static String format(String msg, Object... args) {
+ return String.format(msg.replace("{}", "%s"), args);
}
private static void run(Runnable runnable, boolean daemon) {
@@ -240,7 +261,7 @@ public class IpcServer {
public void run() {
try {
- info("IpcServer started at %s", getLocalAddress().toString());
+ info("IpcServer started at {}", getLocalAddress().toString());
use();
run(this::expirationCheck, true);
while (!closing) {
@@ -251,6 +272,8 @@ public class IpcServer {
if (!closing) {
error("Error running sync server loop", t);
}
+ } finally {
+ forkedProcess = false;
}
}
@@ -260,7 +283,7 @@ public class IpcServer {
clients.put(socket, Thread.currentThread());
c = clients.size();
}
- info("New client connected (%d connected)", c);
+ info("New client connected ({} connected)", c);
use();
Map<String, Context> clientContexts = new ConcurrentHashMap<>();
try {
@@ -295,7 +318,7 @@ public class IpcServer {
contexts.put(context.id, context);
clientContexts.put(context.id, context);
synchronized (output) {
- debug("Created context %s", context.id);
+ debug("Created context {}", context.id);
output.writeInt(requestId);
output.writeInt(2);
output.writeUTF(IpcMessages.RESPONSE_CONTEXT);
@@ -331,7 +354,7 @@ public class IpcServer {
contexts.remove(contextId);
context.unlock();
synchronized (output) {
- debug("Closing context %s", context.id);
+ debug("Closing context {}", context.id);
output.writeInt(requestId);
output.writeInt(1);
output.writeUTF(IpcMessages.RESPONSE_CLOSE);
@@ -384,7 +407,7 @@ public class IpcServer {
c = clients.size();
}
if (!closing) {
- info("%d clients remained", c);
+ info("{} clients remained", c);
}
}
}
@@ -392,7 +415,7 @@ public class IpcServer {
private void sendAcquireResponse(DataOutputStream output, SocketChannel socket, int requestId, Context context) {
try {
synchronized (output) {
- debug("Locking in context %s", context.id);
+ debug("Locking in context {}", context.id);
output.writeInt(requestId);
output.writeInt(1);
output.writeUTF(IpcMessages.RESPONSE_ACQUIRE);
diff --git a/maven-resolver-named-locks-ipc/src/test/java/org/eclipse/aether/named/ipc/IpcClientForkedTest.java b/maven-resolver-named-locks-ipc/src/test/java/org/eclipse/aether/named/ipc/IpcClientForkedTest.java
new file mode 100644
index 000000000..1390d7ed1
--- /dev/null
+++ b/maven-resolver-named-locks-ipc/src/test/java/org/eclipse/aether/named/ipc/IpcClientForkedTest.java
@@ -0,0 +1,72 @@
+/*
+ * 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.eclipse.aether.named.ipc;
+
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.time.Duration;
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+import org.junit.jupiter.api.io.TempDir;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class IpcClientForkedTest {
+
+ @Test
+ @Timeout(30)
+ void forkedServerDiagnosticsAreWrittenToLog(@TempDir Path tempDir) throws Exception {
+ Path logPath = tempDir.resolve("log");
+ IpcClient client = new IpcClient(tempDir.resolve("repository"), logPath, null);
+ try {
+ client.ensureInitialized();
+
+ long deadline = System.nanoTime() + Duration.ofSeconds(5).toNanos();
+ String logOutput = "";
+ while (System.nanoTime() < deadline) {
+ List<Path> logFiles;
+ try (var paths = Files.list(logPath)) {
+ logFiles = paths.toList();
+ }
+ if (!logFiles.isEmpty()) {
+ logOutput = Files.readString(logFiles.get(0), StandardCharsets.UTF_8);
+ if (logOutput.contains("IpcServer started at")) {
+ break;
+ }
+ }
+ Thread.sleep(10);
+ }
+
+ assertTrue(
+ logOutput.contains("IpcServer started at"),
+ "expected forked server startup diagnostics in log, but was: " + logOutput);
+ } finally {
+ try {
+ if (client.initialized) {
+ client.stopServer();
+ }
+ } finally {
+ client.close();
+ }
+ }
+ }
+}