Skip to content

Commit

Permalink
[Netbeans-5734] Fix extraction of jvm-options from domain.xml and rev…
Browse files Browse the repository at this point in the history 
…ert add-opens directives

The additions of the "add-opens" arguments for the JVM are not
necessary if the jvm-options are correctly extracted from the
domain.xml.

It was observed, that glassfish cleanly starts with JDK 11, so something
strange was going on in the way NetBeans started it. The issue can be
tracked down into ServerTasks#appendOptions. In that method variable
substitution and proxy adjustments are done. The arguments are stored
in a combination of Map argumentName -> value and and a List for the
order of the arguments. The issue is the Map, as this limits each
argumentName to be present at most once, but there are multiple
add-opens directives. Only the last definition survived.
  • Loading branch information
@matthiasblaesing
matthiasblaesing committed Aug 2, 2021
1 parent 104207b commit 3668e87
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 73 deletions.
18 changes: 0 additions & 18 deletions enterprise/glassfish.common/src/org/netbeans/modules/glassfish/common/StartTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
import org.openide.execution.NbProcessDescriptor;
import org.openide.filesystems.FileObject;
import org.openide.filesystems.FileUtil;
import org.openide.modules.SpecificationVersion;
import org.openide.util.Lookup;
import org.openide.util.NbBundle;
import org.openide.util.RequestProcessor;
Expand Down Expand Up @@ -692,9 +691,6 @@ private StartupArgs createProcessDescriptor() throws ProcessCreationException {
// append other options from startup extenders, e.g. for profiling
appendStartupExtenderParams(optList);

// append options to open modules for java 9+
appendModuleOpensParams(optList);

return new StartupArgsEntity(
glassfishArgs,
optList,
Expand Down Expand Up @@ -761,20 +757,6 @@ private void appendStartupExtenderParams(List<String> optList) {
}
}

/**
* Appends options in order to open modules for java 9.0+
* @param optList
*/
private void appendModuleOpensParams(List<String> optList) {
SpecificationVersion currentInstanceJavaVersion = JavaUtils.serverInstancePlatform(instance).getSpecification().getVersion();
SpecificationVersion firstJavaVersionWithModules = new SpecificationVersion("9");

if(currentInstanceJavaVersion.compareTo(firstJavaVersionWithModules)>=0){
optList.add("--add-opens java.base/java.lang=ALL-UNNAMED");
optList.add("--add-opens java.naming/javax.naming.spi=ALL-UNNAMED");
}
}

private String selectDebugPort() throws IOException {
int debugPort = 9009;
ServerSocket t = null;
Expand Down
19 changes: 0 additions & 19 deletions enterprise/glassfish.common/src/org/netbeans/modules/glassfish/common/utils/JavaUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,23 +309,4 @@ public static boolean checkAndRegisterJavaPlatform(final String javaHome) {
return false;
}

/**
* Return the Java SE platforms used by a by GlassFish server instance.
* <p/>
* Have to do this because org.netbeans.modules.glassfish.common.ui.VmCustomizer
* in persistFields() sets the platform to null if it's the default one.
* <p/>
* @param instance GlassFish server instance for which to find the
* selected platform.
* @return Java SE platform {@see JavaPlatform} object selected to use
* by GlassFish server.
*/
public static JavaPlatform serverInstancePlatform(
@NonNull final GlassfishInstance instance) {
JavaPlatform serverInstanceJavaPlatform = instance.getJavaPlatform();
if(serverInstanceJavaPlatform==null){
serverInstanceJavaPlatform=JavaPlatformManager.getDefault().getDefaultPlatform();
}
return serverInstanceJavaPlatform;
}
}
69 changes: 33 additions & 36 deletions ...rise/glassfish.tooling/src/org/netbeans/modules/glassfish/tooling/server/ServerTasks.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,19 +303,33 @@ private static String computeClassPath(Map<String, String> propMap,
*/
private static void appendOptions(StringBuilder argumentBuf,
List<String> optList, Map<String, String> varMap) {
final String METHOD = "appendOptions";
HashMap<String, String> keyValueArgs = new HashMap<>();
LinkedList<String> keyOrder = new LinkedList<>();
String name, value;
final boolean isWindows = OsUtils.isWin();
final String METHOD = "appendOptions"; // NOI18N
// override the values that are found in the domain.xml file.
// this is totally a copy/paste from StartTomcat...
final Map<String,String> PROXY_PROPS = new HashMap<>();
for(String s: new String[] {
"http.proxyHost", // NOI18N
"http.proxyPort", // NOI18N
"http.nonProxyHosts", // NOI18N
"https.proxyHost", // NOI18N
"https.proxyPort", // NOI18N
}) {
PROXY_PROPS.put(JavaUtils.systemPropertyName(s), s);
PROXY_PROPS.put("-D\"" + s, s); // NOI18N
PROXY_PROPS.put("-D" + s, s); // NOI18N
}

// first process optList aquired from domain.xml
for (String opt : optList) {
String name, value;
// do placeholder substitution
opt = Utils.doSub(opt.trim(), varMap);
int splitIndex = opt.indexOf('=');
// && !opt.startsWith("-agentpath:") is a temporary hack to
// not touch already quoted -agentpath. Later we should handle it
// in a better way.
if (splitIndex != -1 && !opt.startsWith("-agentpath:")) {
if (splitIndex != -1 && !opt.startsWith("-agentpath:")) { // NOI18N
// key=value type of option
name = opt.substring(0, splitIndex);
value = Utils.quote(opt.substring(splitIndex + 1));
Expand All @@ -325,44 +339,27 @@ private static void appendOptions(StringBuilder argumentBuf,
} else {
name = opt;
value = null;
LOGGER.log(Level.FINER, METHOD, "jvmOpt", name);
}
if (!keyValueArgs.containsKey(name)) {
keyOrder.add(name);
LOGGER.log(Level.FINER, METHOD, "jvmOpt", name); // NOI18N
}
keyValueArgs.put(name, value);
}

// override the values that are found in the domain.xml file.
// this is totally a copy/paste from StartTomcat...
final String[] PROXY_PROPS = {
"http.proxyHost", // NOI18N
"http.proxyPort", // NOI18N
"http.nonProxyHosts", // NOI18N
"https.proxyHost", // NOI18N
"https.proxyPort", // NOI18N
};
boolean isWindows = OsUtils.isWin();
for (String prop : PROXY_PROPS) {
value = System.getProperty(prop);
if (value != null && value.trim().length() > 0) {
if (isWindows && "http.nonProxyHosts".equals(prop)) {
// enclose in double quotes to escape the pipes separating
// the hosts on windows
value = "\"" + value + "\""; // NOI18N
if(PROXY_PROPS.containsKey(name)) {
String sysValue = System.getProperty(PROXY_PROPS.get(name));
if (sysValue != null && sysValue.trim().length() > 0) {
if (isWindows && "http.nonProxyHosts".equals(PROXY_PROPS.get(name))) { // NOI18N
// enclose in double quotes to escape the pipes separating
// the hosts on windows
sysValue = "\"" + value + "\""; // NOI18N
}
name = JavaUtils.systemPropertyName(PROXY_PROPS.get(name));
value = sysValue;
}
keyValueArgs.put(JavaUtils.systemPropertyName(prop), value);
}
}

// appending key=value options to the command line argument
// using the same order as they came in argument - important!
for (String key : keyOrder) {
argumentBuf.append(' ');
argumentBuf.append(key);
if (keyValueArgs.get(key) != null) {
argumentBuf.append(name);
if(value != null) {
argumentBuf.append("="); // NOI18N
argumentBuf.append(keyValueArgs.get(key));
argumentBuf.append(value);
}
}
}
Expand Down

0 comments on commit 3668e87

Please sign in to comment.