[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

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
+// Zip filesystem provider doesn't allow creating several instances of ZipFileSystem from the same URI,
-    get() = URI.create("jar:${canonicalFile.toPath().toUri()}")
+// 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)
+    // There is no FileSystems.newFileSystem overload accepting the attribute map.
-
+    // So we have to manually iterate over the filesystem providers.
-private val fileSystems = ConcurrentHashMap<URI, FileSystemRefCounter>()
+    return FileSystemProvider.installedProviders().filter { it.scheme == "jar" }.mapNotNull {
-
+        try {
-// Zip filesystem provider creates a singleton zip FileSystem.
+            it.newFileSystem(this.toPath(), attributes)
-// So newFileSystem can return an already existing one.
+        } catch(e: Exception) {
-// And, more painful, closing the filesystem could close it for another consumer thread.
+            when(e) {
-fun File.zipFileSystem(mutable: Boolean = false): FileSystem {
+                is UnsupportedOperationException,
-    val zipUri = this.zipUri
+                is IllegalArgumentException -> null
-    val attributes = hashMapOf("create" to mutable.toString())
+                else -> throw e
-
+            }
-    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)
         }
-    }!!.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 <T> File.withZipFileSystem(mutable: Boolean = false, action: (FileSystem) -> T): T {
+fun <T> File.withZipFileSystem(create: Boolean, action: (FileSystem) -> T): T {
-    val zipFileSystem = this.zipFileSystem(mutable)
+    return this.zipFileSystem(create).use(action)
-    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 <T> File.withZipFileSystem(action: (FileSystem) -> T): T = this.withZipFileSystem(false, action)
-
-fun <T> File.withMutableZipFileSystem(action: (FileSystem) -> T): T = this.withZipFileSystem(true, action)

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." }