fix behavior of System.exit trapping

Installing one globally for the JVM live-time and make behavior dependent
on a thread local variable seems safer than globally switching it out
and having race conditions.

Also now all other calls are forwarded to a potential Nailgun
SecurityManager, which should fix some bugs.

4 files changed, 142 insertions, 10 deletions

diff --git a/nailgun_launcher/NailgunLauncher.java b/nailgun_launcher/NailgunLauncher.java
index 904a646..5a70312 100644
--- a/nailgun_launcher/NailgunLauncher.java
+++ b/nailgun_launcher/NailgunLauncher.java
@@ -20,7 +20,15 @@ public class NailgunLauncher{
     new ConcurrentHashMap<Object,ClassLoader>()
   );
 
-  public final static SecurityManager defaultSecurityManager = System.getSecurityManager();
+  public final static SecurityManager initialSecurityManager
+    = System.getSecurityManager();
+
+  public final static ThreadLocal<Boolean> trapExitCode = 
+    new ThreadLocal<Boolean>() {
+      @Override protected Boolean initialValue() {
+        return false;
+      }
+    };
 
   public static String TARGET = System.getenv("TARGET");
   private static String NAILGUN = "nailgun_launcher/";
@@ -54,6 +62,7 @@ public class NailgunLauncher{
       return;
     }
 
+    System.setSecurityManager( new TrapSecurityManager() );
     installProxySettings();
     String[] diff = args[0].split("\\.");
     long start = _start - (Long.parseLong(diff[0]) * 1000L) - Long.parseLong(diff[1]);
@@ -119,7 +128,7 @@ public class NailgunLauncher{
           compatibilitySourceFiles.add(f);
         }
       }
-      changed = compile(changed, start, "", compatibilityTarget, earlyDeps, compatibilitySourceFiles, defaultSecurityManager);
+      changed = compile(changed, start, "", compatibilityTarget, earlyDeps, compatibilitySourceFiles);
       
       if( classLoaderCache.contains( compatibilityTarget ) ){
         compatibilityClassLoader = classLoaderCache.get( compatibilityTarget );
@@ -145,7 +154,7 @@ public class NailgunLauncher{
         stage1SourceFiles.add(f);
       }
     }
-    changed = compile(changed, start, stage1Classpath, stage1Target, earlyDeps, stage1SourceFiles, defaultSecurityManager);
+    changed = compile(changed, start, stage1Classpath, stage1Target, earlyDeps, stage1SourceFiles);
 
     ClassLoader stage1classLoader;
     if( !changed && classLoaderCache.contains( stage1Classpath ) ){
diff --git a/nailgun_launcher/ProxySecurityManager.java b/nailgun_launcher/ProxySecurityManager.java
new file mode 100644
index 0000000..1a6e49c
--- /dev/null
+++ b/nailgun_launcher/ProxySecurityManager.java
@@ -0,0 +1,102 @@
+package cbt;
+
+import java.security.*;
+import java.io.FileDescriptor;
+import java.net.InetAddress;
+
+/*
+SecurityManager proxy that forwards all calls to the provided target if != null.
+Useful to replace a previously installed SecurityManager, overriding some methods
+but forwarding the rest.
+*/
+public class ProxySecurityManager extends SecurityManager{
+  private SecurityManager target;
+  public ProxySecurityManager(SecurityManager target){
+    this.target = target;
+  }
+  public Object getSecurityContext() {
+    if(target != null)
+      return target.getSecurityContext();
+    else return super.getSecurityContext();
+  }
+  public void checkPermission(Permission perm) {
+    if(target != null) target.checkPermission(perm);
+  }
+  public void checkPermission(Permission perm, Object context) {
+    if(target != null) target.checkPermission(perm, context);
+  }
+  public void checkCreateClassLoader() {
+    if(target != null) target.checkCreateClassLoader();
+  }
+  public void checkAccess(Thread t) {
+    if(target != null) target.checkAccess(t);
+  }
+  public void checkAccess(ThreadGroup g) {
+    if(target != null) target.checkAccess(g);
+  }
+  public void checkExit(int status) {
+    if(target != null) target.checkExit(status);
+  }
+  public void checkExec(String cmd) {
+    if(target != null) target.checkExec(cmd);
+  }
+  public void checkLink(String lib) {
+    if(target != null) target.checkLink(lib);
+  }
+  public void checkRead(FileDescriptor fd) {
+    if(target != null) target.checkRead(fd);
+  }
+  public void checkRead(String file) {
+    if(target != null) target.checkRead(file);
+  }
+  public void checkRead(String file, Object context) {
+    if(target != null) target.checkRead(file, context);
+  }
+  public void checkWrite(FileDescriptor fd) {
+    if(target != null) target.checkWrite(fd);
+  }
+  public void checkWrite(String file) {
+    if(target != null) target.checkWrite(file);
+  }
+  public void checkDelete(String file) {
+    if(target != null) target.checkDelete(file);
+  }
+  public void checkConnect(String host, int port) {
+    if(target != null) target.checkConnect(host, port);
+  }
+  public void checkConnect(String host, int port, Object context) {
+    if(target != null) target.checkConnect(host, port, context);
+  }
+  public void checkListen(int port) {
+    if(target != null) target.checkListen(port);
+  }
+  public void checkAccept(String host, int port) {
+    if(target != null) target.checkAccept(host, port);
+  }
+  public void checkMulticast(InetAddress maddr) {
+    if(target != null) target.checkMulticast(maddr);
+  }
+  public void checkPropertiesAccess() {
+    if(target != null) target.checkPropertiesAccess();
+  }
+  public void checkPropertyAccess(String key) {
+    if(target != null) target.checkPropertyAccess(key);
+  }
+  public void checkPrintJobAccess() {
+    if(target != null) target.checkPrintJobAccess();
+  }
+  public void checkPackageAccess(String pkg) {
+    if(target != null) target.checkPackageAccess(pkg);
+  }
+  public void checkPackageDefinition(String pkg) {
+    if(target != null) target.checkPackageDefinition(pkg);
+  }
+  public void checkSetFactory() {
+    if(target != null) target.checkSetFactory();
+  }
+  public ThreadGroup getThreadGroup() {
+    if(target != null)
+      return target.getThreadGroup();
+    else return super.getThreadGroup();
+  }
+}
diff --git a/nailgun_launcher/Stage0Lib.java b/nailgun_launcher/Stage0Lib.java
index ebf9d09..d6df1c6 100644
--- a/nailgun_launcher/Stage0Lib.java
+++ b/nailgun_launcher/Stage0Lib.java
@@ -19,9 +19,10 @@ public class Stage0Lib{
     }
   }
 
