gnodet-bot commented on code in PR #11102: URL: https://github.com/apache/maven/pull/11102#discussion_r4258645751
########## impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/ProcessRuns.java: ########## @@ -0,0 +1,237 @@ +/* + * 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.cling.invoker; + +import java.io.IOException; +import java.io.Reader; +import java.io.Writer; +import java.nio.charset.StandardCharsets; +import java.nio.file.AtomicMoveNotSupportedException; +import java.nio.file.DirectoryStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; +import java.time.Duration; +import java.time.Instant; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.List; +import java.util.Optional; +import java.util.Properties; + +/** + * Registry of the running Maven processes of the current user, kept as one descriptor file per process. + * All the operations are best effort: a registry which cannot be read or written never fails a build. + */ +public final class ProcessRuns { + + private static final String RUNS_SUBDIR = ".m2/.maven/runs"; + private static final String FILE_PREFIX = "mvn-"; + private static final String FILE_SUFFIX = ".properties"; + + private static final String KEY_PID = "pid"; + private static final String KEY_VERSION = "version"; + private static final String KEY_WORKDIR = "workDir"; + private static final String KEY_EXECROOT = "execRoot"; + private static final String KEY_STARTED = "started"; + + /** The start time of a process is not reported with the same precision all the time. */ + private static final Duration START_TOLERANCE = Duration.ofSeconds(2); + + private ProcessRuns() {} + + private static Path defaultRunsDir() { + return Paths.get(System.getProperty("user.home", ".")).resolve(RUNS_SUBDIR); + } + + private static Path descriptor(Path runsDir, long pid) { + return runsDir.resolve(FILE_PREFIX + pid + FILE_SUFFIX); + } + + /** + * Registers the current process. + */ + public static void install(String version, Path workDir, Path execRoot) { + ProcessHandle current = ProcessHandle.current(); + install( + defaultRunsDir(), + current.pid(), + current.info().startInstant().orElseGet(Instant::now), + version, + workDir, + execRoot); + } + + /** + * Removes the registration of the process. + */ + public static void uninstall(long pid) { + uninstall(defaultRunsDir(), pid); + } + + /** + * Describes the registered processes which are still running. + * + * @return a table of the processes, one line per process, without any trailing line separator + */ + public static String listAlive() { + return format(listAlive(defaultRunsDir())); + } + + static void install(Path runsDir, long pid, Instant started, String version, Path workDir, Path execRoot) { + try { + Files.createDirectories(runsDir); + Properties properties = new Properties(); + properties.setProperty(KEY_PID, Long.toString(pid)); + properties.setProperty(KEY_VERSION, version == null ? "-" : version); + properties.setProperty( + KEY_WORKDIR, + workDir == null ? "?" : workDir.toAbsolutePath().toString()); + properties.setProperty( + KEY_EXECROOT, + execRoot == null ? "?" : execRoot.toAbsolutePath().toString()); + properties.setProperty(KEY_STARTED, started.toString()); + // written aside and moved in place, so a reader never sees a partial descriptor + Path target = descriptor(runsDir, pid); + Path temp = Files.createTempFile(runsDir, FILE_PREFIX + pid + "-", ".tmp"); Review Comment: 💡 **Observation (non-blocking):** `Files.createTempFile()` creates the temp file before writing to it. If `properties.store()` or `Files.move()` throws and the outer `catch (IOException | RuntimeException ignored)` swallows the exception, the `.tmp` file is left behind in `~/.m2/.maven/runs/`. Over time (crash-loops, disk-full conditions) these accumulate. The `DirectoryStream` glob `mvn-*.properties` correctly excludes `.tmp` files, so correctness is unaffected — this is a cosmetic leak. Two easy fixes: ```suggestion Path target = descriptor(runsDir, pid); Path temp = Files.createTempFile(runsDir, FILE_PREFIX + pid + "-", ".tmp"); try (Writer writer = Files.newBufferedWriter(temp, StandardCharsets.UTF_8)) { properties.store(writer, "mvn run"); writer.flush(); Files.move(temp, target, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); ``` Or simply delete `temp` in the catch path (outside the inner try, inside the outer try): ```java } catch (IOException | RuntimeException ignored) { Files.deleteIfExists(temp); // clean up the orphan .tmp } ``` Either approach keeps the temp file from accumulating on repeated write failures. -- 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]
