From 5a709e2b4f49f07f5522b2f55aef0ded87ee8270 Mon Sep 17 00:00:00 2001 From: "L. Sousa" Date: Wed, 5 Aug 2026 22:10:33 +0100 Subject: [PATCH 1/7] refactor: deduplicate clang-dumper include cache --- .../src/pt/up/fe/specs/clang/CacheFiles.java | 204 +++++ .../fe/specs/clang/ClangAstWebResource.java | 3 +- .../pt/up/fe/specs/clang/ClangResources.java | 594 +++++---------- .../up/fe/specs/clang/ClangResourcesTest.java | 698 ++++++------------ 4 files changed, 637 insertions(+), 862 deletions(-) create mode 100644 ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java diff --git a/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java b/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java new file mode 100644 index 000000000..004b902c5 --- /dev/null +++ b/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java @@ -0,0 +1,204 @@ +/** + * Copyright 2026 SPeCS. + *

+ * Licensed 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 pt.up.fe.specs.clang; + +import pt.up.fe.specs.util.SpecsIo; +import pt.up.fe.specs.util.providers.FileResourceProvider; + +import java.io.File; +import java.io.IOException; +import java.io.OutputStream; +import java.io.UncheckedIOException; +import java.nio.file.AtomicMoveNotSupportedException; +import java.nio.file.DirectoryStream; +import java.nio.file.FileAlreadyExistsException; +import java.nio.file.FileSystemException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.attribute.FileTime; +import java.security.DigestInputStream; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.time.Instant; +import java.util.HexFormat; + +final class CacheFiles { + + private CacheFiles() { + } + + static Path createStagingDirectory(Path parent, String prefix) { + try { + Files.createDirectories(parent); + return Files.createTempDirectory(parent, prefix); + } catch (IOException e) { + throw new UncheckedIOException("Could not create cache staging directory below '" + parent + "'", e); + } + } + + static File installFile(File destination, FileResourceProvider resource, String expectedSha256, + String description) { + if (destination.isFile()) { + return destination; + } + + Path stagingDirectory = createStagingDirectory(destination.getParentFile().toPath(), + "." + destination.getName() + ".tmp-"); + try { + File stagedFile = resource.write(stagingDirectory.toFile()); + if (stagedFile == null || !stagedFile.isFile()) { + throw new RuntimeException("Could not download " + description); + } + + if (expectedSha256 != null && !hasExpectedSha256(stagedFile, expectedSha256)) { + throw new RuntimeException("Downloaded " + description + " does not match expected SHA-256 '" + + expectedSha256 + "'"); + } + + return publish(stagedFile.toPath(), destination.toPath()).toFile(); + } finally { + deleteQuietly(stagingDirectory); + } + } + + static Path publish(Path staging, Path destination) { + try { + Files.createDirectories(destination.getParent()); + if (Files.exists(destination)) { + return destination; + } + + try { + Files.move(staging, destination, StandardCopyOption.ATOMIC_MOVE); + } catch (FileAlreadyExistsException e) { + // Another process completed the same object first. + } catch (AtomicMoveNotSupportedException e) { + try { + Files.move(staging, destination); + } catch (FileAlreadyExistsException ignored) { + // Another process completed the same object first. + } catch (FileSystemException collision) { + if (!Files.exists(destination)) { + throw collision; + } + + // Some file systems report a non-empty directory collision as a generic file-system exception. + } + } catch (FileSystemException e) { + if (!Files.exists(destination)) { + throw e; + } + + // Some file systems report a non-empty directory collision as a generic file-system exception. + } + + return destination; + } catch (IOException e) { + throw new UncheckedIOException("Could not publish cache object '" + destination + "'", e); + } + } + + static boolean hasExpectedSha256(File file, String expectedSha256) { + return expectedSha256.equalsIgnoreCase(calculateSha256(file)); + } + + static void touch(Path path) { + try { + Files.setLastModifiedTime(path, FileTime.from(Instant.now())); + } catch (IOException e) { + throw new UncheckedIOException("Could not update cache use time for '" + path + "'", e); + } + } + + static void deleteStaleDirectories(Path parent, Instant cutoff, Path excluded) { + if (!Files.isDirectory(parent)) { + return; + } + + try (DirectoryStream children = Files.newDirectoryStream(parent)) { + for (Path child : children) { + if (!Files.isDirectory(child) || child.getFileName().toString().startsWith(".")) { + continue; + } + + if (excluded != null && child.toAbsolutePath().normalize().equals(excluded.toAbsolutePath().normalize())) { + continue; + } + + if (Files.getLastModifiedTime(child).toInstant().isBefore(cutoff)) { + delete(child); + } + } + } catch (IOException e) { + throw new UncheckedIOException("Could not clean stale cache directories below '" + parent + "'", e); + } + } + + static void deleteStaleStagingDirectories(Path parent, Instant cutoff) { + if (!Files.isDirectory(parent)) { + return; + } + + try (DirectoryStream children = Files.newDirectoryStream(parent)) { + for (Path child : children) { + String name = child.getFileName().toString(); + if (!Files.isDirectory(child) || !name.startsWith(".") || !name.contains(".tmp-")) { + continue; + } + + if (Files.getLastModifiedTime(child).toInstant().isBefore(cutoff)) { + delete(child); + } + } + } catch (IOException e) { + throw new UncheckedIOException("Could not clean stale cache staging directories below '" + parent + "'", + e); + } + } + + static void delete(Path path) { + if (!Files.exists(path)) { + return; + } + + boolean deleted = path.toFile().isDirectory() + ? SpecsIo.deleteFolder(path.toFile()) + : SpecsIo.delete(path.toFile()); + if (!deleted && Files.exists(path)) { + throw new RuntimeException("Could not delete cache path '" + path + "'"); + } + } + + private static void deleteQuietly(Path path) { + try { + delete(path); + } catch (RuntimeException ignored) { + // A failed best-effort cleanup must not hide the download or extraction result. + } + } + + private static String calculateSha256(File file) { + try { + var digest = MessageDigest.getInstance("SHA-256"); + try (var inputStream = new DigestInputStream(Files.newInputStream(file.toPath()), digest)) { + inputStream.transferTo(OutputStream.nullOutputStream()); + } + + return HexFormat.of().formatHex(digest.digest()); + } catch (IOException | NoSuchAlgorithmException e) { + throw new RuntimeException("Could not calculate SHA-256 for file '" + file + "'", e); + } + } +} diff --git a/ClangAstParser/src/pt/up/fe/specs/clang/ClangAstWebResource.java b/ClangAstParser/src/pt/up/fe/specs/clang/ClangAstWebResource.java index 9689587a9..cdbc0c2a1 100644 --- a/ClangAstParser/src/pt/up/fe/specs/clang/ClangAstWebResource.java +++ b/ClangAstParser/src/pt/up/fe/specs/clang/ClangAstWebResource.java @@ -94,7 +94,8 @@ public static ClangDumperManifest getManifest(File resourceFolder) { var releaseTag = getReleaseTag(); var manifestResource = WebResourceProvider.newInstance(getReleaseBaseUrl(releaseTag), MANIFEST_FILENAME, releaseTag); - var manifestFile = manifestResource.writeVersioned(resourceFolder, ClangAstWebResource.class).getFile(); + var manifestFile = CacheFiles.installFile(new File(resourceFolder, MANIFEST_FILENAME), manifestResource, null, + "clang-dumper release manifest"); var manifest = GSON.fromJson(SpecsIo.read(manifestFile), ClangDumperManifest.class); if (manifest == null) { diff --git a/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java b/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java index 9d54bf5eb..f83098dcb 100644 --- a/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java +++ b/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java @@ -23,45 +23,29 @@ import pt.up.fe.specs.util.SpecsIo; import pt.up.fe.specs.util.SpecsLogs; import pt.up.fe.specs.util.SpecsSystem; -import pt.up.fe.specs.util.providers.FileResourceProvider.ResourceWriteData; import pt.up.fe.specs.util.system.ProcessOutputAsString; import java.io.File; -import java.io.IOException; -import java.io.OutputStream; -import java.io.UncheckedIOException; -import java.nio.file.DirectoryNotEmptyException; -import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; -import java.nio.file.NoSuchFileException; import java.nio.file.Path; -import java.nio.file.StandardOpenOption; -import java.security.DigestInputStream; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; -import java.util.HexFormat; import java.util.List; +import java.util.Locale; import java.util.Map; -import java.util.Set; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; public class ClangResources { - private static final Map CLANG_FILES_CACHE = new ConcurrentHashMap<>(); - private static final Map CLANG_FILES_LOCKS = new ConcurrentHashMap<>(); - private final static String CLANG_FOLDERNAME = "clang_ast_exe"; - private final static String INCLUDES_FOLDERNAME = "includes"; - private final static String LAST_USED_FILENAME = "last-used.txt"; - private final static String CACHE_LOCK_FOLDERNAME = ".cache.lock"; - private final static String CACHE_LOCK_OWNER_PREFIX = "owner-"; - private final static Duration CACHE_LOCK_RETRY_INTERVAL = Duration.ofMillis(100); - private final static Duration CACHE_LOCK_STALE_MAX_AGE = Duration.ofMinutes(5); - private final static Duration STALE_CACHE_MAX_AGE = Duration.ofDays(60); + private static final Map CLANG_FILES_CACHE = new ConcurrentHashMap<>(); + private static final String CLANG_FOLDERNAME = "clang_ast_exe"; + private static final String RELEASES_FOLDERNAME = "releases"; + private static final String INCLUDES_FOLDERNAME = "includes"; + private static final Duration STALE_CACHE_MAX_AGE = Duration.ofDays(60); + private static final Duration STALE_STAGING_MAX_AGE = Duration.ofHours(1); private static final Map HAS_LIBC = new ConcurrentHashMap<>(); @@ -73,19 +57,35 @@ public ClangResources(CodeParser options) { public File getBuiltinCudaLib() { var cudaResourceFolder = SpecsIo.mkdir(options.get(CodeParser.DUMPER_FOLDER), "cuda"); - var cudaFolder = SpecsIo.mkdir(cudaResourceFolder, "cudalib"); - var zipFile = ClangAstWebResource.CUDA_LIB.writeVersioned(cudaResourceFolder, ClangResources.class); + var cudaFolder = new File(cudaResourceFolder, "cudalib"); + var zipFile = CacheFiles.installFile(new File(cudaResourceFolder, ClangAstWebResource.CUDA_LIB_FILENAME), + ClangAstWebResource.CUDA_LIB, null, "built-in CUDA archive"); - if (zipFile.isNewFile() || !isCudaInstallation(cudaFolder)) { - SpecsIo.deleteFolderContents(cudaFolder); - SpecsIo.extractZip(zipFile.getFile(), cudaFolder); + if (isCudaInstallation(cudaFolder)) { + return cudaFolder; } - return cudaFolder; + if (cudaFolder.exists()) { + CacheFiles.delete(cudaFolder.toPath()); + } + + var stagingFolder = CacheFiles.createStagingDirectory(cudaResourceFolder.toPath(), ".cudalib.tmp-"); + try { + if (!SpecsIo.extractZip(zipFile, stagingFolder.toFile()) || !isCudaInstallation(stagingFolder.toFile())) { + throw new RuntimeException("Built-in CUDA archive did not contain a valid CUDA installation"); + } + + return CacheFiles.publish(stagingFolder, cudaFolder.toPath()).toFile(); + } finally { + CacheFiles.delete(stagingFolder); + } } private static boolean isCudaInstallation(File folder) { - return folder.isDirectory() && new File(folder, "include/cuda_runtime.h").isFile(); + return folder.isDirectory() + && new File(folder, "include/cuda.h").isFile() + && new File(folder, "include/cuda_runtime.h").isFile() + && new File(folder, "nvvm/libdevice/libdevice.10.bc").isFile(); } public ClangFiles getClangFiles(LibcMode libcMode) { @@ -100,40 +100,47 @@ public ClangFiles getClangFiles(LibcMode libcMode) { var resourceFolder = getClangResourceFolder(); var key = libcMode.name() + "_" + useBuiltinCuda + "_" + source + "_" + resourceFolder.getAbsolutePath(); - var jvmLock = CLANG_FILES_LOCKS.computeIfAbsent(resourceFolder.getAbsolutePath(), ignored -> new Object()); - synchronized (jvmLock) { - var lockFolder = getCacheLockFolder(resourceFolder); - try (var ignored = acquireCacheLock(resourceFolder)) { - var files = CLANG_FILES_CACHE.get(key); - if (files != null) { - if (files.clangExecutable().isFile()) { - writeLastUsed(resourceFolder, Instant.now()); - SpecsLogs.debug(() -> "Using cached version of Clang files: " + files); - return files; - } - - CLANG_FILES_CACHE.remove(key, files); - } + var cached = CLANG_FILES_CACHE.get(key); + if (isUsable(cached)) { + touchUse(resourceFolder, cached.includesFolder()); + SpecsLogs.debug(() -> "Using cached version of Clang files: " + cached.files()); + return cached.files(); + } - var manifest = ClangAstWebResource.getManifest(resourceFolder); - File clangExecutable = prepareResources(manifest, resourceFolder); - List builtinIncludes = prepareIncludes(manifest, resourceFolder, clangExecutable, libcMode); + if (cached != null) { + CLANG_FILES_CACHE.remove(key, cached); + } - if (useBuiltinCuda) { - getBuiltinCudaLib(); - } + var manifest = ClangAstWebResource.getManifest(resourceFolder); + File clangExecutable = prepareResources(manifest, resourceFolder); + var includes = prepareIncludes(manifest, clangExecutable, libcMode); - validateTopLevelCacheFiles(manifest, resourceFolder); - updateLastUsedAndCleanupStaleVersions(resourceFolder); + if (useBuiltinCuda) { + getBuiltinCudaLib(); + } - var newFiles = new ClangFiles(clangExecutable, builtinIncludes); - SpecsLogs.debug(() -> "Using downloaded version of Clang files: " + newFiles); + touchUse(resourceFolder, includes.extractedFolder()); + updateLastUsedAndCleanupStaleVersions(resourceFolder, includes.extractedFolder()); - CLANG_FILES_CACHE.put(key, newFiles); - return newFiles; - } catch (IOException e) { - throw new UncheckedIOException("Could not lock clang-dumper cache '" + lockFolder + "'", e); - } + var newFiles = new CachedClangFiles(new ClangFiles(clangExecutable, includes.folders()), + includes.extractedFolder()); + var existingFiles = CLANG_FILES_CACHE.putIfAbsent(key, newFiles); + var selectedFiles = existingFiles == null ? newFiles : existingFiles; + touchUse(resourceFolder, selectedFiles.includesFolder()); + SpecsLogs.debug(() -> "Using downloaded version of Clang files: " + selectedFiles.files()); + return selectedFiles.files(); + } + + private static boolean isUsable(CachedClangFiles cached) { + return cached != null + && cached.files().clangExecutable().isFile() + && (cached.includesFolder() == null || isIncludesCacheValid(cached.includesFolder())); + } + + private static void touchUse(File resourceFolder, File includesFolder) { + CacheFiles.touch(resourceFolder.toPath()); + if (includesFolder != null) { + CacheFiles.touch(includesFolder.toPath()); } } @@ -163,17 +170,20 @@ private File prepareResources(ClangDumperManifest manifest, File resourceFolder) SupportedPlatform platform = SupportedPlatform.getCurrentPlatform(); var executableKind = ClangAstDumper.usePlugin() ? "plugin" : "tool"; - ResourceWriteData executable = downloadAsset(manifest, executableKind, resourceFolder); + var asset = getCurrentAsset(manifest, executableKind); + File executable = CacheFiles.installFile(new File(resourceFolder, asset.filename()), + ClangAstWebResource.getAssetResource(asset), asset.sha256(), + "clang-dumper asset '" + asset.filename() + "'"); if (platform.isWindows()) { - unblockWindowsFile(executable.getFile()); + unblockWindowsFile(executable); } - if (executable.isNewFile() && (platform.isLinux() || platform.isMacOs())) { - SpecsSystem.runProcess(Arrays.asList("chmod", "+x", executable.getFile().getAbsolutePath()), false, true); + if (platform.isLinux() || platform.isMacOs()) { + SpecsSystem.runProcess(Arrays.asList("chmod", "+x", executable.getAbsolutePath()), false, true); } - return executable.getFile(); + return executable; } private void unblockWindowsFile(File executable) { @@ -196,13 +206,31 @@ private void unblockWindowsFile(File executable) { } public File getClangResourceFolder() { - return SpecsIo.mkdir(options.get(CodeParser.DUMPER_FOLDER), ClangAstWebResource.getReleaseTag()); + var releaseFolder = SpecsIo.mkdir(getReleasesFolder(), ClangAstWebResource.getReleaseTag()); + CacheFiles.touch(releaseFolder.toPath()); + return releaseFolder; } public static File getDefaultTempFolder() { return SpecsIo.getTempFolder(CLANG_FOLDERNAME); } + private File getReleasesFolder() { + return SpecsIo.mkdir(options.get(CodeParser.DUMPER_FOLDER), RELEASES_FOLDERNAME); + } + + private File getIncludesRoot() { + return new File(options.get(CodeParser.DUMPER_FOLDER), INCLUDES_FOLDERNAME); + } + + private File getIncludesFolder(String sha256) { + return getSharedIncludesFolder(options.get(CodeParser.DUMPER_FOLDER), sha256); + } + + static File getSharedIncludesFolder(File cacheFolder, String sha256) { + return new File(new File(cacheFolder, INCLUDES_FOLDERNAME), sha256.toLowerCase(Locale.ROOT)); + } + public static boolean useBuiltinLibc(File clangExecutable, LibcMode libcMode) { return switch (libcMode) { case AUTO -> !hasLibC(clangExecutable); @@ -258,86 +286,105 @@ private static ProcessOutputAsString runClangAstDumper(File clangExecutable, Fil return SpecsSystem.runProcess(arguments, true, false); } - private List prepareIncludes(ClangDumperManifest manifest, File resourceFolder, File clangExecutable, - LibcMode libcMode) { + private PreparedIncludes prepareIncludes(ClangDumperManifest manifest, File clangExecutable, + LibcMode libcMode) { var useBuiltinLibc = useBuiltinLibc(clangExecutable, libcMode); var useBuiltinCuda = options.get(CodeParser.CUDA_PATH).equalsIgnoreCase(CodeParser.getBuiltinOption()); if (!useBuiltinLibc && !useBuiltinCuda) { - return List.of(); + return new PreparedIncludes(List.of(), null); } - return prepareIncludes(manifest, resourceFolder); - } - - private List prepareIncludes(ClangDumperManifest manifest, File resourceFolder) { - var extractedFolder = prepareIncludesFolder(manifest, resourceFolder); + var extractedFolder = prepareIncludesFolder(manifest); var includeFolders = getIncludeFolders(extractedFolder); SpecsLogs.debug(() -> "Includes folders: " + includeFolders); - return includeFolders.stream().map(File::getAbsolutePath).toList(); + return new PreparedIncludes(includeFolders.stream().map(File::getAbsolutePath).toList(), extractedFolder); } - private File prepareIncludesFolder(ClangDumperManifest manifest, File resourceFolder) { + private File prepareIncludesFolder(ClangDumperManifest manifest) { var includesAsset = getCurrentAsset(manifest, "includes"); - var extractedFolder = new File(resourceFolder, INCLUDES_FOLDERNAME); + var extractedFolder = getIncludesFolder(includesAsset.sha256()); if (isIncludesCacheValid(extractedFolder)) { return extractedFolder; } - ResourceWriteData zipFile = downloadAsset(includesAsset, resourceFolder); - + var includesRoot = getIncludesRoot(); + CacheFiles.deleteStaleStagingDirectories(includesRoot.toPath(), + Instant.now().minus(STALE_STAGING_MAX_AGE)); + var stagingFolder = CacheFiles.createStagingDirectory(includesRoot.toPath(), + "." + includesAsset.sha256() + ".tmp-"); try { - SpecsIo.mkdir(extractedFolder); - SpecsIo.deleteFolderContents(extractedFolder); - SpecsIo.extractZip(zipFile.getFile(), extractedFolder); + var downloadFolder = CacheFiles.createStagingDirectory(stagingFolder, ".download-"); + try { + var archive = ClangAstWebResource.getAssetResource(includesAsset).write(downloadFolder.toFile()); + if (archive == null || !archive.isFile()) { + throw new RuntimeException("Could not download clang-dumper includes archive '" + + includesAsset.filename() + "'"); + } + + if (!CacheFiles.hasExpectedSha256(archive, includesAsset.sha256())) { + throw new RuntimeException("Downloaded clang-dumper asset '" + includesAsset.filename() + + "' does not match expected SHA-256 '" + includesAsset.sha256() + "'"); + } + + if (!SpecsIo.extractZip(archive, stagingFolder.toFile())) { + throw new RuntimeException("Could not extract clang-dumper includes archive '" + + includesAsset.filename() + "'"); + } + } finally { + CacheFiles.delete(downloadFolder); + } + + getIncludeFolders(stagingFolder.toFile()); + if (isIncludesCacheValid(extractedFolder)) { + CacheFiles.touch(extractedFolder.toPath()); + return extractedFolder; + } + + if (extractedFolder.exists()) { + CacheFiles.delete(extractedFolder.toPath()); + } + + var publishedFolder = CacheFiles.publish(stagingFolder, extractedFolder.toPath()).toFile(); + if (!isIncludesCacheValid(publishedFolder)) { + throw new RuntimeException("Published clang-dumper includes are invalid: '" + publishedFolder + "'"); + } + + CacheFiles.touch(publishedFolder.toPath()); + return publishedFolder; } finally { - SpecsIo.delete(zipFile.getFile()); + CacheFiles.delete(stagingFolder); } - - return extractedFolder; } - private List getIncludeFolders(File extractedFolder) { + static List getIncludeFolders(File extractedFolder) { + if (!extractedFolder.isDirectory()) { + throw new RuntimeException("Could not find extracted clang-dumper includes folder '" + extractedFolder + "'"); + } + var entrypointsFile = new File(extractedFolder, "entrypoints.txt"); if (!entrypointsFile.isFile()) { throw new RuntimeException("Could not find include archive entrypoints file '" + entrypointsFile + "'"); } - return SpecsIo.read(entrypointsFile).lines() + Path root = extractedFolder.toPath().toAbsolutePath().normalize(); + var includeFolders = new ArrayList(); + var entrypoints = SpecsIo.read(entrypointsFile).lines() .map(String::trim) - .filter(line -> !line.isEmpty()) - .map(line -> new File(extractedFolder, line)) + .filter(value -> !value.isEmpty()) .toList(); - } - - private ResourceWriteData downloadAsset(ClangDumperManifest manifest, String kind, File resourceFolder) { - var asset = getCurrentAsset(manifest, kind); - return downloadAsset(asset, resourceFolder); - } - - private ResourceWriteData downloadAsset(ClangDumperManifestAsset asset, File resourceFolder) { - var resource = ClangAstWebResource.getAssetResource(asset); - var writeData = resource.writeVersioned(resourceFolder, ClangResources.class); - - if (!writeData.isNewFile()) { - return writeData; - } - - if (!hasExpectedSha256(writeData.getFile(), asset)) { - SpecsLogs.info("Downloaded clang-dumper asset '" + asset.filename() - + "' does not match the expected checksum, downloading it again."); - SpecsIo.delete(writeData.getFile()); - writeData = resource.writeVersioned(resourceFolder, ClangResources.class); - } + for (String line : entrypoints) { + Path includeFolder = root.resolve(line).normalize(); + if (!includeFolder.startsWith(root) || !Files.isDirectory(includeFolder)) { + throw new RuntimeException("Include archive entrypoint is not a usable directory: '" + line + "'"); + } - if (!hasExpectedSha256(writeData.getFile(), asset)) { - throw new RuntimeException("Downloaded clang-dumper asset '" + asset.filename() - + "' does not match expected SHA-256 '" + asset.sha256() + "'"); + includeFolders.add(includeFolder.toFile()); } - return writeData; + return includeFolders; } private ClangDumperManifestAsset getCurrentAsset(ClangDumperManifest manifest, String kind) { @@ -347,317 +394,41 @@ private ClangDumperManifestAsset getCurrentAsset(ClangDumperManifest manifest, S } static boolean isIncludesCacheValid(File includesFolder) { - - if (!includesFolder.isDirectory()) { - return false; - } - - var entrypointsFile = new File(includesFolder, "entrypoints.txt"); - if (!entrypointsFile.isFile()) { - SpecsLogs.info("Cached clang-dumper includes are missing entrypoints, extracting them again."); + try { + getIncludeFolders(includesFolder); + return true; + } catch (RuntimeException e) { + SpecsLogs.info("Cached clang-dumper includes are invalid: " + includesFolder); return false; } - - return true; - } - - private void validateTopLevelCacheFiles(ClangDumperManifest manifest, File resourceFolder) { - var executableKind = ClangAstDumper.usePlugin() ? "plugin" : "tool"; - Set expectedNames = Set.of( - ClangAstWebResource.MANIFEST_FILENAME, - getCurrentAsset(manifest, executableKind).filename(), - INCLUDES_FOLDERNAME, - LAST_USED_FILENAME); - - var files = resourceFolder.listFiles(); - if (files == null) { - return; - } - - for (var file : files) { - if (expectedNames.contains(file.getName())) { - continue; - } - - SpecsLogs.info("Deleting unexpected file from clang-dumper cache: " + file); - SpecsIo.delete(file); - } } - private void updateLastUsedAndCleanupStaleVersions(File resourceFolder) { + private void updateLastUsedAndCleanupStaleVersions(File resourceFolder, File includesFolder) { var now = Instant.now(); - writeLastUsed(resourceFolder, now); + touchUse(resourceFolder, includesFolder); - var staleCleanup = new Thread(() -> deleteStaleVersions(now, resourceFolder), + var staleCleanup = new Thread(() -> deleteStaleVersions(now, resourceFolder, includesFolder), "clang-dumper-stale-cache-cleanup"); staleCleanup.setDaemon(true); staleCleanup.start(); } - private static void writeLastUsed(File resourceFolder, Instant timestamp) { - writeTimestamp(new File(resourceFolder, LAST_USED_FILENAME), timestamp); - } - - private static void writeTimestamp(File file, Instant timestamp) { - SpecsIo.write(file, timestamp.toString()); - } - void deleteStaleVersions(Instant now, File currentVersionFolder) { - File cacheBaseFolder = options.get(CodeParser.DUMPER_FOLDER); - var versions = cacheBaseFolder.listFiles(File::isDirectory); - if (versions == null) { - return; - } - - for (var versionFolder : versions) { - if (versionFolder.getAbsoluteFile().equals(currentVersionFolder.getAbsoluteFile())) { - continue; - } - - var jvmLock = CLANG_FILES_LOCKS.computeIfAbsent(versionFolder.getAbsolutePath(), ignored -> new Object()); - try { - synchronized (jvmLock) { - var lastUsedFile = new File(versionFolder, LAST_USED_FILENAME); - if (!lastUsedFile.isFile()) { - continue; - } - - try (var lock = tryAcquireCacheLock(versionFolder)) { - if (lock == null) { - SpecsLogs.debug(() -> "Skipping locked clang-dumper cache folder: " + versionFolder); - continue; - } - - if (!lastUsedFile.isFile()) { - continue; - } - - var lastUsed = Instant.parse(SpecsIo.read(lastUsedFile).trim()); - if (lastUsed.isBefore(now.minus(STALE_CACHE_MAX_AGE))) { - SpecsLogs.info("Deleting stale clang-dumper cache folder: " + versionFolder); - SpecsIo.deleteFolder(versionFolder); - } - } - } - } catch (IOException | RuntimeException e) { - SpecsLogs.warn("Could not inspect clang-dumper cache folder '" + versionFolder + "'", e); - } - } + deleteStaleVersions(now, currentVersionFolder, null); } - static CacheLock acquireCacheLock(File versionFolder) throws IOException { - return acquireCacheLock(versionFolder, true); - } - - private static CacheLock tryAcquireCacheLock(File versionFolder) throws IOException { - return acquireCacheLock(versionFolder, false); - } - - private static CacheLock acquireCacheLock(File versionFolder, boolean wait) throws IOException { - var lockFolder = getCacheLockFolder(versionFolder); - Files.createDirectories(lockFolder.getParentFile().toPath()); - - while (true) { - try { - Files.createDirectory(lockFolder.toPath()); - } catch (FileAlreadyExistsException e) { - if (!isCacheLockStale(lockFolder)) { - if (!wait) { - return null; - } - - waitForCacheLock(); - continue; - } - - recoverStaleCacheLock(lockFolder); - continue; - } - - var ownerFile = new File(lockFolder, CACHE_LOCK_OWNER_PREFIX + UUID.randomUUID()); - try { - Files.writeString(ownerFile.toPath(), getProcessIdentity(), StandardOpenOption.CREATE_NEW, - StandardOpenOption.WRITE); - } catch (NoSuchFileException e) { - // Stale-lock recovery removed the directory while this process was claiming it. - continue; - } catch (IOException e) { - deleteEmptyCacheLock(lockFolder); - throw e; - } - - return new CacheLock(lockFolder, ownerFile); - } - } - - /** - * Returns the temporary lock folder for a cache version. - * - *

The lock folder is outside the version folder because stale cleanup deletes that folder while holding the - * lock. The folder is removed when the lock is released, so normal operation leaves no lock artifact behind.

- */ - static File getCacheLockFolder(File versionFolder) { - var absoluteVersionFolder = versionFolder.getAbsoluteFile(); - return new File(absoluteVersionFolder.getParentFile(), absoluteVersionFolder.getName() + CACHE_LOCK_FOLDERNAME); - } - - private static String getProcessIdentity() { - var process = ProcessHandle.current(); - var startTime = process.info().startInstant().map(Instant::toString).orElse(""); - return process.pid() + System.lineSeparator() + startTime; - } - - private static boolean isCacheLockStale(File lockFolder) throws IOException { - if (!lockFolder.exists()) { - return true; - } - - if (!lockFolder.isDirectory()) { - throw new IOException("Cache lock path is not a directory: '" + lockFolder + "'"); - } - - var ownerFiles = lockFolder.listFiles(File::isFile); - if (ownerFiles == null) { - if (!lockFolder.exists()) { - return true; - } - - throw new IOException("Could not list cache lock folder: '" + lockFolder + "'"); - } - - if (ownerFiles.length == 0) { - return isCacheLockOld(lockFolder); - } - - for (var ownerFile : ownerFiles) { - if (!isCacheLockOwnerStale(lockFolder, ownerFile)) { - return false; - } - } - - return true; - } - - private static boolean isCacheLockOwnerStale(File lockFolder, File ownerFile) throws IOException { - List lines; - try { - lines = Files.readAllLines(ownerFile.toPath()); - } catch (NoSuchFileException e) { - return true; - } - - if (lines.isEmpty()) { - return isCacheLockOld(lockFolder); - } - + private void deleteStaleVersions(Instant now, File currentVersionFolder, File currentIncludesFolder) { + var cutoff = now.minus(STALE_CACHE_MAX_AGE); try { - var pid = Long.parseLong(lines.get(0).trim()); - var process = ProcessHandle.of(pid); - if (process.isEmpty() || !process.get().isAlive()) { - return true; - } - - if (lines.size() > 1 && !lines.get(1).isBlank()) { - var processStart = process.get().info().startInstant(); - if (processStart.isPresent() && !processStart.get().toString().equals(lines.get(1).trim())) { - return true; - } - } - - return false; - } catch (NumberFormatException e) { - return isCacheLockOld(lockFolder); - } - } - - private static boolean isCacheLockOld(File lockFolder) throws IOException { - try { - return Files.getLastModifiedTime(lockFolder.toPath()).toInstant() - .isBefore(Instant.now().minus(CACHE_LOCK_STALE_MAX_AGE)); - } catch (NoSuchFileException e) { - return true; - } - } - - private static void waitForCacheLock() throws IOException { - try { - Thread.sleep(CACHE_LOCK_RETRY_INTERVAL.toMillis()); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new IOException("Interrupted while waiting for clang-dumper cache lock", e); - } - } - - private static void recoverStaleCacheLock(File lockFolder) throws IOException { - if (!lockFolder.isDirectory()) { - return; - } - - var ownerFiles = lockFolder.listFiles(File::isFile); - if (ownerFiles == null) { - return; - } - - for (var ownerFile : ownerFiles) { - if (isCacheLockOwnerStale(lockFolder, ownerFile)) { - Files.deleteIfExists(ownerFile.toPath()); - } - } - - deleteEmptyCacheLock(lockFolder); - } - - private static void deleteEmptyCacheLock(File lockFolder) throws IOException { - try { - Files.deleteIfExists(lockFolder.toPath()); - } catch (DirectoryNotEmptyException e) { - // A replacement owner claimed the lock while stale recovery was in progress. - } - } - - /** - * A temporary claim on one cache version. Each claim has its own owner marker, so releasing an old claim cannot - * remove a newer claim created after stale-lock recovery. - */ - static final class CacheLock implements AutoCloseable { - - private final File lockFolder; - private final File ownerFile; - - private CacheLock(File lockFolder, File ownerFile) { - this.lockFolder = lockFolder; - this.ownerFile = ownerFile; - } - - File ownerFile() { - return ownerFile; - } - - @Override - public void close() { - try { - Files.deleteIfExists(ownerFile.toPath()); - deleteEmptyCacheLock(lockFolder); - } catch (IOException e) { - SpecsLogs.warn("Could not remove temporary clang-dumper cache lock '" + lockFolder + "'", e); - } - } - } - - private static boolean hasExpectedSha256(File file, ClangDumperManifestAsset asset) { - return asset.sha256().equalsIgnoreCase(calculateSha256(file)); - } - - private static String calculateSha256(File file) { - try { - var digest = MessageDigest.getInstance("SHA-256"); - try (var inputStream = new DigestInputStream(Files.newInputStream(file.toPath()), digest)) { - inputStream.transferTo(OutputStream.nullOutputStream()); - } - - return HexFormat.of().formatHex(digest.digest()); - } catch (IOException | NoSuchAlgorithmException e) { - throw new RuntimeException("Could not calculate SHA-256 for file '" + file + "'", e); + CacheFiles.deleteStaleDirectories(getReleasesFolder().toPath(), cutoff, currentVersionFolder.toPath()); + CacheFiles.deleteStaleDirectories(getIncludesRoot().toPath(), cutoff, + currentIncludesFolder == null ? null : currentIncludesFolder.toPath()); + CacheFiles.deleteStaleStagingDirectories(getReleasesFolder().toPath(), + now.minus(STALE_STAGING_MAX_AGE)); + CacheFiles.deleteStaleStagingDirectories(getIncludesRoot().toPath(), + now.minus(STALE_STAGING_MAX_AGE)); + } catch (RuntimeException e) { + SpecsLogs.warn("Could not clean stale clang-dumper cache resources", e); } } @@ -693,4 +464,9 @@ private static String getManifestArch(String platform) { throw new RuntimeException("Unsupported architecture for clang-dumper: " + osArch); } + private record PreparedIncludes(List folders, File extractedFolder) { + } + + private record CachedClangFiles(ClangFiles files, File includesFolder) { + } } diff --git a/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java b/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java index 7da6ca019..e074dc952 100644 --- a/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java +++ b/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java @@ -13,38 +13,45 @@ package pt.up.fe.specs.clang; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assumptions.assumeTrue; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import pt.up.fe.specs.clang.ClangAstWebResource.ClangDumperManifest; +import pt.up.fe.specs.clang.ClangAstWebResource.ClangDumperManifestAsset; +import pt.up.fe.specs.clang.ClangAstWebResource.LocalBuild; +import pt.up.fe.specs.clang.ClangAstWebResource.Release; +import pt.up.fe.specs.clang.codeparser.CodeParser; +import pt.up.fe.specs.clang.dumper.ClangAstDumper; +import pt.up.fe.specs.clang.parsers.TopLevelNodesParser; +import pt.up.fe.specs.util.providers.FileResourceProvider; import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.nio.file.attribute.FileTime; import java.time.Duration; import java.time.Instant; +import java.util.ArrayList; import java.util.List; -import java.util.Map; import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicInteger; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.io.TempDir; - -import pt.up.fe.specs.clang.ClangAstWebResource.LocalBuild; -import pt.up.fe.specs.clang.ClangAstWebResource.Release; -import pt.up.fe.specs.clang.codeparser.CodeParser; -import pt.up.fe.specs.clang.parsers.TopLevelNodesParser; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; public class ClangResourcesTest { - private static final Duration PROCESS_TIMEOUT = Duration.ofSeconds(10); + private static final Duration PROCESS_TIMEOUT = Duration.ofSeconds(30); + private static final String HELLO_SHA256 = "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824"; @TempDir Path tempFolder; @@ -72,7 +79,9 @@ public void relativePathIsRejected() { @Test public void localBuildSelectsExpectedTool() throws IOException { - var toolName = SupportedPlatform.getCurrentPlatform().isWindows() ? "tool.exe" : "tool"; + var toolName = ClangAstDumper.usePlugin() + ? System.mapLibraryName("plugin") + : SupportedPlatform.getCurrentPlatform().isWindows() ? "tool.exe" : "tool"; var tool = tempFolder.resolve(toolName).toFile(); assertTrue(tool.createNewFile()); @@ -85,11 +94,23 @@ public void localBuildRequiresExpectedTool() { } @Test - public void includesCacheValidationOnlyChecksRequiredFiles() throws IOException { - var includesFolder = tempFolder.resolve("includes"); - assertFalse(ClangResources.isIncludesCacheValid(includesFolder.toFile())); + public void manifestValidationAndAssetSelectionArePreserved() { + var tool = asset("tool", "tool", "linux", "x64"); + var plugin = asset("plugin", "plugin", "linux", "x64"); + var manifest = new ClangDumperManifest(1, List.of(tool, plugin)); + + assertDoesNotThrow(manifest::validate); + assertEquals(tool, manifest.getAsset("linux", "x64", "tool")); + assertEquals(plugin, manifest.getAsset("linux", "x64", "plugin")); + assertEquals(HELLO_SHA256, tool.sha256()); + assertThrows(RuntimeException.class, () -> manifest.getAsset("windows", "x64", "tool")); + assertThrows(RuntimeException.class, () -> new ClangDumperManifest(2, List.of(tool)).validate()); + assertThrows(RuntimeException.class, () -> new ClangDumperManifest(1, List.of()).validate()); + } - Files.createDirectory(includesFolder); + @Test + public void includesCacheValidationChecksEntrypointsButNotEveryFile() throws IOException { + var includesFolder = tempFolder.resolve("includes"); assertFalse(ClangResources.isIncludesCacheValid(includesFolder.toFile())); Files.createDirectories(includesFolder.resolve("builtin")); @@ -101,299 +122,140 @@ public void includesCacheValidationOnlyChecksRequiredFiles() throws IOException Files.writeString(includesFolder.resolve("builtin/header.h"), "modified"); assertTrue(ClangResources.isIncludesCacheValid(includesFolder.toFile())); + Files.writeString(includesFolder.resolve("entrypoints.txt"), "missing\n"); + assertFalse(ClangResources.isIncludesCacheValid(includesFolder.toFile())); } @Test - public void staleCacheCleanupSkipsLockedVersions() throws IOException { - var currentVersion = Files.createDirectory(tempFolder.resolve("current")).toFile(); - var staleVersion = Files.createDirectory(tempFolder.resolve("stale")).toFile(); - Files.writeString(staleVersion.toPath().resolve("last-used.txt"), - Instant.now().minus(Duration.ofDays(61)).toString()); + public void entrypointsPreserveDeclaredIncludeOrder() throws IOException { + var includesFolder = Files.createDirectories(tempFolder.resolve("includes")); + var first = Files.createDirectories(includesFolder.resolve("first")); + var second = Files.createDirectories(includesFolder.resolve("second")); + Files.writeString(includesFolder.resolve("entrypoints.txt"), "second\nfirst\n"); - var parser = CodeParser.newInstance(); - parser.set(CodeParser.DUMPER_FOLDER, tempFolder.toFile()); - var resources = new ClangResources(parser); - - try (var ignored = ClangResources.acquireCacheLock(staleVersion)) { - - resources.deleteStaleVersions(Instant.now(), currentVersion); - assertTrue(staleVersion.isDirectory()); - } - - assertFalse(ClangResources.getCacheLockFolder(staleVersion).exists()); - - resources.deleteStaleVersions(Instant.now(), currentVersion); - assertFalse(staleVersion.exists()); + assertEquals(List.of(second.toFile(), first.toFile()), + ClangResources.getIncludeFolders(includesFolder.toFile())); } @Test - public void cacheLockSerializesConcurrentAcquisition() throws Exception { - var versionFolder = Files.createDirectory(tempFolder.resolve("version")).toFile(); - var executor = Executors.newSingleThreadExecutor(); - ClangResources.CacheLock firstLock = ClangResources.acquireCacheLock(versionFolder); - - try { - var secondLock = executor.submit(() -> ClangResources.acquireCacheLock(versionFolder)); - assertFalse(secondLock.isDone()); - - firstLock.close(); - firstLock = null; - try (var ignored = secondLock.get(10, TimeUnit.SECONDS)) { - assertTrue(versionFolder.isDirectory()); - } - } finally { - if (firstLock != null) { - firstLock.close(); - } - executor.shutdownNow(); - executor.awaitTermination(10, TimeUnit.SECONDS); - } + public void sharedIncludesAreAddressedOnlyBySha() throws IOException { + var sha = "a".repeat(64); + var sharedFolder = ClangResources.getSharedIncludesFolder(tempFolder.toFile(), sha); + Files.createDirectories(sharedFolder.toPath().resolve("builtin")); + Files.writeString(sharedFolder.toPath().resolve("entrypoints.txt"), "builtin\n"); - assertFalse(ClangResources.getCacheLockFolder(versionFolder).exists()); + var firstRelease = tempFolder.resolve("releases/v1").toFile(); + var secondRelease = tempFolder.resolve("releases/v2").toFile(); + assertNotEquals(firstRelease, secondRelease); + assertEquals(sharedFolder, ClangResources.getSharedIncludesFolder(tempFolder.toFile(), sha)); + assertTrue(ClangResources.isIncludesCacheValid(sharedFolder)); } @Test - public void cacheLockSerializesAcquisitionAcrossJvms() throws Exception { - var versionFolder = Files.createDirectory(tempFolder.resolve("version")).toFile(); - var firstAcquired = tempFolder.resolve("first.acquired"); - var firstRelease = tempFolder.resolve("first.release"); - var secondAcquired = tempFolder.resolve("second.acquired"); - var secondRelease = tempFolder.resolve("second.release"); - var firstLog = tempFolder.resolve("first.log"); - var secondLog = tempFolder.resolve("second.log"); - - Process first = startCacheLockProcess(versionFolder, firstAcquired, firstRelease, firstLog); - Process second = null; - try { - assertTrue(waitForFile(firstAcquired, PROCESS_TIMEOUT)); - - second = startCacheLockProcess(versionFolder, secondAcquired, secondRelease, secondLog); - assertFalse(waitForFile(secondAcquired, Duration.ofMillis(750)), - "A second JVM acquired a cache lock that was still held"); + public void corruptNewDownloadIsRejectedWithoutRetry() throws IOException { + var source = Files.writeString(tempFolder.resolve("source"), "bad"); + var writes = new AtomicInteger(); + var destination = tempFolder.resolve("release/tool").toFile(); - Files.createFile(firstRelease); - assertTrue(waitForFile(secondAcquired, PROCESS_TIMEOUT)); + assertThrows(RuntimeException.class, + () -> CacheFiles.installFile(destination, copyingResource(source, writes), HELLO_SHA256, "test asset")); - Files.createFile(secondRelease); - waitForProcess(first, firstLog); - waitForProcess(second, secondLog); - } finally { - releaseProcess(firstRelease); - releaseProcess(secondRelease); - stopProcess(first); - stopProcess(second); + assertEquals(1, writes.get()); + assertFalse(destination.exists()); + try (var children = Files.list(destination.getParentFile().toPath())) { + assertTrue(children.noneMatch(path -> path.getFileName().toString().startsWith(".tool.tmp-"))); } - - assertFalse(ClangResources.getCacheLockFolder(versionFolder).exists()); } @Test - public void cacheLockAllowsDifferentVersionsToProceedAcrossJvms() throws Exception { - var firstVersion = Files.createDirectory(tempFolder.resolve("version-1")).toFile(); - var secondVersion = Files.createDirectory(tempFolder.resolve("version-2")).toFile(); - var firstAcquired = tempFolder.resolve("first.acquired"); - var firstRelease = tempFolder.resolve("first.release"); - var secondAcquired = tempFolder.resolve("second.acquired"); - var secondRelease = tempFolder.resolve("second.release"); - var firstLog = tempFolder.resolve("first.log"); - var secondLog = tempFolder.resolve("second.log"); - - Process first = startCacheLockProcess(firstVersion, firstAcquired, firstRelease, firstLog); - Process second = null; - try { - assertTrue(waitForFile(firstAcquired, PROCESS_TIMEOUT)); - - second = startCacheLockProcess(secondVersion, secondAcquired, secondRelease, secondLog); - assertTrue(waitForFile(secondAcquired, PROCESS_TIMEOUT), - "A different dumper version was blocked by an unrelated cache lock"); + public void existingReleaseResourceIsReused() throws IOException { + var destination = tempFolder.resolve("release/tool").toFile(); + Files.createDirectories(destination.toPath().getParent()); + Files.writeString(destination.toPath(), "cached"); + var writes = new AtomicInteger(); + var source = Files.writeString(tempFolder.resolve("source"), "new"); - Files.createFile(firstRelease); - Files.createFile(secondRelease); - waitForProcess(first, firstLog); - waitForProcess(second, secondLog); - } finally { - releaseProcess(firstRelease); - releaseProcess(secondRelease); - stopProcess(first); - stopProcess(second); - } - - assertFalse(ClangResources.getCacheLockFolder(firstVersion).exists()); - assertFalse(ClangResources.getCacheLockFolder(secondVersion).exists()); + assertEquals(destination, + CacheFiles.installFile(destination, copyingResource(source, writes), HELLO_SHA256, "test asset")); + assertEquals(0, writes.get()); + assertEquals("cached", Files.readString(destination.toPath())); } @Test - public void cacheLockRecoversAfterOwningJvmIsTerminated() throws Exception { - var versionFolder = Files.createDirectory(tempFolder.resolve("version")).toFile(); - var firstAcquired = tempFolder.resolve("first.acquired"); - var firstRelease = tempFolder.resolve("first.release"); - var secondAcquired = tempFolder.resolve("second.acquired"); - var secondRelease = tempFolder.resolve("second.release"); - var firstLog = tempFolder.resolve("first.log"); - var secondLog = tempFolder.resolve("second.log"); + public void concurrentPublicationLeavesOneValidIncludesTree() throws Exception { + var includesRoot = Files.createDirectories(tempFolder.resolve("includes")); + var sha = "b".repeat(64); + var finalFolder = includesRoot.resolve(sha); + var executor = Executors.newFixedThreadPool(4); + var futures = new ArrayList>(); - Process first = startCacheLockProcess(versionFolder, firstAcquired, firstRelease, firstLog); - Process second = null; try { - assertTrue(waitForFile(firstAcquired, PROCESS_TIMEOUT)); - first.destroyForcibly(); - assertTrue(first.waitFor(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS)); - - second = startCacheLockProcess(versionFolder, secondAcquired, secondRelease, secondLog); - assertTrue(waitForFile(secondAcquired, PROCESS_TIMEOUT), - "A cache lock left by a terminated JVM was not recovered"); + for (int i = 0; i < 4; i++) { + futures.add(executor.submit(() -> publishTestIncludes(includesRoot, finalFolder, sha))); + } - Files.createFile(secondRelease); - waitForProcess(second, secondLog); + for (var future : futures) { + assertEquals(finalFolder, future.get(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS)); + } } finally { - releaseProcess(firstRelease); - releaseProcess(secondRelease); - stopProcess(first); - stopProcess(second); + executor.shutdownNow(); + executor.awaitTermination(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); } - assertFalse(ClangResources.getCacheLockFolder(versionFolder).exists()); - } - - @Test - public void cacheLockDoesNotTreatLiveOwnerAsStaleWhenDirectoryIsOld() throws Exception { - var versionFolder = Files.createDirectory(tempFolder.resolve("version")).toFile(); - var lockFolder = ClangResources.getCacheLockFolder(versionFolder).toPath(); - var secondAcquired = tempFolder.resolve("second.acquired"); - var secondRelease = tempFolder.resolve("second.release"); - var secondLog = tempFolder.resolve("second.log"); - - ClangResources.CacheLock first = ClangResources.acquireCacheLock(versionFolder); - Process second = null; - try { - Files.setLastModifiedTime(lockFolder, FileTime.from(Instant.now().minus(Duration.ofHours(1)))); - - second = startCacheLockProcess(versionFolder, secondAcquired, secondRelease, secondLog); - assertFalse(waitForFile(secondAcquired, Duration.ofMillis(750)), - "A live cache-lock owner was incorrectly treated as stale"); - - first.close(); - first = null; - assertTrue(waitForFile(secondAcquired, PROCESS_TIMEOUT)); - Files.createFile(secondRelease); - waitForProcess(second, secondLog); - } finally { - if (first != null) { - first.close(); - } - releaseProcess(secondRelease); - stopProcess(second); + assertTrue(ClangResources.isIncludesCacheValid(finalFolder.toFile())); + try (var children = Files.list(includesRoot)) { + assertTrue(children.noneMatch(path -> path.getFileName().toString().startsWith("." + sha + ".tmp-"))); } - - assertFalse(lockFolder.toFile().exists()); } @Test - public void cacheLockUsesProcessStartTimeWhenPidIsStillAlive() throws Exception { - var versionFolder = Files.createDirectory(tempFolder.resolve("version")).toFile(); - var lockFolder = ClangResources.getCacheLockFolder(versionFolder).toPath(); - Files.createDirectory(lockFolder); - assumeTrue(ProcessHandle.current().info().startInstant().isPresent(), - "The current platform does not expose process start times"); - Files.writeString(lockFolder.resolve("owner"), - ProcessHandle.current().pid() + System.lineSeparator() + Instant.EPOCH + System.lineSeparator()); - - var acquired = tempFolder.resolve("acquired"); - var release = tempFolder.resolve("release"); - var log = tempFolder.resolve("child.log"); - Process child = startCacheLockProcess(versionFolder, acquired, release, log); - try { - assertTrue(waitForFile(acquired, PROCESS_TIMEOUT), - "A lock with a reused PID and a different process start time was not recovered"); - Files.createFile(release); - waitForProcess(child, log); - } finally { - releaseProcess(release); - stopProcess(child); - } - - assertFalse(lockFolder.toFile().exists()); - } + public void abandonedStagingDirectoriesAreCleaned() throws IOException { + var includesRoot = Files.createDirectories(tempFolder.resolve("includes")); + var staging = CacheFiles.createStagingDirectory(includesRoot, ".sha.tmp-"); + Files.setLastModifiedTime(staging, FileTime.from(Instant.now().minus(Duration.ofHours(2)))); - @Test - public void cacheLockRecoversAnOldOwnerlessLock() throws Exception { - var versionFolder = Files.createDirectory(tempFolder.resolve("version")).toFile(); - var lockFolder = ClangResources.getCacheLockFolder(versionFolder).toPath(); - Files.createDirectory(lockFolder); - Files.setLastModifiedTime(lockFolder, FileTime.from(Instant.now().minus(Duration.ofHours(1)))); - - var acquired = tempFolder.resolve("acquired"); - var release = tempFolder.resolve("release"); - var log = tempFolder.resolve("child.log"); - Process child = startCacheLockProcess(versionFolder, acquired, release, log); - try { - assertTrue(waitForFile(acquired, PROCESS_TIMEOUT), "An old ownerless cache lock was not recovered"); - Files.createFile(release); - waitForProcess(child, log); - } finally { - releaseProcess(release); - stopProcess(child); - } + CacheFiles.deleteStaleStagingDirectories(includesRoot, Instant.now().minus(Duration.ofHours(1))); - assertFalse(lockFolder.toFile().exists()); + assertFalse(Files.exists(staging)); } @Test - public void cacheLockDoesNotStealARecentOwnerlessLock() throws Exception { - var versionFolder = Files.createDirectory(tempFolder.resolve("version")).toFile(); - var lockFolder = ClangResources.getCacheLockFolder(versionFolder).toPath(); - Files.createDirectory(lockFolder); - - var acquired = tempFolder.resolve("acquired"); - var release = tempFolder.resolve("release"); - var log = tempFolder.resolve("child.log"); - Process child = startCacheLockProcess(versionFolder, acquired, release, log); - try { - assertFalse(waitForFile(acquired, Duration.ofMillis(750)), - "A lock without owner metadata was stolen before its claim was old enough"); + public void staleReleaseAndSharedIncludesAreRemovedAfterSixtyDays() throws IOException { + var releases = Files.createDirectories(tempFolder.resolve("releases")); + var current = Files.createDirectories(releases.resolve("current")); + var staleRelease = Files.createDirectories(releases.resolve("stale")); + var staleIncludes = Files.createDirectories( + ClangResources.getSharedIncludesFolder(tempFolder.toFile(), "c".repeat(64)).toPath().resolve("builtin")); + Files.writeString(staleIncludes.getParent().resolve("entrypoints.txt"), "builtin\n"); + var old = FileTime.from(Instant.now().minus(Duration.ofDays(61))); + Files.setLastModifiedTime(staleRelease, old); + Files.setLastModifiedTime(staleIncludes.getParent(), old); - Files.delete(lockFolder); - assertTrue(waitForFile(acquired, PROCESS_TIMEOUT)); - Files.createFile(release); - waitForProcess(child, log); - } finally { - releaseProcess(release); - stopProcess(child); - } + var parser = CodeParser.newInstance(); + parser.set(CodeParser.DUMPER_FOLDER, tempFolder.toFile()); + new ClangResources(parser).deleteStaleVersions(Instant.now(), current.toFile()); - assertFalse(lockFolder.toFile().exists()); + assertTrue(Files.exists(current)); + assertFalse(Files.exists(staleRelease)); + assertFalse(Files.exists(staleIncludes.getParent())); } @Test - public void staleCacheCleanupSkipsLockedVersionAcrossJvms() throws Exception { - var currentVersion = Files.createDirectory(tempFolder.resolve("current")).toFile(); - var staleVersion = Files.createDirectory(tempFolder.resolve("stale")).toFile(); - Files.writeString(staleVersion.toPath().resolve("last-used.txt"), - Instant.now().minus(Duration.ofDays(61)).toString()); - - var acquired = tempFolder.resolve("acquired"); - var release = tempFolder.resolve("release"); - var log = tempFolder.resolve("child.log"); - Process child = startCacheLockProcess(staleVersion, acquired, release, log); - try { - assertTrue(waitForFile(acquired, PROCESS_TIMEOUT)); - - var parser = CodeParser.newInstance(); - parser.set(CodeParser.DUMPER_FOLDER, tempFolder.toFile()); - new ClangResources(parser).deleteStaleVersions(Instant.now(), currentVersion); - assertTrue(staleVersion.isDirectory(), "Stale cleanup deleted a version locked by another JVM"); + public void usingSharedIncludesRefreshesItsLastUsedTime() throws IOException { + var shared = Files.createDirectories( + ClangResources.getSharedIncludesFolder(tempFolder.toFile(), "d".repeat(64)).toPath()); + Files.createDirectories(shared.resolve("builtin")); + Files.writeString(shared.resolve("entrypoints.txt"), "builtin\n"); + Files.setLastModifiedTime(shared, FileTime.from(Instant.now().minus(Duration.ofDays(61)))); - Files.createFile(release); - waitForProcess(child, log); - } finally { - releaseProcess(release); - stopProcess(child); - } + CacheFiles.touch(shared); var parser = CodeParser.newInstance(); parser.set(CodeParser.DUMPER_FOLDER, tempFolder.toFile()); - new ClangResources(parser).deleteStaleVersions(Instant.now(), currentVersion); - assertFalse(staleVersion.exists()); + new ClangResources(parser).deleteStaleVersions(Instant.now(), + Files.createDirectories(tempFolder.resolve("releases/current")).toFile()); + + assertTrue(Files.exists(shared)); } @Test @@ -418,110 +280,136 @@ public void releaseResourcesCanBeInitializedBySeparateJvms() throws Exception { var secondExecutable = new File(Files.readString(secondDone).trim()); assertEquals(firstExecutable.getAbsoluteFile(), secondExecutable.getAbsoluteFile()); assertTrue(firstExecutable.isFile()); - assertFalse(cacheFolder.toPath().resolve(ClangAstWebResource.getReleaseTag() + ".cache.lock").toFile().exists()); + assertTrue(cacheFolder.toPath().resolve("releases").resolve(ClangAstWebResource.getReleaseTag()).toFile().isDirectory()); } @Test - public void cacheLockReleaseDoesNotRemoveAReclaimedLock() throws Exception { - var versionFolder = Files.createDirectory(tempFolder.resolve("version")).toFile(); - var lockFolder = ClangResources.getCacheLockFolder(versionFolder).toPath(); - ClangResources.CacheLock first = ClangResources.acquireCacheLock(versionFolder); - - Files.writeString(first.ownerFile().toPath(), Long.MAX_VALUE + System.lineSeparator()); - ClangResources.CacheLock second = ClangResources.acquireCacheLock(versionFolder); + public void sameJvmInstancesReuseReleaseFilesAndRefreshSharedIncludes() throws Exception { + var firstParser = newParser(CodeParser.getBuiltinOption()); + var secondParser = newParser(CodeParser.getBuiltinOption()); + var thirdParser = newParser(CodeParser.getBuiltinOption()); + var firstResources = new ClangResources(firstParser); + var secondResources = new ClangResources(secondParser); + var thirdResources = new ClangResources(thirdParser); + var executor = Executors.newFixedThreadPool(3); try { - first.close(); - assertTrue(Files.isDirectory(lockFolder), - "A stale lock owner released and deleted a replacement owner's lock"); - } finally { - second.close(); - } - } + var first = executor.submit(() -> firstResources.getClangFiles(LibcMode.SYSTEM)); + var second = executor.submit(() -> secondResources.getClangFiles(LibcMode.SYSTEM)); + var third = executor.submit(() -> thirdResources.getClangFiles(LibcMode.BUILTIN_AND_LIBC)); - @Test - public void cachedUseDoesNotBypassTheCacheLockBeforeUpdatingLastUsed() throws Exception { - var parser = CodeParser.newInstance(); - parser.set(CodeParser.DUMPER_FOLDER, tempFolder.toFile()); - var resources = new ClangResources(parser); - var versionFolder = resources.getClangResourceFolder(); - var fakeExecutable = Files.createFile(tempFolder.resolve("fake-tool")).toFile(); - var cache = getClangFilesCache(); - var cacheKey = getClangFilesCacheKey(resources, LibcMode.SYSTEM); - var cachedFiles = new ClangFiles(fakeExecutable, List.of()); - cache.put(cacheKey, cachedFiles); - - ClangResources.CacheLock lock = ClangResources.acquireCacheLock(versionFolder); - var executor = Executors.newSingleThreadExecutor(); - try { - var future = executor.submit(() -> resources.getClangFiles(LibcMode.SYSTEM)); - assertThrows(TimeoutException.class, () -> future.get(750, TimeUnit.MILLISECONDS), - "A cached use updated last-used without coordinating with the cache lock"); + var firstFiles = first.get(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); + var secondFiles = second.get(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); + var thirdFiles = third.get(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); - lock.close(); - lock = null; - assertSame(cachedFiles, future.get(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS)); - } finally { - if (lock != null) { - lock.close(); + assertEquals(firstFiles, secondFiles); + assertEquals(firstFiles.clangExecutable().getAbsoluteFile(), thirdFiles.clangExecutable().getAbsoluteFile()); + assertTrue(firstFiles.clangExecutable().isFile()); + + assumeTrue(!firstFiles.builtinIncludes().isEmpty()); + var shared = new File(firstFiles.builtinIncludes().get(0)).toPath(); + while (!Files.isRegularFile(shared.resolve("entrypoints.txt"))) { + shared = shared.getParent(); } + Files.setLastModifiedTime(shared, FileTime.from(Instant.now().minus(Duration.ofDays(61)))); + firstResources.getClangFiles(LibcMode.SYSTEM); + assertTrue(Files.getLastModifiedTime(shared).toInstant().isAfter(Instant.now().minus(Duration.ofDays(1)))); + } finally { executor.shutdownNow(); executor.awaitTermination(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); - cache.remove(cacheKey); } } @Test - public void sameJvmInstancesShareReleaseCacheInitialization() throws Exception { - var firstParser = CodeParser.newInstance(); - firstParser.set(CodeParser.DUMPER_FOLDER, tempFolder.toFile()); - firstParser.set(CodeParser.CUDA_PATH, CodeParser.getBuiltinOption()); + public void builtinCudaIncludesAreAvailableWithSystemLibc() { + var parser = newParser(CodeParser.getBuiltinOption()); + var clangFiles = new ClangResources(parser).getClangFiles(LibcMode.SYSTEM); + var hasCudaWrapper = clangFiles.builtinIncludes().stream() + .map(folder -> new File(folder, "__clang_cuda_runtime_wrapper.h")) + .anyMatch(File::isFile); - var secondParser = CodeParser.newInstance(); - secondParser.set(CodeParser.DUMPER_FOLDER, tempFolder.toFile()); - secondParser.set(CodeParser.CUDA_PATH, CodeParser.getBuiltinOption()); + assertTrue(hasCudaWrapper, "Built-in CUDA must provide Clang's CUDA runtime wrapper independently of libc"); + } - var thirdParser = CodeParser.newInstance(); - thirdParser.set(CodeParser.DUMPER_FOLDER, tempFolder.toFile()); - thirdParser.set(CodeParser.CUDA_PATH, CodeParser.getBuiltinOption()); + @Test + public void builtinCudaArchiveHasCanonicalInstallationLayout() { + var parser = newParser(CodeParser.getBuiltinOption()); + var cudaFolder = new ClangResources(parser).getBuiltinCudaLib(); - var executor = Executors.newFixedThreadPool(3); - try { - var first = executor.submit(() -> new ClangResources(firstParser).getClangFiles(LibcMode.SYSTEM)); - var second = executor.submit(() -> new ClangResources(secondParser).getClangFiles(LibcMode.SYSTEM)); - var third = executor.submit( - () -> new ClangResources(thirdParser).getClangFiles(LibcMode.BUILTIN_AND_LIBC)); + assertEquals(tempFolder.resolve("cuda/cudalib").toFile().getAbsolutePath(), cudaFolder.getAbsolutePath()); + assertTrue(new File(cudaFolder, "include/cuda.h").isFile()); + assertTrue(new File(cudaFolder, "include/cuda_runtime.h").isFile()); + assertTrue(new File(cudaFolder, "nvvm/libdevice/libdevice.10.bc").isFile()); + } - var firstFiles = first.get(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); - var secondFiles = second.get(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); - var thirdFiles = third.get(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); + @Test + public void libcDetectionIsScopedToTheExecutable() throws IOException { + assumeTrue(!SupportedPlatform.getCurrentPlatform().isWindows(), "Shell fixtures require a Unix executable"); - assertSame(firstFiles, secondFiles, "Same-JVM instances did not share the ClangFiles cache entry"); - assertEquals(firstFiles.clangExecutable().getAbsoluteFile(), thirdFiles.clangExecutable().getAbsoluteFile()); - assertTrue(firstFiles.clangExecutable().isFile()); - } finally { - executor.shutdownNow(); - executor.awaitTermination(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); - } + var systemLibcDumper = tempFolder.resolve("system-libc-dumper"); + Files.writeString(systemLibcDumper, + "#!/bin/sh\nprintf '%s\\n' '" + TopLevelNodesParser.getTopLevelNodesHeader() + "'\n"); + assertTrue(systemLibcDumper.toFile().setExecutable(true)); + + var builtinLibcDumper = tempFolder.resolve("builtin-libc-dumper"); + Files.writeString(builtinLibcDumper, "#!/bin/sh\nexit 1\n"); + assertTrue(builtinLibcDumper.toFile().setExecutable(true)); + + assertFalse(ClangResources.useBuiltinLibc(systemLibcDumper.toFile(), LibcMode.AUTO)); + assertTrue(ClangResources.useBuiltinLibc(builtinLibcDumper.toFile(), LibcMode.AUTO)); } - private Process startCacheLockProcess(File versionFolder, Path acquired, Path release, Path log) - throws IOException { + private CodeParser newParser(String cudaPath) { + var parser = CodeParser.newInstance(); + parser.set(CodeParser.DUMPER_FOLDER, tempFolder.toFile()); + parser.set(CodeParser.CUDA_PATH, cudaPath); + return parser; + } - var javaExecutable = Path.of(System.getProperty("java.home"), "bin", - SupportedPlatform.getCurrentPlatform().isWindows() ? "java.exe" : "java"); + private static ClangDumperManifestAsset asset(String filename, String kind, String platform, String arch) { + return new ClangDumperManifestAsset(filename, kind, platform, arch, 18, HELLO_SHA256); + } - return new ProcessBuilder( - javaExecutable.toString(), - "-cp", - System.getProperty("java.class.path"), - CacheLockProcess.class.getName(), - versionFolder.getAbsolutePath(), - acquired.toAbsolutePath().toString(), - release.toAbsolutePath().toString()) - .redirectErrorStream(true) - .redirectOutput(log.toFile()) - .start(); + private static FileResourceProvider copyingResource(Path source, AtomicInteger writes) { + return new FileResourceProvider() { + @Override + public File write(File folder) { + writes.incrementAndGet(); + try { + var destination = folder.toPath().resolve(getFilename()); + Files.copy(source, destination, StandardCopyOption.REPLACE_EXISTING); + return destination.toFile(); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @Override + public String version() { + return "test"; + } + + @Override + public String getFilename() { + return source.getFileName().toString(); + } + }; + } + + private static Path publishTestIncludes(Path includesRoot, Path finalFolder, String sha) { + Path staging = CacheFiles.createStagingDirectory(includesRoot, "." + sha + ".tmp-"); + try { + try { + Files.createDirectories(staging.resolve("builtin")); + Files.writeString(staging.resolve("entrypoints.txt"), "builtin\n"); + } catch (IOException e) { + throw new RuntimeException(e); + } + + return CacheFiles.publish(staging, finalFolder); + } finally { + CacheFiles.delete(staging); + } } private Process startResourceProcess(File cacheFolder, Path done, Path log) throws IOException { @@ -532,8 +420,7 @@ private Process startResourceProcess(File cacheFolder, Path done, Path log) thro javaExecutable.toString(), "-cp", System.getProperty("java.class.path"), - CacheLockProcess.class.getName(), - "resources", + ResourceProcess.class.getName(), cacheFolder.getAbsolutePath(), done.toAbsolutePath().toString()) .redirectErrorStream(true) @@ -541,34 +428,6 @@ private Process startResourceProcess(File cacheFolder, Path done, Path log) thro .start(); } - @SuppressWarnings("unchecked") - private Map getClangFilesCache() throws ReflectiveOperationException { - var cacheField = ClangResources.class.getDeclaredField("CLANG_FILES_CACHE"); - cacheField.setAccessible(true); - return (Map) cacheField.get(null); - } - - private String getClangFilesCacheKey(ClangResources resources, LibcMode libcMode) { - var source = ClangAstWebResource.getDumperSource(); - var sourceKey = source instanceof Release - ? source + "_" + resources.getClangResourceFolder().getAbsolutePath() - : source.toString(); - return libcMode.name() + "_false_" + sourceKey; - } - - private boolean waitForFile(Path file, Duration timeout) throws InterruptedException { - var deadline = System.nanoTime() + timeout.toNanos(); - do { - if (Files.isRegularFile(file)) { - return true; - } - - Thread.sleep(10); - } while (System.nanoTime() < deadline); - - return Files.isRegularFile(file); - } - private void waitForProcess(Process process, Path log) throws Exception { assertTrue(process.waitFor(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS), () -> "Child JVM did not finish. Output: " + readLog(log)); @@ -583,12 +442,6 @@ private String readLog(Path log) { } } - private void releaseProcess(Path release) throws IOException { - if (release != null && !Files.exists(release)) { - Files.createFile(release); - } - } - private void stopProcess(Process process) throws InterruptedException { if (process == null || !process.isAlive()) { return; @@ -598,75 +451,16 @@ private void stopProcess(Process process) throws InterruptedException { process.waitFor(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); } - public static final class CacheLockProcess { + public static final class ResourceProcess { - private CacheLockProcess() { + private ResourceProcess() { } public static void main(String[] args) throws Exception { - if (args[0].equals("resources")) { - var parser = CodeParser.newInstance(); - parser.set(CodeParser.DUMPER_FOLDER, new File(args[1])); - var clangFiles = new ClangResources(parser).getClangFiles(LibcMode.SYSTEM); - Files.writeString(Path.of(args[2]), clangFiles.clangExecutable().getAbsolutePath()); - return; - } - - var versionFolder = new File(args[0]); - var acquired = Path.of(args[1]); - var release = Path.of(args[2]); - - try (var ignored = ClangResources.acquireCacheLock(versionFolder)) { - Files.writeString(acquired, Long.toString(ProcessHandle.current().pid())); - while (!Files.exists(release)) { - Thread.sleep(10); - } - } + var parser = CodeParser.newInstance(); + parser.set(CodeParser.DUMPER_FOLDER, new File(args[0])); + var clangFiles = new ClangResources(parser).getClangFiles(LibcMode.SYSTEM); + Files.writeString(Path.of(args[1]), clangFiles.clangExecutable().getAbsolutePath()); } } - - @Test - public void builtinCudaIncludesAreAvailableWithSystemLibc() { - var parser = CodeParser.newInstance(); - parser.set(CodeParser.CUDA_PATH, CodeParser.getBuiltinOption()); - - var clangFiles = new ClangResources(parser).getClangFiles(LibcMode.SYSTEM); - var hasCudaWrapper = clangFiles.builtinIncludes().stream() - .map(folder -> new File(folder, "__clang_cuda_runtime_wrapper.h")) - .anyMatch(File::isFile); - - assertTrue(hasCudaWrapper, "Built-in CUDA must provide Clang's CUDA runtime wrapper independently of libc"); - } - - @Test - public void builtinCudaArchiveHasCanonicalInstallationLayout() { - var parser = CodeParser.newInstance(); - parser.set(CodeParser.DUMPER_FOLDER, tempFolder.toFile()); - parser.set(CodeParser.CUDA_PATH, CodeParser.getBuiltinOption()); - - var cudaFolder = new ClangResources(parser).getBuiltinCudaLib(); - - assertEquals(tempFolder.resolve("cuda/cudalib").toFile().getAbsolutePath(), - cudaFolder.getAbsolutePath()); - assertTrue(new File(cudaFolder, "include/cuda.h").isFile()); - assertTrue(new File(cudaFolder, "include/cuda_runtime.h").isFile()); - assertTrue(new File(cudaFolder, "nvvm/libdevice/libdevice.10.bc").isFile()); - } - - @Test - public void libcDetectionIsScopedToTheExecutable() throws IOException { - assumeTrue(!SupportedPlatform.getCurrentPlatform().isWindows(), "Shell fixtures require a Unix executable"); - - var systemLibcDumper = tempFolder.resolve("system-libc-dumper"); - Files.writeString(systemLibcDumper, - "#!/bin/sh\nprintf '%s\\n' '" + TopLevelNodesParser.getTopLevelNodesHeader() + "'\n"); - assertTrue(systemLibcDumper.toFile().setExecutable(true)); - - var builtinLibcDumper = tempFolder.resolve("builtin-libc-dumper"); - Files.writeString(builtinLibcDumper, "#!/bin/sh\nexit 1\n"); - assertTrue(builtinLibcDumper.toFile().setExecutable(true)); - - assertFalse(ClangResources.useBuiltinLibc(systemLibcDumper.toFile(), LibcMode.AUTO)); - assertTrue(ClangResources.useBuiltinLibc(builtinLibcDumper.toFile(), LibcMode.AUTO)); - } } From 4f20b87c4932c8b149859a583800fff266208dcb Mon Sep 17 00:00:00 2001 From: "L. Sousa" Date: Wed, 5 Aug 2026 22:51:21 +0100 Subject: [PATCH 2/7] fix: enforce immutable clang-dumper include cache --- .../src/pt/up/fe/specs/clang/CacheFiles.java | 27 +++- .../pt/up/fe/specs/clang/ClangResources.java | 91 +++++------ .../up/fe/specs/clang/ClangResourcesTest.java | 150 ++++++++++++++---- 3 files changed, 183 insertions(+), 85 deletions(-) diff --git a/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java b/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java index 004b902c5..7434f04a2 100644 --- a/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java +++ b/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java @@ -25,6 +25,7 @@ import java.nio.file.FileAlreadyExistsException; import java.nio.file.FileSystemException; import java.nio.file.Files; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.nio.file.attribute.FileTime; @@ -123,6 +124,11 @@ static void touch(Path path) { } static void deleteStaleDirectories(Path parent, Instant cutoff, Path excluded) { + deleteStaleDirectories(parent, cutoff, excluded, () -> {}); + } + + static void deleteStaleDirectories(Path parent, Instant cutoff, Path excluded, + Runnable beforeRevalidation) { if (!Files.isDirectory(parent)) { return; } @@ -137,7 +143,7 @@ static void deleteStaleDirectories(Path parent, Instant cutoff, Path excluded) { continue; } - if (Files.getLastModifiedTime(child).toInstant().isBefore(cutoff)) { + if (isStaleAndUnchanged(child, cutoff, beforeRevalidation)) { delete(child); } } @@ -158,7 +164,7 @@ static void deleteStaleStagingDirectories(Path parent, Instant cutoff) { continue; } - if (Files.getLastModifiedTime(child).toInstant().isBefore(cutoff)) { + if (isStaleAndUnchanged(child, cutoff, () -> {})) { delete(child); } } @@ -168,6 +174,23 @@ static void deleteStaleStagingDirectories(Path parent, Instant cutoff) { } } + private static boolean isStaleAndUnchanged(Path path, Instant cutoff, Runnable beforeRevalidation) { + try { + var initialMtime = Files.getLastModifiedTime(path); + if (!initialMtime.toInstant().isBefore(cutoff)) { + return false; + } + + beforeRevalidation.run(); + var currentMtime = Files.getLastModifiedTime(path); + return initialMtime.equals(currentMtime); + } catch (NoSuchFileException e) { + return false; + } catch (IOException e) { + throw new UncheckedIOException("Could not inspect cache path '" + path + "'", e); + } + } + static void delete(Path path) { if (!Files.exists(path)) { return; diff --git a/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java b/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java index f83098dcb..0e0012964 100644 --- a/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java +++ b/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java @@ -23,6 +23,7 @@ import pt.up.fe.specs.util.SpecsIo; import pt.up.fe.specs.util.SpecsLogs; import pt.up.fe.specs.util.SpecsSystem; +import pt.up.fe.specs.util.providers.FileResourceProvider; import pt.up.fe.specs.util.system.ProcessOutputAsString; import java.io.File; @@ -57,35 +58,19 @@ public ClangResources(CodeParser options) { public File getBuiltinCudaLib() { var cudaResourceFolder = SpecsIo.mkdir(options.get(CodeParser.DUMPER_FOLDER), "cuda"); - var cudaFolder = new File(cudaResourceFolder, "cudalib"); - var zipFile = CacheFiles.installFile(new File(cudaResourceFolder, ClangAstWebResource.CUDA_LIB_FILENAME), - ClangAstWebResource.CUDA_LIB, null, "built-in CUDA archive"); + var cudaFolder = SpecsIo.mkdir(cudaResourceFolder, "cudalib"); + var zipFile = ClangAstWebResource.CUDA_LIB.writeVersioned(cudaResourceFolder, ClangResources.class); - if (isCudaInstallation(cudaFolder)) { - return cudaFolder; + if (zipFile.isNewFile() || !isCudaInstallation(cudaFolder)) { + SpecsIo.deleteFolderContents(cudaFolder); + SpecsIo.extractZip(zipFile.getFile(), cudaFolder); } - if (cudaFolder.exists()) { - CacheFiles.delete(cudaFolder.toPath()); - } - - var stagingFolder = CacheFiles.createStagingDirectory(cudaResourceFolder.toPath(), ".cudalib.tmp-"); - try { - if (!SpecsIo.extractZip(zipFile, stagingFolder.toFile()) || !isCudaInstallation(stagingFolder.toFile())) { - throw new RuntimeException("Built-in CUDA archive did not contain a valid CUDA installation"); - } - - return CacheFiles.publish(stagingFolder, cudaFolder.toPath()).toFile(); - } finally { - CacheFiles.delete(stagingFolder); - } + return cudaFolder; } private static boolean isCudaInstallation(File folder) { - return folder.isDirectory() - && new File(folder, "include/cuda.h").isFile() - && new File(folder, "include/cuda_runtime.h").isFile() - && new File(folder, "nvvm/libdevice/libdevice.10.bc").isFile(); + return folder.isDirectory() && new File(folder, "include/cuda_runtime.h").isFile(); } public ClangFiles getClangFiles(LibcMode libcMode) { @@ -101,8 +86,11 @@ public ClangFiles getClangFiles(LibcMode libcMode) { var key = libcMode.name() + "_" + useBuiltinCuda + "_" + source + "_" + resourceFolder.getAbsolutePath(); var cached = CLANG_FILES_CACHE.get(key); - if (isUsable(cached)) { + if (cached != null) { touchUse(resourceFolder, cached.includesFolder()); + } + + if (isUsable(cached)) { SpecsLogs.debug(() -> "Using cached version of Clang files: " + cached.files()); return cached.files(); } @@ -223,10 +211,6 @@ private File getIncludesRoot() { return new File(options.get(CodeParser.DUMPER_FOLDER), INCLUDES_FOLDERNAME); } - private File getIncludesFolder(String sha256) { - return getSharedIncludesFolder(options.get(CodeParser.DUMPER_FOLDER), sha256); - } - static File getSharedIncludesFolder(File cacheFolder, String sha256) { return new File(new File(cacheFolder, INCLUDES_FOLDERNAME), sha256.toLowerCase(Locale.ROOT)); } @@ -304,21 +288,30 @@ private PreparedIncludes prepareIncludes(ClangDumperManifest manifest, File clan private File prepareIncludesFolder(ClangDumperManifest manifest) { var includesAsset = getCurrentAsset(manifest, "includes"); - var extractedFolder = getIncludesFolder(includesAsset.sha256()); + return resolveIncludes(options.get(CodeParser.DUMPER_FOLDER), includesAsset, + ClangAstWebResource.getAssetResource(includesAsset)); + } - if (isIncludesCacheValid(extractedFolder)) { - return extractedFolder; + static File resolveIncludes(File cacheFolder, ClangDumperManifestAsset includesAsset, + FileResourceProvider archiveResource) { + var extractedFolder = getSharedIncludesFolder(cacheFolder, includesAsset.sha256()); + if (extractedFolder.exists()) { + CacheFiles.touch(extractedFolder.toPath()); + if (isIncludesCacheValid(extractedFolder)) { + return extractedFolder; + } + + throw invalidIncludesCache(extractedFolder, includesAsset.sha256()); } - var includesRoot = getIncludesRoot(); - CacheFiles.deleteStaleStagingDirectories(includesRoot.toPath(), - Instant.now().minus(STALE_STAGING_MAX_AGE)); - var stagingFolder = CacheFiles.createStagingDirectory(includesRoot.toPath(), + var includesRoot = extractedFolder.getParentFile().toPath(); + CacheFiles.deleteStaleStagingDirectories(includesRoot, Instant.now().minus(STALE_STAGING_MAX_AGE)); + var stagingFolder = CacheFiles.createStagingDirectory(includesRoot, "." + includesAsset.sha256() + ".tmp-"); try { var downloadFolder = CacheFiles.createStagingDirectory(stagingFolder, ".download-"); try { - var archive = ClangAstWebResource.getAssetResource(includesAsset).write(downloadFolder.toFile()); + var archive = archiveResource.write(downloadFolder.toFile()); if (archive == null || !archive.isFile()) { throw new RuntimeException("Could not download clang-dumper includes archive '" + includesAsset.filename() + "'"); @@ -338,27 +331,33 @@ private File prepareIncludesFolder(ClangDumperManifest manifest) { } getIncludeFolders(stagingFolder.toFile()); - if (isIncludesCacheValid(extractedFolder)) { + if (extractedFolder.exists()) { CacheFiles.touch(extractedFolder.toPath()); - return extractedFolder; - } + if (isIncludesCacheValid(extractedFolder)) { + return extractedFolder; + } - if (extractedFolder.exists()) { - CacheFiles.delete(extractedFolder.toPath()); + throw invalidIncludesCache(extractedFolder, includesAsset.sha256()); } var publishedFolder = CacheFiles.publish(stagingFolder, extractedFolder.toPath()).toFile(); + CacheFiles.touch(publishedFolder.toPath()); if (!isIncludesCacheValid(publishedFolder)) { - throw new RuntimeException("Published clang-dumper includes are invalid: '" + publishedFolder + "'"); + throw invalidIncludesCache(publishedFolder, includesAsset.sha256()); } - CacheFiles.touch(publishedFolder.toPath()); return publishedFolder; } finally { CacheFiles.delete(stagingFolder); } } + private static RuntimeException invalidIncludesCache(File includesFolder, String sha256) { + return new RuntimeException("Invalid clang-dumper includes cache directory '" + + includesFolder.getAbsolutePath() + "' for SHA-256 '" + sha256 + + "'; delete this directory manually to regenerate"); + } + static List getIncludeFolders(File extractedFolder) { if (!extractedFolder.isDirectory()) { throw new RuntimeException("Could not find extracted clang-dumper includes folder '" + extractedFolder + "'"); @@ -406,11 +405,7 @@ static boolean isIncludesCacheValid(File includesFolder) { private void updateLastUsedAndCleanupStaleVersions(File resourceFolder, File includesFolder) { var now = Instant.now(); touchUse(resourceFolder, includesFolder); - - var staleCleanup = new Thread(() -> deleteStaleVersions(now, resourceFolder, includesFolder), - "clang-dumper-stale-cache-cleanup"); - staleCleanup.setDaemon(true); - staleCleanup.start(); + deleteStaleVersions(now, resourceFolder, includesFolder); } void deleteStaleVersions(Instant now, File currentVersionFolder) { diff --git a/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java b/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java index e074dc952..80b7447d4 100644 --- a/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java +++ b/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java @@ -26,18 +26,24 @@ import java.io.File; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; import java.nio.file.attribute.FileTime; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; +import java.util.HexFormat; import java.util.List; import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -138,17 +144,58 @@ public void entrypointsPreserveDeclaredIncludeOrder() throws IOException { } @Test - public void sharedIncludesAreAddressedOnlyBySha() throws IOException { - var sha = "a".repeat(64); - var sharedFolder = ClangResources.getSharedIncludesFolder(tempFolder.toFile(), sha); - Files.createDirectories(sharedFolder.toPath().resolve("builtin")); - Files.writeString(sharedFolder.toPath().resolve("entrypoints.txt"), "builtin\n"); - - var firstRelease = tempFolder.resolve("releases/v1").toFile(); - var secondRelease = tempFolder.resolve("releases/v2").toFile(); + public void releasesWithTheSameIncludesShaShareOneExtraction() throws Exception { + var archive = createIncludesArchive(); + var sha = sha256(archive); + var firstAsset = new ClangDumperManifestAsset("v1-includes.zip", "includes", "linux", "x64", 18, sha); + var secondAsset = new ClangDumperManifestAsset("v2-includes.zip", "includes", "linux", "x64", 18, sha); + var firstManifest = new ClangDumperManifest(1, List.of(firstAsset)); + var secondManifest = new ClangDumperManifest(1, List.of(secondAsset)); + var firstRelease = Files.createDirectories(tempFolder.resolve("releases/v1")); + var secondRelease = Files.createDirectories(tempFolder.resolve("releases/v2")); + var firstWrites = new AtomicInteger(); + var secondWrites = new AtomicInteger(); + + firstManifest.validate(); + secondManifest.validate(); assertNotEquals(firstRelease, secondRelease); - assertEquals(sharedFolder, ClangResources.getSharedIncludesFolder(tempFolder.toFile(), sha)); - assertTrue(ClangResources.isIncludesCacheValid(sharedFolder)); + + var firstIncludes = ClangResources.resolveIncludes(tempFolder.toFile(), firstAsset, + copyingResource(archive, firstWrites)); + var secondIncludes = ClangResources.resolveIncludes(tempFolder.toFile(), secondAsset, + copyingResource(archive, secondWrites)); + + assertEquals(firstAsset, firstManifest.getAsset("linux", "x64", "includes")); + assertEquals(secondAsset, secondManifest.getAsset("linux", "x64", "includes")); + assertEquals(firstIncludes, secondIncludes); + assertEquals(1, firstWrites.get()); + assertEquals(0, secondWrites.get()); + assertTrue(ClangResources.isIncludesCacheValid(firstIncludes)); + try (var children = Files.list(tempFolder.resolve("includes"))) { + assertEquals(1, children.filter(Files::isDirectory).count()); + } + } + + @Test + public void invalidPublishedIncludesFailWithoutRepair() throws IOException { + var sha = "a".repeat(64); + var invalidFolder = ClangResources.getSharedIncludesFolder(tempFolder.toFile(), sha); + Files.createDirectories(invalidFolder.toPath()); + Files.writeString(invalidFolder.toPath().resolve("entrypoints.txt"), "missing\n"); + var writes = new AtomicInteger(); + var unusedArchive = tempFolder.resolve("unused.zip"); + + var error = assertThrows(RuntimeException.class, + () -> ClangResources.resolveIncludes(tempFolder.toFile(), + new ClangDumperManifestAsset("includes.zip", "includes", "linux", "x64", 18, sha), + copyingResource(unusedArchive, writes))); + + assertTrue(error.getMessage().contains(invalidFolder.getAbsolutePath())); + assertTrue(error.getMessage().contains(sha)); + assertTrue(error.getMessage().contains("delete this directory manually to regenerate")); + assertTrue(invalidFolder.isDirectory()); + assertEquals("missing\n", Files.readString(invalidFolder.toPath().resolve("entrypoints.txt"))); + assertEquals(0, writes.get()); } @Test @@ -182,29 +229,37 @@ public void existingReleaseResourceIsReused() throws IOException { } @Test - public void concurrentPublicationLeavesOneValidIncludesTree() throws Exception { - var includesRoot = Files.createDirectories(tempFolder.resolve("includes")); - var sha = "b".repeat(64); - var finalFolder = includesRoot.resolve(sha); + public void concurrentInitializationLeavesOneValidIncludesTree() throws Exception { + var archive = createIncludesArchive(); + var sha = sha256(archive); + var asset = new ClangDumperManifestAsset("includes.zip", "includes", "linux", "x64", 18, sha); + var writes = new AtomicInteger(); var executor = Executors.newFixedThreadPool(4); var futures = new ArrayList>(); try { for (int i = 0; i < 4; i++) { - futures.add(executor.submit(() -> publishTestIncludes(includesRoot, finalFolder, sha))); + futures.add(executor.submit(() -> ClangResources.resolveIncludes(tempFolder.toFile(), asset, + copyingResource(archive, writes)).toPath())); } for (var future : futures) { - assertEquals(finalFolder, future.get(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS)); + assertEquals(ClangResources.getSharedIncludesFolder(tempFolder.toFile(), sha).toPath(), + future.get(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS)); } } finally { executor.shutdownNow(); executor.awaitTermination(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); } - assertTrue(ClangResources.isIncludesCacheValid(finalFolder.toFile())); - try (var children = Files.list(includesRoot)) { - assertTrue(children.noneMatch(path -> path.getFileName().toString().startsWith("." + sha + ".tmp-"))); + var finalFolder = ClangResources.getSharedIncludesFolder(tempFolder.toFile(), sha); + assertTrue(ClangResources.isIncludesCacheValid(finalFolder)); + assertTrue(writes.get() >= 1); + try (var children = Files.list(finalFolder.toPath().getParent())) { + var childPaths = children.toList(); + assertEquals(1, childPaths.stream().filter(Files::isDirectory).count()); + assertTrue(childPaths.stream() + .noneMatch(path -> path.getFileName().toString().startsWith("." + sha + ".tmp-"))); } } @@ -242,13 +297,17 @@ public void staleReleaseAndSharedIncludesAreRemovedAfterSixtyDays() throws IOExc @Test public void usingSharedIncludesRefreshesItsLastUsedTime() throws IOException { - var shared = Files.createDirectories( - ClangResources.getSharedIncludesFolder(tempFolder.toFile(), "d".repeat(64)).toPath()); + var sha = "d".repeat(64); + var shared = Files.createDirectories(ClangResources.getSharedIncludesFolder(tempFolder.toFile(), sha).toPath()); Files.createDirectories(shared.resolve("builtin")); Files.writeString(shared.resolve("entrypoints.txt"), "builtin\n"); Files.setLastModifiedTime(shared, FileTime.from(Instant.now().minus(Duration.ofDays(61)))); - - CacheFiles.touch(shared); + var writes = new AtomicInteger(); + var unusedArchive = tempFolder.resolve("unused.zip"); + var asset = new ClangDumperManifestAsset("includes.zip", "includes", "linux", "x64", 18, sha); + assertEquals(shared.toFile(), ClangResources.resolveIncludes(tempFolder.toFile(), asset, + copyingResource(unusedArchive, writes))); + assertEquals(0, writes.get()); var parser = CodeParser.newInstance(); parser.set(CodeParser.DUMPER_FOLDER, tempFolder.toFile()); @@ -258,6 +317,18 @@ public void usingSharedIncludesRefreshesItsLastUsedTime() throws IOException { assertTrue(Files.exists(shared)); } + @Test + public void cleanupRevalidationKeepsAnEntryTouchedDuringDeletionCheck() throws IOException { + var releases = Files.createDirectories(tempFolder.resolve("releases")); + var stale = Files.createDirectories(releases.resolve("stale")); + Files.setLastModifiedTime(stale, FileTime.from(Instant.now().minus(Duration.ofDays(61)))); + + CacheFiles.deleteStaleDirectories(releases, Instant.now().minus(Duration.ofDays(60)), null, + () -> CacheFiles.touch(stale)); + + assertTrue(Files.exists(stale)); + } + @Test public void releaseResourcesCanBeInitializedBySeparateJvms() throws Exception { var cacheFolder = Files.createDirectory(tempFolder.resolve("cache")).toFile(); @@ -396,19 +467,28 @@ public String getFilename() { }; } - private static Path publishTestIncludes(Path includesRoot, Path finalFolder, String sha) { - Path staging = CacheFiles.createStagingDirectory(includesRoot, "." + sha + ".tmp-"); - try { - try { - Files.createDirectories(staging.resolve("builtin")); - Files.writeString(staging.resolve("entrypoints.txt"), "builtin\n"); - } catch (IOException e) { - throw new RuntimeException(e); - } + private Path createIncludesArchive() throws IOException { + var archive = tempFolder.resolve("includes.zip"); + try (var zip = new ZipOutputStream(Files.newOutputStream(archive))) { + zip.putNextEntry(new ZipEntry("builtin/")); + zip.closeEntry(); + zip.putNextEntry(new ZipEntry("entrypoints.txt")); + zip.write("builtin\n".getBytes(StandardCharsets.UTF_8)); + zip.closeEntry(); + zip.putNextEntry(new ZipEntry("builtin/header.h")); + zip.write("header\n".getBytes(StandardCharsets.UTF_8)); + zip.closeEntry(); + } - return CacheFiles.publish(staging, finalFolder); - } finally { - CacheFiles.delete(staging); + return archive; + } + + private static String sha256(Path file) throws IOException { + try { + return HexFormat.of().formatHex(MessageDigest.getInstance("SHA-256") + .digest(Files.readAllBytes(file))); + } catch (NoSuchAlgorithmException e) { + throw new AssertionError(e); } } From ff0ecadaca73a4c6d70457cee2fcfb599c5b4d4c Mon Sep 17 00:00:00 2001 From: "L. Sousa" Date: Wed, 5 Aug 2026 23:14:23 +0100 Subject: [PATCH 3/7] fix: serialize clang-dumper cache maintenance --- .../src/pt/up/fe/specs/clang/CacheFiles.java | 60 +++--- .../pt/up/fe/specs/clang/ClangResources.java | 116 +++++++---- .../up/fe/specs/clang/ClangResourcesTest.java | 185 +++++++++++++++++- 3 files changed, 295 insertions(+), 66 deletions(-) diff --git a/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java b/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java index 7434f04a2..fb913ac04 100644 --- a/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java +++ b/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.io.OutputStream; import java.io.UncheckedIOException; +import java.nio.channels.FileChannel; import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.DirectoryStream; import java.nio.file.FileAlreadyExistsException; @@ -28,18 +29,46 @@ import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; import java.nio.file.attribute.FileTime; import java.security.DigestInputStream; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.time.Instant; import java.util.HexFormat; +import java.util.function.Supplier; final class CacheFiles { + // FileChannel rejects overlapping locks in one JVM; this monitor serializes the small critical section. + private static final Object MAINTENANCE_MONITOR = new Object(); + private static final String MAINTENANCE_LOCK_FILENAME = ".maintenance.lock"; + private CacheFiles() { } + static T withMaintenanceLock(Path cacheRoot, Supplier action) { + var lockPath = cacheRoot.resolve(MAINTENANCE_LOCK_FILENAME); + synchronized (MAINTENANCE_MONITOR) { + try { + Files.createDirectories(cacheRoot); + try (var channel = FileChannel.open(lockPath, StandardOpenOption.CREATE, StandardOpenOption.WRITE); + var ignored = channel.lock()) { + return action.get(); + } + } catch (IOException e) { + throw new UncheckedIOException("Could not access cache maintenance lock '" + lockPath + "'", e); + } + } + } + + static void withMaintenanceLock(Path cacheRoot, Runnable action) { + withMaintenanceLock(cacheRoot, () -> { + action.run(); + return null; + }); + } + static Path createStagingDirectory(Path parent, String prefix) { try { Files.createDirectories(parent); @@ -123,12 +152,7 @@ static void touch(Path path) { } } - static void deleteStaleDirectories(Path parent, Instant cutoff, Path excluded) { - deleteStaleDirectories(parent, cutoff, excluded, () -> {}); - } - - static void deleteStaleDirectories(Path parent, Instant cutoff, Path excluded, - Runnable beforeRevalidation) { + static void deleteStaleDirectories(Path cacheRoot, Path parent, Instant cutoff, Path excluded) { if (!Files.isDirectory(parent)) { return; } @@ -143,16 +167,14 @@ static void deleteStaleDirectories(Path parent, Instant cutoff, Path excluded, continue; } - if (isStaleAndUnchanged(child, cutoff, beforeRevalidation)) { - delete(child); - } + withMaintenanceLock(cacheRoot, () -> deleteIfStale(child, cutoff)); } } catch (IOException e) { throw new UncheckedIOException("Could not clean stale cache directories below '" + parent + "'", e); } } - static void deleteStaleStagingDirectories(Path parent, Instant cutoff) { + static void deleteStaleStagingDirectories(Path cacheRoot, Path parent, Instant cutoff) { if (!Files.isDirectory(parent)) { return; } @@ -164,9 +186,7 @@ static void deleteStaleStagingDirectories(Path parent, Instant cutoff) { continue; } - if (isStaleAndUnchanged(child, cutoff, () -> {})) { - delete(child); - } + withMaintenanceLock(cacheRoot, () -> deleteIfStale(child, cutoff)); } } catch (IOException e) { throw new UncheckedIOException("Could not clean stale cache staging directories below '" + parent + "'", @@ -174,18 +194,14 @@ static void deleteStaleStagingDirectories(Path parent, Instant cutoff) { } } - private static boolean isStaleAndUnchanged(Path path, Instant cutoff, Runnable beforeRevalidation) { + private static void deleteIfStale(Path path, Instant cutoff) { try { - var initialMtime = Files.getLastModifiedTime(path); - if (!initialMtime.toInstant().isBefore(cutoff)) { - return false; + if (Files.isDirectory(path) + && Files.getLastModifiedTime(path).toInstant().isBefore(cutoff)) { + delete(path); } - - beforeRevalidation.run(); - var currentMtime = Files.getLastModifiedTime(path); - return initialMtime.equals(currentMtime); } catch (NoSuchFileException e) { - return false; + // Another cleanup or publisher already removed the candidate. } catch (IOException e) { throw new UncheckedIOException("Could not inspect cache path '" + path + "'", e); } diff --git a/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java b/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java index 0e0012964..57a628ae6 100644 --- a/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java +++ b/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java @@ -86,10 +86,6 @@ public ClangFiles getClangFiles(LibcMode libcMode) { var key = libcMode.name() + "_" + useBuiltinCuda + "_" + source + "_" + resourceFolder.getAbsolutePath(); var cached = CLANG_FILES_CACHE.get(key); - if (cached != null) { - touchUse(resourceFolder, cached.includesFolder()); - } - if (isUsable(cached)) { SpecsLogs.debug(() -> "Using cached version of Clang files: " + cached.files()); return cached.files(); @@ -119,17 +115,41 @@ public ClangFiles getClangFiles(LibcMode libcMode) { return selectedFiles.files(); } - private static boolean isUsable(CachedClangFiles cached) { - return cached != null - && cached.files().clangExecutable().isFile() - && (cached.includesFolder() == null || isIncludesCacheValid(cached.includesFolder())); - } + private boolean isUsable(CachedClangFiles cached) { + if (cached == null) { + return false; + } + + return CacheFiles.withMaintenanceLock(options.get(CodeParser.DUMPER_FOLDER).toPath(), () -> { + if (!cached.files().clangExecutable().isFile()) { + return false; + } + + var includesFolder = cached.includesFolder(); + if (includesFolder == null) { + return true; + } + + if (!includesFolder.exists()) { + return false; + } - private static void touchUse(File resourceFolder, File includesFolder) { - CacheFiles.touch(resourceFolder.toPath()); - if (includesFolder != null) { CacheFiles.touch(includesFolder.toPath()); - } + if (!isIncludesCacheValid(includesFolder)) { + throw invalidIncludesCache(includesFolder, includesFolder.getName()); + } + + return true; + }); + } + + private void touchUse(File resourceFolder, File includesFolder) { + CacheFiles.withMaintenanceLock(options.get(CodeParser.DUMPER_FOLDER).toPath(), () -> { + CacheFiles.touch(resourceFolder.toPath()); + if (includesFolder != null) { + CacheFiles.touch(includesFolder.toPath()); + } + }); } static File getLocalExecutable(File buildFolder) { @@ -194,9 +214,12 @@ private void unblockWindowsFile(File executable) { } public File getClangResourceFolder() { - var releaseFolder = SpecsIo.mkdir(getReleasesFolder(), ClangAstWebResource.getReleaseTag()); - CacheFiles.touch(releaseFolder.toPath()); - return releaseFolder; + var cacheFolder = options.get(CodeParser.DUMPER_FOLDER); + return CacheFiles.withMaintenanceLock(cacheFolder.toPath(), () -> { + var releaseFolder = SpecsIo.mkdir(getReleasesFolder(), ClangAstWebResource.getReleaseTag()); + CacheFiles.touch(releaseFolder.toPath()); + return releaseFolder; + }); } public static File getDefaultTempFolder() { @@ -295,17 +318,14 @@ private File prepareIncludesFolder(ClangDumperManifest manifest) { static File resolveIncludes(File cacheFolder, ClangDumperManifestAsset includesAsset, FileResourceProvider archiveResource) { var extractedFolder = getSharedIncludesFolder(cacheFolder, includesAsset.sha256()); - if (extractedFolder.exists()) { - CacheFiles.touch(extractedFolder.toPath()); - if (isIncludesCacheValid(extractedFolder)) { - return extractedFolder; - } - - throw invalidIncludesCache(extractedFolder, includesAsset.sha256()); + var existingFolder = useExistingIncludes(cacheFolder, extractedFolder, includesAsset.sha256()); + if (existingFolder != null) { + return existingFolder; } var includesRoot = extractedFolder.getParentFile().toPath(); - CacheFiles.deleteStaleStagingDirectories(includesRoot, Instant.now().minus(STALE_STAGING_MAX_AGE)); + CacheFiles.deleteStaleStagingDirectories(cacheFolder.toPath(), includesRoot, + Instant.now().minus(STALE_STAGING_MAX_AGE)); var stagingFolder = CacheFiles.createStagingDirectory(includesRoot, "." + includesAsset.sha256() + ".tmp-"); try { @@ -331,27 +351,43 @@ static File resolveIncludes(File cacheFolder, ClangDumperManifestAsset includesA } getIncludeFolders(stagingFolder.toFile()); - if (extractedFolder.exists()) { - CacheFiles.touch(extractedFolder.toPath()); - if (isIncludesCacheValid(extractedFolder)) { - return extractedFolder; - } - - throw invalidIncludesCache(extractedFolder, includesAsset.sha256()); + existingFolder = useExistingIncludes(cacheFolder, extractedFolder, includesAsset.sha256()); + if (existingFolder != null) { + return existingFolder; } var publishedFolder = CacheFiles.publish(stagingFolder, extractedFolder.toPath()).toFile(); - CacheFiles.touch(publishedFolder.toPath()); - if (!isIncludesCacheValid(publishedFolder)) { - throw invalidIncludesCache(publishedFolder, includesAsset.sha256()); + existingFolder = useExistingIncludes(cacheFolder, publishedFolder, includesAsset.sha256()); + if (existingFolder == null) { + throw new RuntimeException("Published clang-dumper includes disappeared: '" + + publishedFolder.getAbsolutePath() + "'"); } - return publishedFolder; + return existingFolder; } finally { CacheFiles.delete(stagingFolder); } } + private static File useExistingIncludes(File cacheFolder, File includesFolder, String sha256) { + if (!includesFolder.exists()) { + return null; + } + + return CacheFiles.withMaintenanceLock(cacheFolder.toPath(), () -> { + if (!includesFolder.exists()) { + return null; + } + + CacheFiles.touch(includesFolder.toPath()); + if (!isIncludesCacheValid(includesFolder)) { + throw invalidIncludesCache(includesFolder, sha256); + } + + return includesFolder; + }); + } + private static RuntimeException invalidIncludesCache(File includesFolder, String sha256) { return new RuntimeException("Invalid clang-dumper includes cache directory '" + includesFolder.getAbsolutePath() + "' for SHA-256 '" + sha256 @@ -414,13 +450,15 @@ void deleteStaleVersions(Instant now, File currentVersionFolder) { private void deleteStaleVersions(Instant now, File currentVersionFolder, File currentIncludesFolder) { var cutoff = now.minus(STALE_CACHE_MAX_AGE); + var cacheRoot = options.get(CodeParser.DUMPER_FOLDER).toPath(); try { - CacheFiles.deleteStaleDirectories(getReleasesFolder().toPath(), cutoff, currentVersionFolder.toPath()); - CacheFiles.deleteStaleDirectories(getIncludesRoot().toPath(), cutoff, + CacheFiles.deleteStaleDirectories(cacheRoot, getReleasesFolder().toPath(), cutoff, + currentVersionFolder.toPath()); + CacheFiles.deleteStaleDirectories(cacheRoot, getIncludesRoot().toPath(), cutoff, currentIncludesFolder == null ? null : currentIncludesFolder.toPath()); - CacheFiles.deleteStaleStagingDirectories(getReleasesFolder().toPath(), + CacheFiles.deleteStaleStagingDirectories(cacheRoot, getReleasesFolder().toPath(), now.minus(STALE_STAGING_MAX_AGE)); - CacheFiles.deleteStaleStagingDirectories(getIncludesRoot().toPath(), + CacheFiles.deleteStaleStagingDirectories(cacheRoot, getIncludesRoot().toPath(), now.minus(STALE_STAGING_MAX_AGE)); } catch (RuntimeException e) { SpecsLogs.warn("Could not clean stale clang-dumper cache resources", e); diff --git a/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java b/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java index 80b7447d4..f26472c99 100644 --- a/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java +++ b/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java @@ -24,12 +24,16 @@ import pt.up.fe.specs.clang.parsers.TopLevelNodesParser; import pt.up.fe.specs.util.providers.FileResourceProvider; +import java.io.BufferedReader; import java.io.File; import java.io.IOException; +import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; +import java.nio.channels.FileChannel; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; import java.nio.file.attribute.FileTime; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; @@ -39,6 +43,7 @@ import java.util.HexFormat; import java.util.List; import java.util.concurrent.Executors; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; @@ -269,7 +274,7 @@ public void abandonedStagingDirectoriesAreCleaned() throws IOException { var staging = CacheFiles.createStagingDirectory(includesRoot, ".sha.tmp-"); Files.setLastModifiedTime(staging, FileTime.from(Instant.now().minus(Duration.ofHours(2)))); - CacheFiles.deleteStaleStagingDirectories(includesRoot, Instant.now().minus(Duration.ofHours(1))); + CacheFiles.deleteStaleStagingDirectories(tempFolder, includesRoot, Instant.now().minus(Duration.ofHours(1))); assertFalse(Files.exists(staging)); } @@ -318,15 +323,60 @@ public void usingSharedIncludesRefreshesItsLastUsedTime() throws IOException { } @Test - public void cleanupRevalidationKeepsAnEntryTouchedDuringDeletionCheck() throws IOException { + public void maintenanceLockMakesUsageWinOverContendingCleanup() throws Exception { var releases = Files.createDirectories(tempFolder.resolve("releases")); var stale = Files.createDirectories(releases.resolve("stale")); Files.setLastModifiedTime(stale, FileTime.from(Instant.now().minus(Duration.ofDays(61)))); + var usageStarted = new CountDownLatch(1); + var allowUsageToFinish = new CountDownLatch(1); + var cleanupStarted = new CountDownLatch(1); + var executor = Executors.newFixedThreadPool(2); - CacheFiles.deleteStaleDirectories(releases, Instant.now().minus(Duration.ofDays(60)), null, - () -> CacheFiles.touch(stale)); + try { + var usage = executor.submit(() -> CacheFiles.withMaintenanceLock(tempFolder, () -> { + CacheFiles.touch(stale); + usageStarted.countDown(); + awaitLatch(allowUsageToFinish); + })); + assertTrue(usageStarted.await(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS)); + + var cleanup = executor.submit(() -> { + cleanupStarted.countDown(); + CacheFiles.deleteStaleDirectories(tempFolder, releases, + Instant.now().minus(Duration.ofDays(60)), null); + }); + assertTrue(cleanupStarted.await(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS)); + assertFalse(cleanup.isDone()); + + allowUsageToFinish.countDown(); + usage.get(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); + cleanup.get(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); + assertTrue(Files.exists(stale)); + } finally { + allowUsageToFinish.countDown(); + executor.shutdownNow(); + executor.awaitTermination(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); + } + } - assertTrue(Files.exists(stale)); + @Test + public void cleanupWinnerMakesSubsequentIncludesResolutionAcknowledgeTheMiss() throws Exception { + var archive = createIncludesArchive(); + var sha = sha256(archive); + var asset = new ClangDumperManifestAsset("includes.zip", "includes", "linux", "x64", 18, sha); + var shared = ClangResources.getSharedIncludesFolder(tempFolder.toFile(), sha); + Files.createDirectories(shared.toPath().resolve("builtin")); + Files.writeString(shared.toPath().resolve("entrypoints.txt"), "builtin\n"); + Files.setLastModifiedTime(shared.toPath(), FileTime.from(Instant.now().minus(Duration.ofDays(61)))); + + CacheFiles.deleteStaleDirectories(tempFolder, shared.toPath().getParent(), + Instant.now().minus(Duration.ofDays(60)), null); + assertFalse(shared.exists()); + + var writes = new AtomicInteger(); + assertEquals(shared, ClangResources.resolveIncludes(tempFolder.toFile(), asset, + copyingResource(archive, writes))); + assertEquals(1, writes.get()); } @Test @@ -354,6 +404,50 @@ public void releaseResourcesCanBeInitializedBySeparateJvms() throws Exception { assertTrue(cacheFolder.toPath().resolve("releases").resolve(ClangAstWebResource.getReleaseTag()).toFile().isDirectory()); } + @Test + public void maintenanceLockIsSharedAcrossJvmProcesses() throws Exception { + var holder = startMaintenanceProcess(MaintenanceLockHolderProcess.class, tempFolder); + var contender = (Process) null; + try (var holderOutput = new BufferedReader( + new InputStreamReader(holder.getInputStream(), StandardCharsets.UTF_8))) { + assertEquals("READY", holderOutput.readLine()); + + contender = startMaintenanceProcess(MaintenanceLockProbeProcess.class, tempFolder); + try (var contenderOutput = new BufferedReader( + new InputStreamReader(contender.getInputStream(), StandardCharsets.UTF_8))) { + assertEquals("BLOCKED", contenderOutput.readLine()); + + holder.getOutputStream().write('\n'); + holder.getOutputStream().flush(); + assertEquals("DONE", holderOutput.readLine()); + + contender.getOutputStream().write('\n'); + contender.getOutputStream().flush(); + assertEquals("ENTERED", contenderOutput.readLine()); + assertEquals("DONE", contenderOutput.readLine()); + assertTrue(contender.waitFor(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS)); + assertEquals(0, contender.exitValue()); + } + } finally { + try { + holder.getOutputStream().write('\n'); + holder.getOutputStream().flush(); + } catch (IOException ignored) { + // The holder may already have exited after the assertion path. + } + if (contender != null) { + try { + contender.getOutputStream().write('\n'); + contender.getOutputStream().flush(); + } catch (IOException ignored) { + // The contender may already have exited after the assertion path. + } + } + stopProcess(contender); + stopProcess(holder); + } + } + @Test public void sameJvmInstancesReuseReleaseFilesAndRefreshSharedIncludes() throws Exception { var firstParser = newParser(CodeParser.getBuiltinOption()); @@ -467,6 +561,17 @@ public String getFilename() { }; } + private static void awaitLatch(CountDownLatch latch) { + try { + if (!latch.await(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS)) { + throw new RuntimeException("Timed out waiting for maintenance-lock test coordination"); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException(e); + } + } + private Path createIncludesArchive() throws IOException { var archive = tempFolder.resolve("includes.zip"); try (var zip = new ZipOutputStream(Files.newOutputStream(archive))) { @@ -508,6 +613,19 @@ private Process startResourceProcess(File cacheFolder, Path done, Path log) thro .start(); } + private Process startMaintenanceProcess(Class processClass, Path cacheFolder) throws IOException { + var javaExecutable = Path.of(System.getProperty("java.home"), "bin", + SupportedPlatform.getCurrentPlatform().isWindows() ? "java.exe" : "java"); + + return new ProcessBuilder( + javaExecutable.toString(), + "-cp", + System.getProperty("java.class.path"), + processClass.getName(), + cacheFolder.toAbsolutePath().toString()) + .start(); + } + private void waitForProcess(Process process, Path log) throws Exception { assertTrue(process.waitFor(PROCESS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS), () -> "Child JVM did not finish. Output: " + readLog(log)); @@ -543,4 +661,61 @@ public static void main(String[] args) throws Exception { Files.writeString(Path.of(args[1]), clangFiles.clangExecutable().getAbsolutePath()); } } + + public static final class MaintenanceLockHolderProcess { + + private MaintenanceLockHolderProcess() { + } + + public static void main(String[] args) { + CacheFiles.withMaintenanceLock(Path.of(args[0]), () -> { + System.out.println("READY"); + System.out.flush(); + try { + System.in.read(); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + System.out.println("DONE"); + System.out.flush(); + } + } + + public static final class MaintenanceLockProbeProcess { + + private MaintenanceLockProbeProcess() { + } + + public static void main(String[] args) { + var lockPath = Path.of(args[0], ".maintenance.lock"); + try (var channel = FileChannel.open(lockPath, StandardOpenOption.CREATE, StandardOpenOption.WRITE)) { + var lock = channel.tryLock(); + if (lock != null) { + try (lock) { + System.out.println("ACQUIRED"); + System.out.flush(); + } + return; + } + + System.out.println("BLOCKED"); + System.out.flush(); + } catch (IOException e) { + throw new RuntimeException(e); + } + + try { + System.in.read(); + } catch (IOException e) { + throw new RuntimeException(e); + } + CacheFiles.withMaintenanceLock(Path.of(args[0]), () -> { + System.out.println("ENTERED"); + System.out.flush(); + }); + System.out.println("DONE"); + System.out.flush(); + } + } } From 50bc072ce0c6a963532ace22d95a0e54dd080492 Mon Sep 17 00:00:00 2001 From: "L. Sousa" Date: Wed, 5 Aug 2026 23:43:34 +0100 Subject: [PATCH 4/7] fix: lock clang-dumper staging directories --- .../src/pt/up/fe/specs/clang/CacheFiles.java | 176 ++++++++++++++++-- .../pt/up/fe/specs/clang/ClangResources.java | 32 ++-- .../up/fe/specs/clang/ClangResourcesTest.java | 27 ++- 3 files changed, 204 insertions(+), 31 deletions(-) diff --git a/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java b/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java index fb913ac04..0b02fefb8 100644 --- a/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java +++ b/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java @@ -21,6 +21,8 @@ import java.io.OutputStream; import java.io.UncheckedIOException; import java.nio.channels.FileChannel; +import java.nio.channels.FileLock; +import java.nio.channels.OverlappingFileLockException; import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.DirectoryStream; import java.nio.file.FileAlreadyExistsException; @@ -69,13 +71,123 @@ static void withMaintenanceLock(Path cacheRoot, Runnable action) { }); } - static Path createStagingDirectory(Path parent, String prefix) { + static StagingDirectory createStagingDirectory(Path parent, String prefix) { + Path lockPath; try { Files.createDirectories(parent); - return Files.createTempDirectory(parent, prefix); + lockPath = Files.createTempFile(parent, prefix, ".lock"); } catch (IOException e) { throw new UncheckedIOException("Could not create cache staging directory below '" + parent + "'", e); } + + FileChannel channel = null; + FileLock lock = null; + Path stagingPath = null; + try { + channel = FileChannel.open(lockPath, StandardOpenOption.WRITE); + lock = channel.lock(); + stagingPath = lockPath.resolveSibling(removeLockSuffix(lockPath.getFileName().toString())); + Files.createDirectory(stagingPath); + return new StagingDirectory(stagingPath, lockPath, channel, lock); + } catch (IOException e) { + cleanupStagingCreation(stagingPath, lockPath, channel, lock); + throw new UncheckedIOException("Could not create cache staging directory below '" + parent + "'", e); + } catch (RuntimeException e) { + cleanupStagingCreation(stagingPath, lockPath, channel, lock); + throw e; + } + } + + private static String removeLockSuffix(String filename) { + return filename.substring(0, filename.length() - ".lock".length()); + } + + private static void cleanupStagingCreation(Path stagingPath, Path lockPath, + FileChannel channel, FileLock lock) { + if (lock != null) { + try { + lock.release(); + } catch (IOException ignored) { + // The channel close below also releases the OS lock. + } + } + + if (channel != null) { + try { + channel.close(); + } catch (IOException ignored) { + // Best-effort cleanup after staging creation failed. + } + } + + if (stagingPath != null) { + deleteQuietly(stagingPath); + } + + try { + Files.deleteIfExists(lockPath); + } catch (IOException ignored) { + // Best-effort cleanup after staging creation failed. + } + } + + static final class StagingDirectory implements AutoCloseable { + + private final Path path; + private final Path lockPath; + private final FileChannel channel; + private final FileLock lock; + private boolean closed; + + private StagingDirectory(Path path, Path lockPath, FileChannel channel, FileLock lock) { + this.path = path; + this.lockPath = lockPath; + this.channel = channel; + this.lock = lock; + } + + Path path() { + return path; + } + + Path lockPath() { + return lockPath; + } + + @Override + public void close() { + if (closed) { + return; + } + + closed = true; + IOException failure = null; + try { + lock.release(); + } catch (IOException e) { + failure = e; + } + + try { + channel.close(); + } catch (IOException e) { + if (failure == null) { + failure = e; + } + } + + try { + Files.deleteIfExists(lockPath); + } catch (IOException e) { + if (failure == null) { + failure = e; + } + } + + if (failure != null) { + throw new UncheckedIOException("Could not close cache staging lock '" + lockPath + "'", failure); + } + } } static File installFile(File destination, FileResourceProvider resource, String expectedSha256, @@ -84,10 +196,10 @@ static File installFile(File destination, FileResourceProvider resource, String return destination; } - Path stagingDirectory = createStagingDirectory(destination.getParentFile().toPath(), + var stagingDirectory = createStagingDirectory(destination.getParentFile().toPath(), "." + destination.getName() + ".tmp-"); try { - File stagedFile = resource.write(stagingDirectory.toFile()); + File stagedFile = resource.write(stagingDirectory.path().toFile()); if (stagedFile == null || !stagedFile.isFile()) { throw new RuntimeException("Could not download " + description); } @@ -99,7 +211,11 @@ static File installFile(File destination, FileResourceProvider resource, String return publish(stagedFile.toPath(), destination.toPath()).toFile(); } finally { - deleteQuietly(stagingDirectory); + try { + deleteQuietly(stagingDirectory.path()); + } finally { + stagingDirectory.close(); + } } } @@ -174,7 +290,18 @@ static void deleteStaleDirectories(Path cacheRoot, Path parent, Instant cutoff, } } - static void deleteStaleStagingDirectories(Path cacheRoot, Path parent, Instant cutoff) { + private static void deleteIfStale(Path path, Instant cutoff) { + try { + if (Files.isDirectory(path) + && Files.getLastModifiedTime(path).toInstant().isBefore(cutoff)) { + delete(path); + } + } catch (IOException e) { + throw new UncheckedIOException("Could not inspect cache path '" + path + "'", e); + } + } + + static void deleteUnlockedStagingDirectories(Path parent) { if (!Files.isDirectory(parent)) { return; } @@ -186,24 +313,47 @@ static void deleteStaleStagingDirectories(Path cacheRoot, Path parent, Instant c continue; } - withMaintenanceLock(cacheRoot, () -> deleteIfStale(child, cutoff)); + deleteIfUnlockedStaging(child); } } catch (IOException e) { - throw new UncheckedIOException("Could not clean stale cache staging directories below '" + parent + "'", + throw new UncheckedIOException("Could not clean cache staging directories below '" + parent + "'", e); } } - private static void deleteIfStale(Path path, Instant cutoff) { + private static void deleteIfUnlockedStaging(Path stagingPath) { + var lockPath = stagingPath.resolveSibling(stagingPath.getFileName() + ".lock"); + boolean acquired = false; try { - if (Files.isDirectory(path) - && Files.getLastModifiedTime(path).toInstant().isBefore(cutoff)) { - delete(path); + try (var channel = FileChannel.open(lockPath, StandardOpenOption.CREATE, StandardOpenOption.WRITE)) { + FileLock lock; + try { + lock = channel.tryLock(); + } catch (OverlappingFileLockException e) { + return; + } + + if (lock == null) { + return; + } + + acquired = true; + try (lock) { + delete(stagingPath); + } } } catch (NoSuchFileException e) { // Another cleanup or publisher already removed the candidate. } catch (IOException e) { - throw new UncheckedIOException("Could not inspect cache path '" + path + "'", e); + throw new UncheckedIOException("Could not inspect cache staging lock '" + lockPath + "'", e); + } finally { + if (acquired) { + try { + Files.deleteIfExists(lockPath); + } catch (IOException e) { + throw new UncheckedIOException("Could not delete cache staging lock '" + lockPath + "'", e); + } + } } } diff --git a/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java b/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java index 57a628ae6..ae7b722f6 100644 --- a/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java +++ b/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java @@ -46,7 +46,6 @@ public class ClangResources { private static final String RELEASES_FOLDERNAME = "releases"; private static final String INCLUDES_FOLDERNAME = "includes"; private static final Duration STALE_CACHE_MAX_AGE = Duration.ofDays(60); - private static final Duration STALE_STAGING_MAX_AGE = Duration.ofHours(1); private static final Map HAS_LIBC = new ConcurrentHashMap<>(); @@ -324,14 +323,13 @@ static File resolveIncludes(File cacheFolder, ClangDumperManifestAsset includesA } var includesRoot = extractedFolder.getParentFile().toPath(); - CacheFiles.deleteStaleStagingDirectories(cacheFolder.toPath(), includesRoot, - Instant.now().minus(STALE_STAGING_MAX_AGE)); + CacheFiles.deleteUnlockedStagingDirectories(includesRoot); var stagingFolder = CacheFiles.createStagingDirectory(includesRoot, "." + includesAsset.sha256() + ".tmp-"); try { - var downloadFolder = CacheFiles.createStagingDirectory(stagingFolder, ".download-"); + var downloadFolder = CacheFiles.createStagingDirectory(stagingFolder.path(), ".download-"); try { - var archive = archiveResource.write(downloadFolder.toFile()); + var archive = archiveResource.write(downloadFolder.path().toFile()); if (archive == null || !archive.isFile()) { throw new RuntimeException("Could not download clang-dumper includes archive '" + includesAsset.filename() + "'"); @@ -342,21 +340,25 @@ static File resolveIncludes(File cacheFolder, ClangDumperManifestAsset includesA + "' does not match expected SHA-256 '" + includesAsset.sha256() + "'"); } - if (!SpecsIo.extractZip(archive, stagingFolder.toFile())) { + if (!SpecsIo.extractZip(archive, stagingFolder.path().toFile())) { throw new RuntimeException("Could not extract clang-dumper includes archive '" + includesAsset.filename() + "'"); } } finally { - CacheFiles.delete(downloadFolder); + try { + CacheFiles.delete(downloadFolder.path()); + } finally { + downloadFolder.close(); + } } - getIncludeFolders(stagingFolder.toFile()); + getIncludeFolders(stagingFolder.path().toFile()); existingFolder = useExistingIncludes(cacheFolder, extractedFolder, includesAsset.sha256()); if (existingFolder != null) { return existingFolder; } - var publishedFolder = CacheFiles.publish(stagingFolder, extractedFolder.toPath()).toFile(); + var publishedFolder = CacheFiles.publish(stagingFolder.path(), extractedFolder.toPath()).toFile(); existingFolder = useExistingIncludes(cacheFolder, publishedFolder, includesAsset.sha256()); if (existingFolder == null) { throw new RuntimeException("Published clang-dumper includes disappeared: '" @@ -365,7 +367,11 @@ static File resolveIncludes(File cacheFolder, ClangDumperManifestAsset includesA return existingFolder; } finally { - CacheFiles.delete(stagingFolder); + try { + CacheFiles.delete(stagingFolder.path()); + } finally { + stagingFolder.close(); + } } } @@ -456,10 +462,8 @@ private void deleteStaleVersions(Instant now, File currentVersionFolder, File cu currentVersionFolder.toPath()); CacheFiles.deleteStaleDirectories(cacheRoot, getIncludesRoot().toPath(), cutoff, currentIncludesFolder == null ? null : currentIncludesFolder.toPath()); - CacheFiles.deleteStaleStagingDirectories(cacheRoot, getReleasesFolder().toPath(), - now.minus(STALE_STAGING_MAX_AGE)); - CacheFiles.deleteStaleStagingDirectories(cacheRoot, getIncludesRoot().toPath(), - now.minus(STALE_STAGING_MAX_AGE)); + CacheFiles.deleteUnlockedStagingDirectories(getReleasesFolder().toPath()); + CacheFiles.deleteUnlockedStagingDirectories(getIncludesRoot().toPath()); } catch (RuntimeException e) { SpecsLogs.warn("Could not clean stale clang-dumper cache resources", e); } diff --git a/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java b/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java index f26472c99..1b282e72c 100644 --- a/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java +++ b/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java @@ -269,14 +269,33 @@ public void concurrentInitializationLeavesOneValidIncludesTree() throws Exceptio } @Test - public void abandonedStagingDirectoriesAreCleaned() throws IOException { + public void activelyLockedStagingDirectoriesArePreserved() throws Exception { var includesRoot = Files.createDirectories(tempFolder.resolve("includes")); var staging = CacheFiles.createStagingDirectory(includesRoot, ".sha.tmp-"); - Files.setLastModifiedTime(staging, FileTime.from(Instant.now().minus(Duration.ofHours(2)))); + try { + CacheFiles.deleteUnlockedStagingDirectories(includesRoot); + assertTrue(Files.exists(staging.path())); + assertTrue(Files.exists(staging.lockPath())); + } finally { + staging.close(); + CacheFiles.delete(staging.path()); + } + } + + @Test + public void unlockedStagingDirectoriesAreCleaned() throws Exception { + var includesRoot = Files.createDirectories(tempFolder.resolve("includes")); + var staging = CacheFiles.createStagingDirectory(includesRoot, ".sha.tmp-"); + var stagingPath = staging.path(); + var lockPath = staging.lockPath(); + staging.close(); + Files.createFile(lockPath); - CacheFiles.deleteStaleStagingDirectories(tempFolder, includesRoot, Instant.now().minus(Duration.ofHours(1))); + assertTrue(Files.exists(lockPath)); + CacheFiles.deleteUnlockedStagingDirectories(includesRoot); - assertFalse(Files.exists(staging)); + assertFalse(Files.exists(stagingPath)); + assertFalse(Files.exists(lockPath)); } @Test From 50c2f6c28e2b485215fa1779795a180c11cfe6fa Mon Sep 17 00:00:00 2001 From: "L. Sousa" Date: Wed, 5 Aug 2026 23:58:29 +0100 Subject: [PATCH 5/7] simplify: clean staging locks directly --- .../src/pt/up/fe/specs/clang/CacheFiles.java | 107 ++++-------------- .../pt/up/fe/specs/clang/ClangResources.java | 16 +-- .../up/fe/specs/clang/ClangResourcesTest.java | 15 ++- 3 files changed, 44 insertions(+), 94 deletions(-) diff --git a/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java b/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java index 0b02fefb8..06a6d6e8e 100644 --- a/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java +++ b/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java @@ -81,37 +81,36 @@ static StagingDirectory createStagingDirectory(Path parent, String prefix) { } FileChannel channel = null; - FileLock lock = null; Path stagingPath = null; try { channel = FileChannel.open(lockPath, StandardOpenOption.WRITE); - lock = channel.lock(); + channel.lock(); stagingPath = lockPath.resolveSibling(removeLockSuffix(lockPath.getFileName().toString())); Files.createDirectory(stagingPath); - return new StagingDirectory(stagingPath, lockPath, channel, lock); + return new StagingDirectory(stagingPath, lockPath, channel); } catch (IOException e) { - cleanupStagingCreation(stagingPath, lockPath, channel, lock); + cleanupStagingCreation(stagingPath, lockPath, channel); throw new UncheckedIOException("Could not create cache staging directory below '" + parent + "'", e); } catch (RuntimeException e) { - cleanupStagingCreation(stagingPath, lockPath, channel, lock); + cleanupStagingCreation(stagingPath, lockPath, channel); throw e; } } + static Path createTemporaryDirectory(Path parent, String prefix) { + try { + Files.createDirectories(parent); + return Files.createTempDirectory(parent, prefix); + } catch (IOException e) { + throw new UncheckedIOException("Could not create cache temporary directory below '" + parent + "'", e); + } + } + private static String removeLockSuffix(String filename) { return filename.substring(0, filename.length() - ".lock".length()); } - private static void cleanupStagingCreation(Path stagingPath, Path lockPath, - FileChannel channel, FileLock lock) { - if (lock != null) { - try { - lock.release(); - } catch (IOException ignored) { - // The channel close below also releases the OS lock. - } - } - + private static void cleanupStagingCreation(Path stagingPath, Path lockPath, FileChannel channel) { if (channel != null) { try { channel.close(); @@ -131,61 +130,20 @@ private static void cleanupStagingCreation(Path stagingPath, Path lockPath, } } - static final class StagingDirectory implements AutoCloseable { - - private final Path path; - private final Path lockPath; - private final FileChannel channel; - private final FileLock lock; - private boolean closed; - - private StagingDirectory(Path path, Path lockPath, FileChannel channel, FileLock lock) { - this.path = path; - this.lockPath = lockPath; - this.channel = channel; - this.lock = lock; - } - - Path path() { - return path; - } - - Path lockPath() { - return lockPath; - } + record StagingDirectory(Path path, Path lockPath, FileChannel channel) implements AutoCloseable { @Override public void close() { - if (closed) { - return; - } - - closed = true; - IOException failure = null; - try { - lock.release(); - } catch (IOException e) { - failure = e; - } - try { channel.close(); } catch (IOException e) { - if (failure == null) { - failure = e; - } + throw new UncheckedIOException("Could not close cache staging lock '" + lockPath + "'", e); } try { Files.deleteIfExists(lockPath); } catch (IOException e) { - if (failure == null) { - failure = e; - } - } - - if (failure != null) { - throw new UncheckedIOException("Could not close cache staging lock '" + lockPath + "'", failure); + throw new UncheckedIOException("Could not close cache staging lock '" + lockPath + "'", e); } } } @@ -301,19 +259,14 @@ private static void deleteIfStale(Path path, Instant cutoff) { } } - static void deleteUnlockedStagingDirectories(Path parent) { + static void deleteUnlockedStagingLocks(Path parent) { if (!Files.isDirectory(parent)) { return; } - try (DirectoryStream children = Files.newDirectoryStream(parent)) { - for (Path child : children) { - String name = child.getFileName().toString(); - if (!Files.isDirectory(child) || !name.startsWith(".") || !name.contains(".tmp-")) { - continue; - } - - deleteIfUnlockedStaging(child); + try (DirectoryStream locks = Files.newDirectoryStream(parent, ".*.tmp-*.lock")) { + for (Path lock : locks) { + deleteIfUnlockedStagingLock(lock); } } catch (IOException e) { throw new UncheckedIOException("Could not clean cache staging directories below '" + parent + "'", @@ -321,11 +274,9 @@ static void deleteUnlockedStagingDirectories(Path parent) { } } - private static void deleteIfUnlockedStaging(Path stagingPath) { - var lockPath = stagingPath.resolveSibling(stagingPath.getFileName() + ".lock"); - boolean acquired = false; + private static void deleteIfUnlockedStagingLock(Path lockPath) { try { - try (var channel = FileChannel.open(lockPath, StandardOpenOption.CREATE, StandardOpenOption.WRITE)) { + try (var channel = FileChannel.open(lockPath, StandardOpenOption.WRITE)) { FileLock lock; try { lock = channel.tryLock(); @@ -337,23 +288,15 @@ private static void deleteIfUnlockedStaging(Path stagingPath) { return; } - acquired = true; try (lock) { - delete(stagingPath); + delete(lockPath.resolveSibling(removeLockSuffix(lockPath.getFileName().toString()))); } } + Files.deleteIfExists(lockPath); } catch (NoSuchFileException e) { // Another cleanup or publisher already removed the candidate. } catch (IOException e) { throw new UncheckedIOException("Could not inspect cache staging lock '" + lockPath + "'", e); - } finally { - if (acquired) { - try { - Files.deleteIfExists(lockPath); - } catch (IOException e) { - throw new UncheckedIOException("Could not delete cache staging lock '" + lockPath + "'", e); - } - } } } diff --git a/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java b/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java index ae7b722f6..553018493 100644 --- a/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java +++ b/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java @@ -323,13 +323,13 @@ static File resolveIncludes(File cacheFolder, ClangDumperManifestAsset includesA } var includesRoot = extractedFolder.getParentFile().toPath(); - CacheFiles.deleteUnlockedStagingDirectories(includesRoot); + CacheFiles.deleteUnlockedStagingLocks(includesRoot); var stagingFolder = CacheFiles.createStagingDirectory(includesRoot, "." + includesAsset.sha256() + ".tmp-"); try { - var downloadFolder = CacheFiles.createStagingDirectory(stagingFolder.path(), ".download-"); + var downloadFolder = CacheFiles.createTemporaryDirectory(stagingFolder.path(), ".download-"); try { - var archive = archiveResource.write(downloadFolder.path().toFile()); + var archive = archiveResource.write(downloadFolder.toFile()); if (archive == null || !archive.isFile()) { throw new RuntimeException("Could not download clang-dumper includes archive '" + includesAsset.filename() + "'"); @@ -345,11 +345,7 @@ static File resolveIncludes(File cacheFolder, ClangDumperManifestAsset includesA + includesAsset.filename() + "'"); } } finally { - try { - CacheFiles.delete(downloadFolder.path()); - } finally { - downloadFolder.close(); - } + CacheFiles.delete(downloadFolder); } getIncludeFolders(stagingFolder.path().toFile()); @@ -462,8 +458,8 @@ private void deleteStaleVersions(Instant now, File currentVersionFolder, File cu currentVersionFolder.toPath()); CacheFiles.deleteStaleDirectories(cacheRoot, getIncludesRoot().toPath(), cutoff, currentIncludesFolder == null ? null : currentIncludesFolder.toPath()); - CacheFiles.deleteUnlockedStagingDirectories(getReleasesFolder().toPath()); - CacheFiles.deleteUnlockedStagingDirectories(getIncludesRoot().toPath()); + CacheFiles.deleteUnlockedStagingLocks(getReleasesFolder().toPath()); + CacheFiles.deleteUnlockedStagingLocks(getIncludesRoot().toPath()); } catch (RuntimeException e) { SpecsLogs.warn("Could not clean stale clang-dumper cache resources", e); } diff --git a/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java b/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java index 1b282e72c..c1a51e62c 100644 --- a/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java +++ b/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java @@ -273,7 +273,7 @@ public void activelyLockedStagingDirectoriesArePreserved() throws Exception { var includesRoot = Files.createDirectories(tempFolder.resolve("includes")); var staging = CacheFiles.createStagingDirectory(includesRoot, ".sha.tmp-"); try { - CacheFiles.deleteUnlockedStagingDirectories(includesRoot); + CacheFiles.deleteUnlockedStagingLocks(includesRoot); assertTrue(Files.exists(staging.path())); assertTrue(Files.exists(staging.lockPath())); } finally { @@ -292,12 +292,23 @@ public void unlockedStagingDirectoriesAreCleaned() throws Exception { Files.createFile(lockPath); assertTrue(Files.exists(lockPath)); - CacheFiles.deleteUnlockedStagingDirectories(includesRoot); + CacheFiles.deleteUnlockedStagingLocks(includesRoot); assertFalse(Files.exists(stagingPath)); assertFalse(Files.exists(lockPath)); } + @Test + public void orphanedStagingLocksAreCleaned() throws IOException { + var includesRoot = Files.createDirectories(tempFolder.resolve("includes")); + var lockPath = includesRoot.resolve(".orphan.tmp-123.lock"); + Files.createFile(lockPath); + + CacheFiles.deleteUnlockedStagingLocks(includesRoot); + + assertFalse(Files.exists(lockPath)); + } + @Test public void staleReleaseAndSharedIncludesAreRemovedAfterSixtyDays() throws IOException { var releases = Files.createDirectories(tempFolder.resolve("releases")); From e5627207df05afd0b3858c375d8af3d76a5057b4 Mon Sep 17 00:00:00 2001 From: "L. Sousa" Date: Thu, 6 Aug 2026 00:11:43 +0100 Subject: [PATCH 6/7] fix: serialize staging lock registration --- .../src/pt/up/fe/specs/clang/CacheFiles.java | 14 +++++++++----- .../up/fe/specs/clang/ClangAstWebResource.java | 5 +++-- .../src/pt/up/fe/specs/clang/ClangResources.java | 11 ++++++----- .../pt/up/fe/specs/clang/ClangResourcesTest.java | 16 +++++++++------- 4 files changed, 27 insertions(+), 19 deletions(-) diff --git a/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java b/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java index 06a6d6e8e..3a4290753 100644 --- a/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java +++ b/ClangAstParser/src/pt/up/fe/specs/clang/CacheFiles.java @@ -71,7 +71,11 @@ static void withMaintenanceLock(Path cacheRoot, Runnable action) { }); } - static StagingDirectory createStagingDirectory(Path parent, String prefix) { + static StagingDirectory createStagingDirectory(Path cacheRoot, Path parent, String prefix) { + return withMaintenanceLock(cacheRoot, () -> createStagingDirectoryLocked(parent, prefix)); + } + + private static StagingDirectory createStagingDirectoryLocked(Path parent, String prefix) { Path lockPath; try { Files.createDirectories(parent); @@ -148,13 +152,13 @@ public void close() { } } - static File installFile(File destination, FileResourceProvider resource, String expectedSha256, + static File installFile(Path cacheRoot, File destination, FileResourceProvider resource, String expectedSha256, String description) { if (destination.isFile()) { return destination; } - var stagingDirectory = createStagingDirectory(destination.getParentFile().toPath(), + var stagingDirectory = createStagingDirectory(cacheRoot, destination.getParentFile().toPath(), "." + destination.getName() + ".tmp-"); try { File stagedFile = resource.write(stagingDirectory.path().toFile()); @@ -259,14 +263,14 @@ private static void deleteIfStale(Path path, Instant cutoff) { } } - static void deleteUnlockedStagingLocks(Path parent) { + static void deleteUnlockedStagingLocks(Path cacheRoot, Path parent) { if (!Files.isDirectory(parent)) { return; } try (DirectoryStream locks = Files.newDirectoryStream(parent, ".*.tmp-*.lock")) { for (Path lock : locks) { - deleteIfUnlockedStagingLock(lock); + withMaintenanceLock(cacheRoot, () -> deleteIfUnlockedStagingLock(lock)); } } catch (IOException e) { throw new UncheckedIOException("Could not clean cache staging directories below '" + parent + "'", diff --git a/ClangAstParser/src/pt/up/fe/specs/clang/ClangAstWebResource.java b/ClangAstParser/src/pt/up/fe/specs/clang/ClangAstWebResource.java index cdbc0c2a1..f4e6535df 100644 --- a/ClangAstParser/src/pt/up/fe/specs/clang/ClangAstWebResource.java +++ b/ClangAstParser/src/pt/up/fe/specs/clang/ClangAstWebResource.java @@ -94,8 +94,9 @@ public static ClangDumperManifest getManifest(File resourceFolder) { var releaseTag = getReleaseTag(); var manifestResource = WebResourceProvider.newInstance(getReleaseBaseUrl(releaseTag), MANIFEST_FILENAME, releaseTag); - var manifestFile = CacheFiles.installFile(new File(resourceFolder, MANIFEST_FILENAME), manifestResource, null, - "clang-dumper release manifest"); + var cacheRoot = resourceFolder.toPath().getParent().getParent(); + var manifestFile = CacheFiles.installFile(cacheRoot, new File(resourceFolder, MANIFEST_FILENAME), + manifestResource, null, "clang-dumper release manifest"); var manifest = GSON.fromJson(SpecsIo.read(manifestFile), ClangDumperManifest.class); if (manifest == null) { diff --git a/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java b/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java index 553018493..4de8340c0 100644 --- a/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java +++ b/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java @@ -178,7 +178,8 @@ private File prepareResources(ClangDumperManifest manifest, File resourceFolder) var executableKind = ClangAstDumper.usePlugin() ? "plugin" : "tool"; var asset = getCurrentAsset(manifest, executableKind); - File executable = CacheFiles.installFile(new File(resourceFolder, asset.filename()), + File executable = CacheFiles.installFile(options.get(CodeParser.DUMPER_FOLDER).toPath(), + new File(resourceFolder, asset.filename()), ClangAstWebResource.getAssetResource(asset), asset.sha256(), "clang-dumper asset '" + asset.filename() + "'"); @@ -323,8 +324,8 @@ static File resolveIncludes(File cacheFolder, ClangDumperManifestAsset includesA } var includesRoot = extractedFolder.getParentFile().toPath(); - CacheFiles.deleteUnlockedStagingLocks(includesRoot); - var stagingFolder = CacheFiles.createStagingDirectory(includesRoot, + CacheFiles.deleteUnlockedStagingLocks(cacheFolder.toPath(), includesRoot); + var stagingFolder = CacheFiles.createStagingDirectory(cacheFolder.toPath(), includesRoot, "." + includesAsset.sha256() + ".tmp-"); try { var downloadFolder = CacheFiles.createTemporaryDirectory(stagingFolder.path(), ".download-"); @@ -458,8 +459,8 @@ private void deleteStaleVersions(Instant now, File currentVersionFolder, File cu currentVersionFolder.toPath()); CacheFiles.deleteStaleDirectories(cacheRoot, getIncludesRoot().toPath(), cutoff, currentIncludesFolder == null ? null : currentIncludesFolder.toPath()); - CacheFiles.deleteUnlockedStagingLocks(getReleasesFolder().toPath()); - CacheFiles.deleteUnlockedStagingLocks(getIncludesRoot().toPath()); + CacheFiles.deleteUnlockedStagingLocks(cacheRoot, getReleasesFolder().toPath()); + CacheFiles.deleteUnlockedStagingLocks(cacheRoot, getIncludesRoot().toPath()); } catch (RuntimeException e) { SpecsLogs.warn("Could not clean stale clang-dumper cache resources", e); } diff --git a/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java b/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java index c1a51e62c..4ba36b52e 100644 --- a/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java +++ b/ClangAstParser/test/pt/up/fe/specs/clang/ClangResourcesTest.java @@ -210,7 +210,8 @@ public void corruptNewDownloadIsRejectedWithoutRetry() throws IOException { var destination = tempFolder.resolve("release/tool").toFile(); assertThrows(RuntimeException.class, - () -> CacheFiles.installFile(destination, copyingResource(source, writes), HELLO_SHA256, "test asset")); + () -> CacheFiles.installFile(tempFolder, destination, copyingResource(source, writes), HELLO_SHA256, + "test asset")); assertEquals(1, writes.get()); assertFalse(destination.exists()); @@ -228,7 +229,8 @@ public void existingReleaseResourceIsReused() throws IOException { var source = Files.writeString(tempFolder.resolve("source"), "new"); assertEquals(destination, - CacheFiles.installFile(destination, copyingResource(source, writes), HELLO_SHA256, "test asset")); + CacheFiles.installFile(tempFolder, destination, copyingResource(source, writes), HELLO_SHA256, + "test asset")); assertEquals(0, writes.get()); assertEquals("cached", Files.readString(destination.toPath())); } @@ -271,9 +273,9 @@ public void concurrentInitializationLeavesOneValidIncludesTree() throws Exceptio @Test public void activelyLockedStagingDirectoriesArePreserved() throws Exception { var includesRoot = Files.createDirectories(tempFolder.resolve("includes")); - var staging = CacheFiles.createStagingDirectory(includesRoot, ".sha.tmp-"); + var staging = CacheFiles.createStagingDirectory(tempFolder, includesRoot, ".sha.tmp-"); try { - CacheFiles.deleteUnlockedStagingLocks(includesRoot); + CacheFiles.deleteUnlockedStagingLocks(tempFolder, includesRoot); assertTrue(Files.exists(staging.path())); assertTrue(Files.exists(staging.lockPath())); } finally { @@ -285,14 +287,14 @@ public void activelyLockedStagingDirectoriesArePreserved() throws Exception { @Test public void unlockedStagingDirectoriesAreCleaned() throws Exception { var includesRoot = Files.createDirectories(tempFolder.resolve("includes")); - var staging = CacheFiles.createStagingDirectory(includesRoot, ".sha.tmp-"); + var staging = CacheFiles.createStagingDirectory(tempFolder, includesRoot, ".sha.tmp-"); var stagingPath = staging.path(); var lockPath = staging.lockPath(); staging.close(); Files.createFile(lockPath); assertTrue(Files.exists(lockPath)); - CacheFiles.deleteUnlockedStagingLocks(includesRoot); + CacheFiles.deleteUnlockedStagingLocks(tempFolder, includesRoot); assertFalse(Files.exists(stagingPath)); assertFalse(Files.exists(lockPath)); @@ -304,7 +306,7 @@ public void orphanedStagingLocksAreCleaned() throws IOException { var lockPath = includesRoot.resolve(".orphan.tmp-123.lock"); Files.createFile(lockPath); - CacheFiles.deleteUnlockedStagingLocks(includesRoot); + CacheFiles.deleteUnlockedStagingLocks(tempFolder, includesRoot); assertFalse(Files.exists(lockPath)); } From 2dee23b2adc478814319af087753cea526a01f5d Mon Sep 17 00:00:00 2001 From: "L. Sousa" Date: Thu, 6 Aug 2026 00:22:52 +0100 Subject: [PATCH 7/7] fix: clean current release staging locks --- ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java b/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java index 4de8340c0..228eba6b7 100644 --- a/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java +++ b/ClangAstParser/src/pt/up/fe/specs/clang/ClangResources.java @@ -459,7 +459,7 @@ private void deleteStaleVersions(Instant now, File currentVersionFolder, File cu currentVersionFolder.toPath()); CacheFiles.deleteStaleDirectories(cacheRoot, getIncludesRoot().toPath(), cutoff, currentIncludesFolder == null ? null : currentIncludesFolder.toPath()); - CacheFiles.deleteUnlockedStagingLocks(cacheRoot, getReleasesFolder().toPath()); + CacheFiles.deleteUnlockedStagingLocks(cacheRoot, currentVersionFolder.toPath()); CacheFiles.deleteUnlockedStagingLocks(cacheRoot, getIncludesRoot().toPath()); } catch (RuntimeException e) { SpecsLogs.warn("Could not clean stale clang-dumper cache resources", e);