Fix occasional failed imports due to race conditions

jvm/src/main/scala/io/kaitai/struct/formats/JavaClassSpecs.scala

@@ -8,15 +8,24 @@ import java.io.{File, FileNotFoundException}
 import scala.collection.mutable
 import scala.concurrent.ExecutionContext.Implicits.global
 import scala.concurrent.Future
+import scala.collection.concurrent
+import java.util.concurrent.ConcurrentHashMap
+import scala.jdk.CollectionConverters._
 
 /**
   * Java implementation of ClassSpec container, doing imports from local files.
   */
 class JavaClassSpecs(relPath: String, absPaths: Seq[String], firstSpec: ClassSpec)
   extends ClassSpecs(firstSpec) {
 
-  private val relFiles = mutable.Map[String, ClassSpec]()
-  private val absFiles = mutable.Map[String, ClassSpec]()
+  // We're using thread-safe `ConcurrentHashMap` for `relFiles` and `absFiles`,
+  // because these hash maps may be mutated concurrently by multiple threads in
+  // `JavaClassSpecs.cached()`. Using a non-thread-safe hash map here could
+  // occasionally cause `cacheMap.get(name)` in `JavaClassSpecs.cached()` to
+  // fail internally and throw an `ArrayIndexOutOfBoundsException`, see
+  // https://github.com/kaitai-io/kaitai_struct/issues/951
+  private val relFiles: concurrent.Map[String, ClassSpec] = new ConcurrentHashMap[String, ClassSpec]().asScala
+  private val absFiles: concurrent.Map[String, ClassSpec] = new ConcurrentHashMap[String, ClassSpec]().asScala
 
   override def importRelative(name: String, path: List[String], inFile: Option[String]): Future[Option[ClassSpec]] = Future {
     Log.importOps.info(() => s".. importing relative $name")

shared/src/main/scala/io/kaitai/struct/precompile/LoadImports.scala

@@ -76,8 +76,23 @@ class LoadImports(specs: ClassSpecs) {
           //
           // In theory, duplicate imports shouldn't be returned at all by
           // import* methods due to caching, but we won't rely on it here.
-          if (!specs.contains(specName)) {
-            specs(specName) = spec
+          //
+          // The `synchronized` block is necessary because this code is run
+          // concurrently by multiple threads (each resolving different imports)
+          // and `specs` is a shared non-thread-safe `HashMap`. Without this
+          // synchronization, a few imports were occasionally missing from
+          // `specs` due to a race condition, and even (though rarely) the
+          // implementation of `specs.contains()` could fail internally with an
+          // `ArrayIndexOutOfBoundsException`. For more details, see
+          // https://github.com/kaitai-io/kaitai_struct/issues/951
+          val isNewSpec = specs.synchronized {
+            val isNew = !specs.contains(specName)
+            if (isNew) {
+              specs(specName) = spec
+            }
+            isNew
+          }
+          if (isNewSpec) {
             processClass(spec, ImportPath.updateWorkDir(workDir, impPath))
           } else {
             Log.importOps.warn(() => s"... we have that already, ignoring")

shared/src/main/scala/io/kaitai/struct/precompile/ResolveTypes.scala

@@ -80,6 +80,7 @@ class ResolveTypes(specs: ClassSpecs, topClass: ClassSpec, opaqueTypes: Boolean)
           (Some(ClassSpec.opaquePlaceholder(typeName)), None)
         } else {
           // Opaque types are disabled => that is an error
+          Log.typeResolve.info(() => "    => ??? (opaque type are disabled => error)")
           (None, Some(TypeNotFoundErr(typeName, curClass, path)))
         }
       case Some(x) =>