Overhead of SecurityManager

[justinas-dabravolskas]

Running javac(not limited to) inside nailgun process has very high contention caused by security checks, in our case mostly:

java.io.UnixFileSystem.canonicalize0(Native Method)
java.io.UnixFileSystem.canonicalize(UnixFileSystem.java:172)
java.io.File.getCanonicalPath(File.java:618)
java.io.FilePermission$1.run(FilePermission.java:215)
java.io.FilePermission$1.run(FilePermission.java:203)
java.security.AccessController.doPrivileged(Native Method)
java.io.FilePermission.init(FilePermission.java:203)
java.io.FilePermission.<init>(FilePermission.java:277)
sun.net.www.protocol.file.FileURLConnection.getPermission(FileURLConnection.java:225)
sun.misc.URLClassPath.check(URLClassPath.java:604)

We iterated over several solutions to this and settled on a custom java agent that ‘disables’ security manager. This keeps nailgun protocol intact, but in our case improves compilation speed within pantsbuild from 75 to 12 minutes.

package com.r9.nailgun;

import java.io.IOException;
import java.lang.instrument.ClassDefinition;
import java.lang.instrument.Instrumentation;
import java.lang.instrument.UnmodifiableClassException;
import java.net.JarURLConnection;

import javassist.ClassPool;
import javassist.CtClass;
import javassist.CtMethod;

/**
 * This class disables security manager without breaking nailgun protocol that depends on it.
 * Implementation notes:
 * Can't use transformer cause classes are already loaded.
 * Can't add fields and methods or mark fields public with redefinition.
 * SecurityManager is not accessible with reflection.
 * Tested with nailgun 0.9.1 and java 8, should be compatible with nailgun 0.9.3 (and java 9 ???)
 * Warning: This agent disables security manager. Use on your own risk.
 * 
 * @author Justinas Dabravolskas
 * @author Darius Prakaitis
 *
 */
public class ExitAgent {

    // This is a storage for securityManager that is used in Runtime.exit
    public static volatile java.lang.SecurityManager exitSecurity;

    public static void premain(String agentArgs, Instrumentation inst) {
        try {

            // make ExitAgent accessible to application, we probably load the second instance in
            // different class loader but who cares
            JarURLConnection connection = (JarURLConnection) ExitAgent.class.getClassLoader().getResource("com/r9/nailgun/ExitAgent.class").openConnection();
            inst.appendToBootstrapClassLoaderSearch(connection.getJarFile());

            inst.redefineClasses(new ClassDefinition(Class.forName("java.lang.System"), getSystemByteCode()));

            inst.redefineClasses(new ClassDefinition(Class.forName("java.lang.Runtime"), getRuntimeByteCode()));

        } catch (ClassNotFoundException e1) {
            e1.printStackTrace();
            throw new RuntimeException(e1);
        } catch (UnmodifiableClassException e1) {
            e1.printStackTrace();
            throw new RuntimeException(e1);
        } catch (IOException e) {
            e.printStackTrace();
            throw new RuntimeException(e);
        }
    }

    private static byte[] getSystemByteCode() {
        try {
            ClassPool cp = ClassPool.getDefault();
            CtClass cc = cp.get("java.lang.System");
            // don't access volatile field at all
            CtMethod getSecurityManager = cc.getDeclaredMethod("getSecurityManager", new CtClass[] {});
            getSecurityManager.setBody("{return null;}");

            CtMethod setSecurityManager = cc.getDeclaredMethod("setSecurityManager", new CtClass[] { cp.get("java.lang.SecurityManager") });
            // make security manager available to Runtime.exit only
            setSecurityManager.setBody("{com.r9.nailgun.ExitAgent.exitSecurity=$1;}");
            byte[] byteCode = cc.toBytecode();
            cc.detach();
            return byteCode;
        } catch (Exception ex) {
            ex.printStackTrace();
            throw new RuntimeException(ex);
        }
    }

