From 03bb9138ade38d7cb70ec12e4c397fc6339c1971 Mon Sep 17 00:00:00 2001 From: Ilya Matveev Date: Thu, 11 Jun 2020 19:51:26 +0700 Subject: [PATCH] [klib] Create ZipFileSystem from a Path instead of an URI Calling FileSystems.newFileSystem(URI, ...) throws a FileSystemAlreadyExistsException if a ZipFileSystem for this URI is already created. We still can use a single instance of ZipFileSystem by calling FileSystems.getFileSystem. In this case we use reference counting to determine when this instance can be safely closed. But we cannot count references if the same ZipFileSystem is used from different class loaders. This patch fixes this issue by creating a file system from Path instead of an URI. Contract of FileSystemProvider.newFileSystem(Path, ...) doesn't imply throwing FileSystemAlreadyExistsException. Issue #KT-37443 fixed --- .../jetbrains/kotlin/konan/file/ZipUtil.kt | 65 +++++++------------ .../kotlin/library/KotlinLibraryUtils.kt | 4 +- 2 files changed, 26 insertions(+), 43 deletions(-) diff --git a/compiler/util-io/src/org/jetbrains/kotlin/konan/file/ZipUtil.kt b/compiler/util-io/src/org/jetbrains/kotlin/konan/file/ZipUtil.kt index a94ce04f054..054e2dc70b3 100644 --- a/compiler/util-io/src/org/jetbrains/kotlin/konan/file/ZipUtil.kt +++ b/compiler/util-io/src/org/jetbrains/kotlin/konan/file/ZipUtil.kt @@ -7,30 +7,30 @@ package org.jetbrains.kotlin.konan.file import java.net.URI import java.nio.file.* -import java.util.concurrent.ConcurrentHashMap +import java.nio.file.spi.FileSystemProvider -private val File.zipUri: URI - get() = URI.create("jar:${canonicalFile.toPath().toUri()}") +// Zip filesystem provider doesn't allow creating several instances of ZipFileSystem from the same URI, +// so newFileSystem(URI, ...) throws a FileSystemAlreadyExistsException in this case. +// But FileSystemProvider.newFileSystem(File, ...) cannot throw this exception and allows creating several filesystems. +// See also: +// https://bugs.java.com/bugdatabase/view_bug.do?bug_id=7001822 +// https://bugs.java.com/bugdatabase/view_bug.do?bug_id=6994161 +fun File.zipFileSystem(create: Boolean = false): FileSystem { + val attributes = hashMapOf("create" to create.toString()) -private data class FileSystemRefCounter(val fileSystem: FileSystem, val counter: Int) - -private val fileSystems = ConcurrentHashMap() - -// Zip filesystem provider creates a singleton zip FileSystem. -// So newFileSystem can return an already existing one. -// And, more painful, closing the filesystem could close it for another consumer thread. -fun File.zipFileSystem(mutable: Boolean = false): FileSystem { - val zipUri = this.zipUri - val attributes = hashMapOf("create" to mutable.toString()) - - return fileSystems.compute(zipUri) { key, value -> - if (value == null) { - FileSystemRefCounter(FileSystems.newFileSystem(key, attributes, null), 1) - } else { - // TODO: If a file system already exists, we cannot change its mutability. - FileSystemRefCounter(value.fileSystem, value.counter + 1) + // There is no FileSystems.newFileSystem overload accepting the attribute map. + // So we have to manually iterate over the filesystem providers. + return FileSystemProvider.installedProviders().filter { it.scheme == "jar" }.mapNotNull { + try { + it.newFileSystem(this.toPath(), attributes) + } catch(e: Exception) { + when(e) { + is UnsupportedOperationException, + is IllegalArgumentException -> null + else -> throw e + } } - }!!.fileSystem + }.first() } fun FileSystem.file(file: File) = File(this.getPath(file.path)) @@ -40,7 +40,7 @@ fun FileSystem.file(path: String) = File(this.getPath(path)) private fun File.toPath() = Paths.get(this.path) fun File.zipDirAs(unixFile: File) { - unixFile.withMutableZipFileSystem { + unixFile.withZipFileSystem(create = true) { // Time attributes are not preserved to ensure that the output zip file bytes deterministic for a fixed set of // input files. this.recursiveCopyTo(it.file("/"), resetTimeAttributes = true) @@ -55,25 +55,8 @@ fun Path.unzipTo(directory: Path) { } } -fun File.withZipFileSystem(mutable: Boolean = false, action: (FileSystem) -> T): T { - val zipFileSystem = this.zipFileSystem(mutable) - return try { - action(zipFileSystem) - } finally { - fileSystems.compute(zipUri) { _, value -> - require(value != null) - if (value.counter == 1) { - value.fileSystem.close() - // Returning null removes this entry from the map - // See https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ConcurrentMap.html. - null - } else { - FileSystemRefCounter(value.fileSystem, value.counter - 1) - } - } - } +fun File.withZipFileSystem(create: Boolean, action: (FileSystem) -> T): T { + return this.zipFileSystem(create).use(action) } fun File.withZipFileSystem(action: (FileSystem) -> T): T = this.withZipFileSystem(false, action) - -fun File.withMutableZipFileSystem(action: (FileSystem) -> T): T = this.withZipFileSystem(true, action) \ No newline at end of file diff --git a/compiler/util-klib/src/org/jetbrains/kotlin/library/KotlinLibraryUtils.kt b/compiler/util-klib/src/org/jetbrains/kotlin/library/KotlinLibraryUtils.kt index 4c47394b9d4..65dbd839976 100644 --- a/compiler/util-klib/src/org/jetbrains/kotlin/library/KotlinLibraryUtils.kt +++ b/compiler/util-klib/src/org/jetbrains/kotlin/library/KotlinLibraryUtils.kt @@ -2,7 +2,7 @@ package org.jetbrains.kotlin.library import org.jetbrains.kotlin.konan.file.File import org.jetbrains.kotlin.konan.file.file -import org.jetbrains.kotlin.konan.file.withMutableZipFileSystem +import org.jetbrains.kotlin.konan.file.withZipFileSystem import org.jetbrains.kotlin.library.impl.zippedKotlinLibraryChecks const val KLIB_FILE_EXTENSION = "klib" @@ -23,7 +23,7 @@ fun File.unpackZippedKonanLibraryTo(newDir: File) { newDir.delete() } - this.withMutableZipFileSystem { + this.withZipFileSystem { it.file("/").recursiveCopyTo(newDir) } check(newDir.exists) { "Could not unpack $this as $newDir." }