From d2e7fcfd0b71a53f22a7e8fbcf5c920c1c689a00 Mon Sep 17 00:00:00 2001 From: Christopher Vogt Date: Thu, 15 Sep 2016 12:18:00 +0100 Subject: 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. --- nailgun_launcher/NailgunLauncher.java | 15 ++++- nailgun_launcher/ProxySecurityManager.java | 102 +++++++++++++++++++++++++++++ nailgun_launcher/Stage0Lib.java | 11 ++-- nailgun_launcher/TrapSecurityManager.java | 24 ++++++- stage1/Stage1Lib.scala | 6 +- 5 files changed, 146 insertions(+), 12 deletions(-) create mode 100644 nailgun_launcher/ProxySecurityManager.java 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() ); - public final static SecurityManager defaultSecurityManager = System.getSecurityManager(); + public final static SecurityManager initialSecurityManager + = System.getSecurityManager(); + + public final static ThreadLocal trapExitCode = + new ThreadLocal() { + @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 sourceFiles, SecurityManager defaultSecurityManager + EarlyDependencies earlyDeps, List 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); } } diff --git a/stage1/Stage1Lib.scala b/stage1/Stage1Lib.scala index 9e500a3..43c4f84 100644 --- a/stage1/Stage1Lib.scala +++ b/stage1/Stage1Lib.scala @@ -251,14 +251,16 @@ ${files.sorted.mkString(" \\\n")} } def trapExitCode( code: => ExitCode ): ExitCode = { + val trapExitCodeBefore = NailgunLauncher.trapExitCode.get try{ - System.setSecurityManager( new TrapSecurityManager ) + NailgunLauncher.trapExitCode.set(true) code } catch { case CatchTrappedExitCode(exitCode) => + logger.stage1(s"caught exit code $exitCode") exitCode } finally { - System.setSecurityManager(NailgunLauncher.defaultSecurityManager) + NailgunLauncher.trapExitCode.set(trapExitCodeBefore) } } -- cgit v1.2.3