Skip to content
This repository has been archived by the owner on Apr 19, 2023. It is now read-only.

Reduce overhead of the security manager #149

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Reduce overhead of the security manager #149

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3366a9e
Reduce overhead of the security manager
retronym Aug 28, 2018
995fc5e
Overrides of checkXXX delegate to the corresponding method in base
retronym Sep 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 137 additions & 1 deletion nailgun-server/src/main/java/com/martiansoftware/nailgun/NGSecurityManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

package com.martiansoftware.nailgun;

import java.io.FileDescriptor;
import java.net.InetAddress;
import java.security.Permission;

/**
Expand Down Expand Up @@ -58,10 +60,144 @@ public void checkPermission(Permission perm, Object context) {
}
}

/** Avoid constructing a FilePermission object in checkRead if base manager is null. */
// Overrides below avoid the cost of creating Permissions objects if base manager is null.
// FilePermission, in particular, is expensive to create.

public void checkRead(String file) {
if (base != null) {
super.checkRead(file);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be base.checkRead() instead of super.checkRead()? I have the same question for the new overloads added in this PR. It looks like @qma asked the same question on the original patch: #11 (comment)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On reflection, I suspect that this was done for backwards-compatibility reasons, since overriding and delegating to base would be a behavior change whereas this change is only a performance optimization.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super.checkRead will eventually get to base.checkPermission. But it would be potentially more efficient to call base.checkRead directly here. The semantics should be the same for any well behaved SecurityManager.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pushed a commit that delegates directly to the corresponding method.

}
}

public void checkCreateClassLoader() {
if (base != null) {
super.checkCreateClassLoader();
}
}

public void checkAccess(Thread t) {
if (base != null) {
super.checkAccess(t);
}
}

public void checkAccess(ThreadGroup g) {
if (base != null) {
super.checkAccess(g);
}
}

public void checkExec(String cmd) {
if (base != null) {
super.checkExec(cmd);
}
}

public void checkLink(String lib) {
if (base != null) {
super.checkLink(lib);
}
}

public void checkRead(FileDescriptor fd) {
if (base != null) {
super.checkRead(fd);
}
}

public void checkRead(String file, Object context) {
if (base != null) {
super.checkRead(file, context);
}
}

public void checkWrite(FileDescriptor fd) {
if (base != null) {
super.checkWrite(fd);
}
}

public void checkWrite(String file) {
if (base != null) {
super.checkWrite(file);
}
}

public void checkDelete(String file) {
if (base != null) {
super.checkDelete(file);
}
}

public void checkConnect(String host, int port) {
if (base != null) {
super.checkConnect(host, port);
}
}

public void checkConnect(String host, int port, Object context) {
if (base != null) {
super.checkConnect(host, port, context);
}
}

public void checkListen(int port) {
if (base != null) {
super.checkListen(port);
}
}

public void checkAccept(String host, int port) {
if (base != null) {
super.checkAccept(host, port);
}
}

public void checkMulticast(InetAddress maddr) {
if (base != null) {
super.checkMulticast(maddr);
}
}

public void checkPropertiesAccess() {
if (base != null) {
super.checkPropertiesAccess();
}
}

public void checkPropertyAccess(String key) {
if (base != null) {
super.checkPropertyAccess(key);
}
}

public void checkPrintJobAccess() {
if (base != null) {
super.checkPrintJobAccess();
}
}

public void checkPackageAccess(String pkg) {
if (base != null) {
super.checkPackageAccess(pkg);
}
}

public void checkPackageDefinition(String pkg) {
if (base != null) {
super.checkPackageDefinition(pkg);
}
}

public void checkSetFactory() {
if (base != null) {
super.checkSetFactory();
}
}

public void checkSecurityAccess(String target) {
if (base != null) {
super.checkSecurityAccess(target);
}
}
}