gnodet-bot commented on code in PR #29823:
URL: https://github.com/apache/camel/pull/29823#discussion_r4562128511
##########
dsl/camel-jbang/camel-jbang-run/src/main/java/org/apache/camel/dsl/jbang/run/ProjectRunner.java:
##########
@@ -47,6 +51,57 @@ public class ProjectRunner extends ProjectExporter {
*/
public static final String RUN_PLATFORM_DIR = ".camel-jbang-run";
+ /**
+ * The file in the directory of a run with the pid of the process that
runs it (the Camel CLI): a run that is killed
+ * or crashes cannot remove its directory, and a later run removes it
(CAMEL-26912). The file is hidden, as the
+ * project of the run is exported to the directory.
+ */
+ static final String RUN_PID_FILE = ".camel-run-pid";
+
+ /**
+ * Creates the directory of a run in {@link #RUN_PLATFORM_DIR}, with the
pid of this process, and removes the
+ * directories of the earlier runs whose process is gone.
+ */
+ public static Path createRunDir() throws IOException {
+ removeStaleRunDirs();
+ Path dir = Paths.get(RUN_PLATFORM_DIR,
Long.toString(System.currentTimeMillis()));
+ Files.createDirectories(dir);
+ Files.writeString(dir.resolve(RUN_PID_FILE),
Long.toString(ProcessHandle.current().pid()));
+ return dir;
+ }
+
+ /**
+ * Removes the directories of the runs in {@link #RUN_PLATFORM_DIR} whose
process is gone, killed or crashed before
+ * it could remove the directory. The directories of the runs that are
running are kept, and so are the directories
+ * without the pid of their process (of an earlier Camel version), which
may still be running.
+ */
+ public static void removeStaleRunDirs() {
+ removeStaleRunDirs(Paths.get(RUN_PLATFORM_DIR));
+ }
+
+ static void removeStaleRunDirs(Path runDirs) {
+ if (!Files.isDirectory(runDirs)) {
+ return;
+ }
+ try (Stream<Path> dirs = Files.list(runDirs)) {
+ for (Path dir : dirs.filter(Files::isDirectory).toList()) {
+ Path pidFile = dir.resolve(RUN_PID_FILE);
+ long pid;
+ try {
+ pid = Long.parseLong(Files.readString(pidFile).trim());
+ } catch (IOException | NumberFormatException e) {
+ // not known which process runs it
+ continue;
+ }
+ if
(!ProcessHandle.of(pid).map(ProcessHandle::isAlive).orElse(false)) {
+ RunPipeline.removeDir(dir);
+ }
+ }
+ } catch (IOException e) {
+ // removed by a later run
Review Comment:
**Misleading comment.** This `catch` wraps `Files.list(runDirs)` which can
throw for permission errors, disk I/O failures, or other OS-level problems —
not only the "removed by a later run" race. The current comment creates a false
assumption that any swallowed `IOException` here is benign.
```suggestion
} catch (IOException e) {
// best-effort: directory may have been removed concurrently, or
listing failed (permissions, I/O)
}
```
--
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]