-  public static int runMain(String cls, String[] args, ClassLoader cl, SecurityManager defaultSecurityManager) throws Throwable{
+  public static int runMain(String cls, String[] args, ClassLoader cl) throws Throwable{
+    Boolean trapExitCodeBefore = NailgunLauncher.trapExitCode.get();
     try{
-      System.setSecurityManager( new TrapSecurityManager() );
+      NailgunLauncher.trapExitCode.set(true);
       cl.loadClass(cls)
         .getMethod("main", String[].class)
         .invoke( null, (Object) args);
@@ -33,7 +34,7 @@ public class Stage0Lib{
       }
       throw exception;
     } finally {
-      System.setSecurityManager(defaultSecurityManager);
+      NailgunLauncher.trapExitCode.set(trapExitCodeBefore);
     }
   }
 
@@ -54,7 +55,7 @@ public class Stage0Lib{
 
   public static Boolean compile(
     Boolean changed, Long start, String classpath, String target,
-    EarlyDependencies earlyDeps, List<File> sourceFiles, SecurityManager defaultSecurityManager
+    EarlyDependencies earlyDeps, List<File> sourceFiles
   ) throws Throwable{
     File statusFile = new File( new File(target) + ".last-success" );
     Long lastSuccessfullCompile = statusFile.lastModified();
@@ -89,7 +90,7 @@ public class Stage0Lib{
       PrintStream oldOut = System.out;
       try{
         System.setOut(System.err);
-        int exitCode = runMain( "com.typesafe.zinc.Main", zincArgs.toArray(new String[zincArgs.size()]), earlyDeps.zinc, defaultSecurityManager );
+        int exitCode = runMain( "com.typesafe.zinc.Main", zincArgs.toArray(new String[zincArgs.size()]), earlyDeps.zinc );
         if( exitCode == 0 ){
           write( statusFile, "" );
           Files.setLastModifiedTime( statusFile.toPath(), FileTime.fromMillis(start) );
diff --git a/nailgun_launcher/TrapSecurityManager.java b/nailgun_launcher/TrapSecurityManager.java
index ed00582..fada878 100644
--- a/nailgun_launcher/TrapSecurityManager.java
+++ b/nailgun_launcher/TrapSecurityManager.java
@@ -1,19 +1,39 @@
 package cbt;
 import java.security.*;
-public class TrapSecurityManager extends SecurityManager{
+/*
+When enabled, this SecurityManager turns System.exit(...) calls into exceptions that can be caught and handled.
+Installing a SecurityManager is a global side-effect and thus needs extra care in a persistent
+background process like CBT's. The current approach is install it once during JVM-startup.
+When disabled this delegates to the SecurityManager installed before if any, which
+would be Nailgun's if running on Nailgun. If we do not delegate to Nailgun, it seems we
+could in some cases kill the server process
+*/
+public class TrapSecurityManager extends ProxySecurityManager{
+  public TrapSecurityManager(){
+    super(NailgunLauncher.initialSecurityManager);
+  }
+  
   public void checkPermission( Permission permission ){
     /*
     NOTE: is it actually ok, to just make these empty?
     Calling .super leads to ClassNotFound exteption for a lambda.
     Calling to the previous SecurityManager leads to a stack overflow
     */
+    if(!NailgunLauncher.trapExitCode.get()){
+      super.checkPermission(permission);
+    }
   }
   public void checkPermission( Permission permission, Object context ){
     /* Does this methods need to be overidden? */
+    if(!NailgunLauncher.trapExitCode.get()){
+      super.checkPermission(permission, context);
+    }
   }
   @Override
   public void checkExit( int status ){
+    if(NailgunLauncher.trapExitCode.get()){
+      throw new TrappedExitCode(status);
+    }
     super.checkExit(status);
-    throw new TrappedExitCode(status);
   }
 }