    private static byte[] getRuntimeByteCode() {
        try {
            ClassPool cp = ClassPool.getDefault();
            CtClass cc = cp.get("java.lang.Runtime");
            CtMethod m = cc.getDeclaredMethod("exit", new CtClass[] { CtClass.intType });
            m.setBody("{SecurityManager security = com.r9.nailgun.ExitAgent.exitSecurity; if(security != null){security.checkExit($1);} Shutdown.exit($1);}");

            byte[] byteCode = cc.toBytecode();
            cc.detach();
            return byteCode;
        } catch (Exception ex) {
            ex.printStackTrace();
            throw new RuntimeException(ex);
        }
    }

}

[sbalabanov-zz]

That's interesting. Is it only Nailgun related and if such then why? Permission checks are done for all file operations under Oracle's HotSpot implementation of a Path/File provider, it has to do nothing with nailgun.

Also, if you use HotSpot then this approach probably violates Oracle's Java license as system code is essentially altered.

To come along with this problems on projects we use with Nailgun, we are considering following options:

• Implementing a custom FileSystem/FileSystemProvider
• Running on forked OpenJDK which has those checks off

[justinas-dabravolskas]

Security checks are triggered just in case securityManager!=null http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/8-b132/sun/misc/URLClassPath.java#URLClassPath.checkURL%28java.net.URL%29
Overhead is added even with NOOP security manager. Usually you don't run javac under security manager, but Nailgun sets one for exit trap, so this overhead is triggered specifically by Nailgun https://github.com/facebook/nailgun/blob/master/nailgun-server/src/main/java/com/martiansoftware/nailgun/NGServer.java

[sbalabanov-zz]

SecurityManager is the only way to intercept termination that I know of. Without altering bytecode, the only way I see is to pass a parameter to Nailgun whether or not to intercept termination. But, with interception switched off, you can't use System.exit in nail's code as it would do what supposed - terminate jvm instead of terminating a nail. Thoughts?

[justinas-dabravolskas]

The only option 'without side effects' I know is byte code manipulation. I've tried just disabling exit interception and it broke Nailgun protocol, I guess it can be done in more elegant way, but terminating jvm after nail's System.exit doesn't sound like an option.

[sbalabanov-zz]

What I mean is that instead of calling System.exit() the nail should call some other function explicitly (like NGSession.exit() which yet to be implemented) which will make sure the protocol is not broken and terminate a nail but not jvm. A linter should validate what no System.exit() code sneaks in for nail's codebase.
We can consider doing that for Buck.
Are you saying the win is mostly for in-proc javac? How about regular class loader?

[jvican]

the nail should call some other function explicitly (like NGSession.exit())

Are nails not supposed to run ngSession.exit() instead of System.exit() directly? If that's the case and the security manager is disabled completely, would it break the invariants of the Nailgun protocol?

[sbalabanov-zz]

ngSession.exit() does not really interrupt running nail and in fact does not work correctly; the problem is client terminates the socket and the server still listens to that in NGCommunicator which cause listening thread to throw and log an error. This needs to be processed gracefully.

@justinas-dabravolskas do you mind to share numbers about the optimization win, like how many classes were sent to javac to achieve above mentioned improvement?

[justinas-dabravolskas]

javac for ~50k files takes ~75 minutes without agent and 10-15minutes with one (Xmx set for minimal gc overhead, tested on macOS). Anyways overhead of security manager should be linear and not limited to javac. Offtopic suggestion for running javac 1.8 (should not be an issue with java 9) inside nailgun: use -XDuseUnsharedTable. In our case it saves ~2G of heap without noticeable time overhead.

[retronym]

An alternative to simply avoiding use of the security manager would change NGSecurityManager to override more methods from SecurityManager to route around the slow parts.

For instance, the default checkWrite implementation:

    public void checkWrite(String file) {
        checkPermission(new FilePermission(file,
            SecurityConstants.FILE_WRITE_ACTION));
    }

Calls new File(file).canonicalize during construction of FilePermission.

(I just came across this performance problem in the context of IntelliJ IDEA's use of Nailgun in its compile server for Scala)

---

I see now that #11 already implemented this idea for checkRead. I'll submit a PR to make this more comprehensive.