From 0725a336fcc773943a99dd5d8dd91c50e4f0ba71 Mon Sep 17 00:00:00 2001
From: Ilya Matveev <ilya.matveev@jetbrains.com>
Date: Fri, 24 Jan 2020 18:08:44 +0700
Subject: [PATCH] Fix data race in zip FileSystem reference counting

Issue #KT-36076 Fixed
---
 .../org/jetbrains/kotlin/konan/file/File.kt   |  5 +++
 .../jetbrains/kotlin/konan/file/ZipUtil.kt    | 40 +++++++++++--------
 2 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/compiler/util-io/src/org/jetbrains/kotlin/konan/file/File.kt b/compiler/util-io/src/org/jetbrains/kotlin/konan/file/File.kt
index 9305d282196..498dd839be8 100644
--- a/compiler/util-io/src/org/jetbrains/kotlin/konan/file/File.kt
+++ b/compiler/util-io/src/org/jetbrains/kotlin/konan/file/File.kt
@@ -28,6 +28,11 @@ data class File constructor(internal val javaPath: Path) {
         get() = javaPath.toAbsolutePath().toString()
     val absoluteFile: File
         get() = File(absolutePath)
+    val canonicalPath: String
+        get() = javaPath.toFile().canonicalPath
+    val canonicalFile: File
+        get() = File(canonicalPath)
+
     val name: String
         get() = javaPath.fileName.toString().removeSuffixIfPresent("/") // https://bugs.java.com/bugdatabase/view_bug.do?bug_id=8153248
     val extension: String
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 d6d37e8d0c0..65fea7bbdb6 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
@@ -10,25 +10,27 @@ import java.nio.file.*
 import java.util.concurrent.ConcurrentHashMap
 
 private val File.zipUri: URI
-    get() = URI.create("jar:${this.toPath().toUri()}")
+    get() = URI.create("jar:${canonicalFile.toPath().toUri()}")
 
-private val counters = ConcurrentHashMap<FileSystem, Int>()
+private data class FileSystemRefCounter(val fileSystem: FileSystem, val counter: Int)
 
-// Zip filesystem provider creates a singlton zip FileSystem.
+private val fileSystems = ConcurrentHashMap<URI, FileSystemRefCounter>()
+
+// 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 try {
-        FileSystems.newFileSystem(zipUri, attributes, null).also {
-            counters[it] = 1
+
+    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)
         }
-    } catch (e: FileSystemAlreadyExistsException) {
-        FileSystems.getFileSystem(zipUri).also {
-            counters.computeIfPresent(it, { _, u -> u + 1})
-        }
-    }
+    }!!.fileSystem
 }
 
 fun FileSystem.file(file: File) = File(this.getPath(file.path))
@@ -56,12 +58,16 @@ fun <T> File.withZipFileSystem(mutable: Boolean = false, action: (FileSystem) ->
     return try {
         action(zipFileSystem)
     } finally {
-        val counter = counters[zipFileSystem]!!
-        if (counter == 1) {
-            zipFileSystem.close()
-            counters.remove(zipFileSystem)
-        } else {
-            counters.computeIfPresent(zipFileSystem, { _, u -> u - 1})
+        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)
+            }
         }
     }
 }