Refactored ClassLoaderCache to use key locked cache to pave the way for caching classloaders hierarchically without deadlocks

15 files changed, 84 insertions, 73 deletions

diff --git a/nailgun_launcher/NailgunLauncher.java b/nailgun_launcher/NailgunLauncher.java
index 11a8680..2278764 100644
--- a/nailgun_launcher/NailgunLauncher.java
+++ b/nailgun_launcher/NailgunLauncher.java
@@ -21,8 +21,8 @@ public class NailgunLauncher{
    * Persistent cache for caching classloaders for the JVM life time. Can be used as needed by user
    * code to improve startup time.
    */
-  public static ConcurrentHashMap<String,ClassLoader> classLoaderCache =
-    new ConcurrentHashMap<String,ClassLoader>();
+  public static ConcurrentHashMap classLoaderCache =
+    new ConcurrentHashMap();
 
   public static SecurityManager defaultSecurityManager = System.getSecurityManager();
 
diff --git a/stage1/ClassLoaderCache.scala b/stage1/ClassLoaderCache.scala
index 18a0d0e..35008f0 100644
--- a/stage1/ClassLoaderCache.scala
+++ b/stage1/ClassLoaderCache.scala
@@ -1,25 +1,60 @@
 package cbt
 
 import java.net._
+import java.util.concurrent.ConcurrentHashMap
 
-private[cbt] object ClassLoaderCache{
-  private val cache = NailgunLauncher.classLoaderCache
-  def get( classpath: ClassPath )(implicit logger: Logger): ClassLoader
-    = cache.synchronized{
-    val lib = new Stage1Lib(logger)
-    val key = classpath.strings.sorted.mkString(":")
-    if( cache.containsKey(key) ){
-      logger.resolver("CACHE HIT: "++key)
-      cache.get(key)
-    } else {
-      logger.resolver("CACHE MISS: "++key)
-      val cl = new cbt.URLClassLoader( classpath, ClassLoader.getSystemClassLoader )
-      cache.put( key, cl )
-      cl
+class ClassLoaderCache(logger: Logger){
+  val permanent = new KeyLockedLazyCache(
+    NailgunLauncher.classLoaderCache.asInstanceOf[ConcurrentHashMap[String,AnyRef]],
+    NailgunLauncher.classLoaderCache.asInstanceOf[ConcurrentHashMap[AnyRef,ClassLoader]],
+    logger
+  )
+  val transient = new KeyLockedLazyCache(
+    new ConcurrentHashMap[String,AnyRef],
+    new ConcurrentHashMap[AnyRef,ClassLoader],
+    logger
+  )
+}
+
+private[cbt] class LockableKey
+/**
+A cache that lazily computes values if needed during lookup.
+Locking occurs on the key, so separate keys can be looked up
+simultaneously without a deadlock.
+*/
+final private[cbt] class KeyLockedLazyCache[Key <: AnyRef,Value <: AnyRef](
+  keys: ConcurrentHashMap[Key,AnyRef],
+  builds: ConcurrentHashMap[AnyRef,Value],
+  logger: Logger
+){
+  def get( key: Key, value: => Value ): Value = {
+    val keyObject = keys.synchronized{    
+      if( ! (keys containsKey key) ){
+        logger.resolver("CACHE MISS: " ++ key.toString)
+        keys.put( key, new LockableKey )
+      } else {
+        logger.resolver("CACHE HIT: " ++ key.toString)
+      }
+      keys get key
+    }
+    import collection.JavaConversions._
+    logger.resolver("CACHE: \n" ++ keys.mkString("\n"))
+    def k = ClassPath(new java.io.File("c")).asInstanceOf[Key]
+    // synchronizing on key only, so asking for a particular key does
+    // not block the whole cache, but just that cache entry
+    key.synchronized{
+      if( ! (builds containsKey keyObject) ){
+        builds.put( keyObject, value )
+      }
+      builds get keyObject      
     }
   }
-  def remove( classpath: ClassPath ) = {
-    val key = classpath.strings.sorted.mkString(":")
-    cache.remove( key )
+  def remove( key: Key ) = keys.synchronized{    
+    if( (keys containsKey key) ){
+      keys.put( key, new LockableKey )
+    }
+    val keyObject = keys get key
+    keys.remove( key )
+    builds.remove( keyObject )
   }
 }
diff --git a/stage1/ClassPath.scala b/stage1/ClassPath.scala
index 66a1b44..96963e4 100644
--- a/stage1/ClassPath.scala
+++ b/stage1/ClassPath.scala
@@ -25,6 +25,6 @@ case class ClassPath(files: Seq[File]){
   def string = strings.mkString( File.pathSeparator )
   def strings = files.map{
     f => f.string ++ ( if(f.isDirectory) "/" else "" )
-  }
+  }.sorted
   def toConsole = string
 }
diff --git a/stage1/KeyLockedLazyCache.scala b/stage1/KeyLockedLazyCache.scala
deleted file mode 100644
index c8b37ea..0000000
--- a/stage1/KeyLockedLazyCache.scala
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
-package cbt
-import java.util.concurrent.ConcurrentHashMap
-import scala.concurrent.Future
-
-/**
-A cache that lazily computes values if needed during lookup.
-Locking occurs on the key, so separate keys can be looked up
-simultaneously without a deadlock.
-*/
-final private[cbt] class KeyLockedLazyCache[Key <: AnyRef,Value]{
-  private val keys = new ConcurrentHashMap[Key,LockableKey]()
-  private val builds = new ConcurrentHashMap[LockableKey,Value]()
-
-  private class LockableKey
-  def get( key: Key, value: => Value ): Value = {
-    val keyObject = keys.synchronized{    
-      if( ! (keys containsKey key) ){
-        keys.put( key, new LockableKey )
-      }
-      keys get key
-    }
-    // synchronizing on key only, so asking for a particular key does
-    // not block the whole cache, but just that cache entry
-    key.synchronized{
-      if( ! (builds containsKey keyObject) ){
-        builds.put( keyObject, value )
-      }
-      builds get keyObject      
-    }
-  }
-}
-*/
diff --git a/stage1/Stage1.scala b/stage1/Stage1.scala
index a593c03..b1017f7 100644
--- a/stage1/Stage1.scala
+++ b/stage1/Stage1.scala
@@ -50,18 +50,21 @@ object Stage1{
 
     val src = stage2.listFiles.toVector.filter(_.isFile).filter(_.toString.endsWith(".scala"))
     val changeIndicator = stage2Target ++ "/cbt/Build.class"
+    
+    val classLoaderCache = new ClassLoaderCache(logger)
 
     logger.stage1("before conditionally running zinc to recompile CBT")
     if( src.exists(newerThan(_, changeIndicator)) ) {
       val stage1Classpath = CbtDependency()(logger).dependencyClasspath
       logger.stage1("cbt.lib has changed. Recompiling with cp: " ++ stage1Classpath.string)
-      zinc( true, src, stage2Target, stage1Classpath, Seq("-deprecation") )( zincVersion = "0.3.9", scalaVersion = constants.scalaVersion )
+      zinc( true, src, stage2Target, stage1Classpath, classLoaderCache, Seq("-deprecation") )( zincVersion = "0.3.9", scalaVersion = constants.scalaVersion )
     }
     logger.stage1(s"[$now] calling CbtDependency.classLoader")
 
+
     logger.stage1(s"[$now] Run Stage2")
     val ExitCode(exitCode) = /*trapExitCode*/{ // this 
-      runMain( mainClass, cwd +: args.drop(1).toVector, CbtDependency()(logger).classLoader )
+      runMain( mainClass, cwd +: args.drop(1).toVector, CbtDependency()(logger).classLoader(classLoaderCache) )
     }
     logger.stage1(s"[$now] Stage1 end")
     System.exit(exitCode)
diff --git a/stage1/Stage1Lib.scala b/stage1/Stage1Lib.scala
index d24ba52..6a8a0ba 100644
--- a/stage1/Stage1Lib.scala
+++ b/stage1/Stage1Lib.scala
@@ -30,7 +30,7 @@ object TrappedExitCode{
     }
 }
 
-case class Context( cwd: File, args: Seq[String], logger: Logger )
+case class Context( cwd: File, args: Seq[String], logger: Logger, classLoaderCache: ClassLoaderCache )
 
 class BaseLib{
   def realpath(name: File) = new File(Paths.get(name.getAbsolutePath).normalize.toString)
@@ -130,6 +130,7 @@ class Stage1Lib( val logger: Logger ) extends BaseLib{
     files: Seq[File],
     compileTarget: File,
     classpath: ClassPath,
+    classLoaderCache: ClassLoaderCache,
     extraArgs: Seq[String] = Seq()
   )( zincVersion: String, scalaVersion: String ): Unit = {
 
@@ -176,7 +177,7 @@ class Stage1Lib( val logger: Logger ) extends BaseLib{
             "-cp", cp,
             "-d", compileTarget.toString
           ) ++ extraArgs.map("-S"++_) ++ files.map(_.toString),
-          zinc.classLoader
+          zinc.classLoader(classLoaderCache)
         )
       }
 
diff --git a/stage1/resolver.scala b/stage1/resolver.scala
index 98955cb..8dde321 100644
--- a/stage1/resolver.scala
+++ b/stage1/resolver.scala
@@ -87,7 +87,7 @@ abstract class Dependency{
   }
 
   private object classLoaderCache extends Cache[URLClassLoader]
-  def classLoader: URLClassLoader = classLoaderCache{
+  def classLoader( classLoaderCache: ClassLoaderCache  ): URLClassLoader = {
     if( concurrencyEnabled ){
       // trigger concurrent building / downloading dependencies
       exportClasspathConcurrently
@@ -112,7 +112,10 @@ abstract class Dependency{
     if(cacheDependencyClassLoader){    
       new URLClassLoader(
         buildClassPath,
-        ClassLoaderCache.get( cachedClassPath )
+        classLoaderCache.permanent.get(
+          cachedClassPath.string,
+          cbt.URLClassLoader( classpath, ClassLoader.getSystemClassLoader )
+        )
       )
     } else {
       new URLClassLoader(
diff --git a/stage2/AdminTasks.scala b/stage2/AdminTasks.scala
index e7fc78b..da4df9f 100644
--- a/stage2/AdminTasks.scala
+++ b/stage2/AdminTasks.scala
@@ -21,14 +21,14 @@ class AdminTasks(lib: Lib, args: Array[String], cwd: File){
     )
     // FIXME: this does not work quite yet, throws NoSuchFileException: /ammonite/repl/frontend/ReplBridge$.class
     lib.runMain(
-      "ammonite.repl.Main", Seq(), d.classLoader
+      "ammonite.repl.Main", Seq(), d.classLoader(new ClassLoaderCache(logger))
     )
   }
   def scala = {
     val version = args.lift(1).getOrElse(constants.scalaVersion)
     val scalac = new ScalaCompilerDependency( version )
     lib.runMain(
-      "scala.tools.nsc.MainGenericRunner", Seq("-cp", scalac.classpath.string), scalac.classLoader
+      "scala.tools.nsc.MainGenericRunner", Seq("-cp", scalac.classpath.string), scalac.classLoader(new ClassLoaderCache(logger))
     )
   }
   def scaffoldBasicBuild: Unit = lib.scaffoldBasicBuild( cwd )
diff --git a/stage2/BasicBuild.scala b/stage2/BasicBuild.scala
index 2f90197..a906b06 100644
--- a/stage2/BasicBuild.scala
+++ b/stage2/BasicBuild.scala
@@ -19,6 +19,7 @@ class BasicBuild( context: Context ) extends Build( context )
 class Build(val context: Context) extends Dependency with TriggerLoop{
   // library available to builds
   implicit final val logger: Logger = context.logger
+  implicit final val classLoaderCache: ClassLoaderCache = context.classLoaderCache
   override final protected val lib: Lib = new Lib(logger)
 
   // ========== general stuff ==========
@@ -147,12 +148,12 @@ class Build(val context: Context) extends Dependency with TriggerLoop{
     lib.compile(
       updated,
       sourceFiles, compileTarget, dependencyClasspath, scalacOptions,
-      zincVersion = zincVersion, scalaVersion = scalaVersion
+      zincVersion = zincVersion, scalaVersion = scalaVersion, context.classLoaderCache
     )
   }
 
   def runClass: String = "Main"
-  def run: ExitCode = lib.runMainIfFound( runClass, context.args, classLoader )
+  def run: ExitCode = lib.runMainIfFound( runClass, context.args, classLoader(context.classLoaderCache) )
 
   def test: ExitCode = lib.test(context)
 
diff --git a/stage2/GitDependency.scala b/stage2/GitDependency.scala
index c3e38b6..993825e 100644
--- a/stage2/GitDependency.scala
+++ b/stage2/GitDependency.scala
@@ -7,7 +7,7 @@ import org.eclipse.jgit.lib.Ref
 
 case class GitDependency(
   url: String, ref: String // example: git://github.com/cvogt/cbt.git#<some-hash>
-)(implicit val logger: Logger) extends Dependency{
+)(implicit val logger: Logger, classLoaderCache: ClassLoaderCache ) extends Dependency{
   override def lib = new Lib(logger)
 
   // TODO: add support for authentication via ssh and/or https
@@ -37,7 +37,7 @@ case class GitDependency(
 
     }
     val managedBuild = lib.loadDynamic(
-      Context( cwd = checkoutDirectory, args = Seq(), logger )
+      Context( cwd = checkoutDirectory, args = Seq(), logger, classLoaderCache )
     )    
     Seq( managedBuild )
   }
diff --git a/stage2/Lib.scala b/stage2/Lib.scala
index e3e1ee1..218208d 100644
--- a/stage2/Lib.scala
+++ b/stage2/Lib.scala
@@ -58,11 +58,11 @@ final class Lib(logger: Logger) extends Stage1Lib(logger) with Scaffold{
   def compile(
     updated: Boolean,
     sourceFiles: Seq[File], compileTarget: File, dependenyClasspath: ClassPath,
-    compileArgs: Seq[String], zincVersion: String, scalaVersion: String
+    compileArgs: Seq[String], zincVersion: String, scalaVersion: String, classLoaderCache: ClassLoaderCache
   ): File = {
     if(sourceFiles.nonEmpty)
       lib.zinc(
-        updated, sourceFiles, compileTarget, dependenyClasspath, compileArgs
+        updated, sourceFiles, compileTarget, dependenyClasspath, classLoaderCache, compileArgs
       )( zincVersion = zincVersion, scalaVersion = scalaVersion )
     compileTarget
   }
@@ -87,7 +87,8 @@ final class Lib(logger: Logger) extends Stage1Lib(logger) with Scaffold{
     jarTarget: File,
     artifactId: String,
     version: String,
-    compileArgs: Seq[String]
+    compileArgs: Seq[String],
+    classLoaderCache: ClassLoaderCache
   ): File = {
     mkdir(Path(apiTarget))
     if(sourceFiles.nonEmpty){
@@ -101,7 +102,7 @@ final class Lib(logger: Logger) extends Stage1Lib(logger) with Scaffold{
           runMain(
             "scala.tools.nsc.ScalaDoc",
             args,
-            ScalaDependencies(scalaVersion)(logger).classLoader
+            ScalaDependencies(scalaVersion)(logger).classLoader(classLoaderCache)
           )
         }
       }
diff --git a/stage2/PackageBuild.scala b/stage2/PackageBuild.scala
index 2866b7c..8f6d185 100644
--- a/stage2/PackageBuild.scala
+++ b/stage2/PackageBuild.scala
@@ -18,7 +18,7 @@ abstract class PackageBuild(context: Context) extends BasicBuild(context) with A
 
   private object cacheDocBasicBuild extends Cache[File]
   def docJar: File = cacheDocBasicBuild{
-    lib.docJar( scalaVersion, sourceFiles, dependencyClasspath, apiTarget, jarTarget, artifactId, version, scalacOptions )
+    lib.docJar( scalaVersion, sourceFiles, dependencyClasspath, apiTarget, jarTarget, artifactId, version, scalacOptions, context.classLoaderCache )
   }
 
   override def jars = jar +: dependencyJars
diff --git a/stage2/Stage2.scala b/stage2/Stage2.scala
index 4145e55..a0a2b57 100644
--- a/stage2/Stage2.scala
+++ b/stage2/Stage2.scala
@@ -27,7 +27,7 @@ object Stage2{
     }
     val task = argsV.lift( taskIndex )
 
-    val context = Context( new File(argsV(0)), argsV.drop( taskIndex + 1 ), logger )
+    val context = Context( new File(argsV(0)), argsV.drop( taskIndex + 1 ), logger, new ClassLoaderCache(logger) )
     val first = lib.loadRoot( context )
     val build = first.finalBuild
 
diff --git a/stage2/mixins.scala b/stage2/mixins.scala
index 2b38cdf..753ee60 100644
--- a/stage2/mixins.scala
+++ b/stage2/mixins.scala
@@ -29,7 +29,7 @@ trait ScalaTest extends Build with Test{
     lib.runMain(
       "org.scalatest.tools.Runner",
       Seq("-R", discoveryPath, "-oF") ++ context.args.drop(1),
-      classLoader
+      classLoader(context.classLoaderCache)
     )
   }
 }
diff --git a/test/test.scala b/test/test.scala
index 47bd28b..ebcaaa1 100644
--- a/test/test.scala
+++ b/test/test.scala
@@ -71,7 +71,7 @@ object Main{
     compile("simple")
     
     {
-      val noContext = Context(cbtHome ++ "/test/nothing", Seq(), logger)
+      val noContext = Context(cbtHome ++ "/test/nothing", Seq(), logger, new ClassLoaderCache(logger))
       val b = new Build(noContext){
         override def dependencies = Seq(
           JavaDependency("net.incongru.watchservice","barbary-watchservice","1.0"),