Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8274397: [macOS] Stop setting env. var JAVA_MAIN_CLASS_<pid> in launcher code #5724

Closed
wants to merge 6 commits into from
88 changes: 63 additions & 25 deletions src/java.base/macosx/native/libjli/java_md_macosx.m
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ static void MacOSXStartup(int argc, char *argv[]) {
* change drastically between update release, and it may even be
* removed or replaced with another mechanism.
*
* NOTE: It is used by SWT, and JavaFX.
* NOTE: It is used by SWT
*/
snprintf(envVar, sizeof(envVar), "APP_NAME_%d", getpid());
setenv(envVar, (arg + 12), 1);
Expand Down Expand Up @@ -828,40 +828,78 @@ static void MacOSXStartup(int argc, char *argv[]) {
jmethodID getCanonicalNameMID = NULL;
NULL_CHECK(getCanonicalNameMID = (*env)->GetMethodID(env, classClass, "getCanonicalName", "()Ljava/lang/String;"));

jclass strClass = NULL;
NULL_CHECK(strClass = (*env)->FindClass(env, "java/lang/String"));

jmethodID lastIndexMID = NULL;
NULL_CHECK(lastIndexMID = (*env)->GetMethodID(env, strClass, "lastIndexOf", "(I)I"));

jmethodID subStringMID = NULL;
NULL_CHECK(subStringMID = (*env)->GetMethodID(env, strClass, "substring", "(I)Ljava/lang/String;"));

jstring mainClassString = (*env)->CallObjectMethod(env, mainClass, getCanonicalNameMID);
if ((*env)->ExceptionCheck(env) || NULL == mainClassString) {
/*
* Clears all errors caused by getCanonicalName() on the mainclass and
* leaves the JAVA_MAIN_CLASS__<pid> empty.
*/
(*env)->ExceptionClear(env);
return;
}

const char *mainClassName = NULL;
NULL_CHECK(mainClassName = (*env)->GetStringUTFChars(env, mainClassString, NULL));
jint lastPeriod = (*env)->CallIntMethod(env, mainClassString, lastIndexMID, (jint)'.');
if ((*env)->ExceptionCheck(env)) {
(*env)->ExceptionClear(env);
return;
}

char envVar[80];
/*
* The JAVA_MAIN_CLASS_<pid> environment variable is used to pass
* the name of a Java class whose main() method is invoked by
* the Java launcher code to start the application, to the AWT code
* in order to assign the name to the Apple menu bar when the app
* is active on the Mac.
* The _<pid> part is added to avoid collisions with child processes.
*
* WARNING: This environment variable is an implementation detail and
* isn't meant for use outside of the core platform. The mechanism for
* passing this information from Java launcher to other modules may
* change drastically between update release, and it may even be
* removed or replaced with another mechanism.
if (lastPeriod != -1) {
mainClassString = (*env)->CallObjectMethod(env, mainClassString, subStringMID, lastPeriod+1);
if ((*env)->ExceptionCheck(env)) {
(*env)->ExceptionClear(env);
return;
}
}

/* There are multiple apple.awt.*" system properties that AWT(the desktop module)
* references that are inherited from Apple JDK.
* This inherited AWT code looks for this property and uses it for the name
* of the app as it appears in the system menu bar.
*
* NOTE: It is used by SWT, and JavaFX.
* No idea if how much external code ever sets it, but use it if set, else
* if not set (the high probability event) set it to the application class name.
*/
snprintf(envVar, sizeof(envVar), "JAVA_MAIN_CLASS_%d", getpid());
setenv(envVar, mainClassName, 1);
const char* propName = "apple.awt.application.name";
jstring jKey = NULL;
NULL_CHECK(jKey = (*env)->NewStringUTF(env, propName));

jclass sysClass = NULL;
NULL_CHECK(sysClass = (*env)->FindClass(env, "java/lang/System"));

jmethodID getPropertyMID = NULL;
NULL_CHECK(getPropertyMID = (*env)->GetStaticMethodID(env, sysClass,
"getProperty", "(Ljava/lang/String;)Ljava/lang/String;"));

jmethodID setPropertyMID = NULL;
NULL_CHECK(setPropertyMID = (*env)->GetStaticMethodID(env, sysClass,
"setProperty",
"(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;"));

jstring jValue = (*env)->CallStaticObjectMethod(env, sysClass, getPropertyMID, jKey);
if ((*env)->ExceptionCheck(env)) {
(*env)->ExceptionClear(env);
(*env)->DeleteLocalRef(env, jKey);
return;
}
if (jValue == NULL) {
(*env)->CallStaticObjectMethod(env, sysClass, setPropertyMID,
jKey, mainClassString);
if ((*env)->ExceptionCheck(env)) {
(*env)->ExceptionClear(env);
(*env)->DeleteLocalRef(env, jKey);
return;
}
} else {
(*env)->DeleteLocalRef(env, jValue);
}

(*env)->ReleaseStringUTFChars(env, mainClassString, mainClassName);
(*env)->DeleteLocalRef(env, jKey);
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure about error handling, the jkey is not removed when NULL_CHECK is used. Also an exceptions are not cleared in case of NULL_CHECK like in other cases, is it intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well .. they aren't removed by the existing code either. And this is the launcher.
So far as I can tell if we error out of here (as I found when I made a typo in a method signature) the
VM exits very rapidly. So if I do anything here, it would be to remove DeleteLocalRef since it really doesn't matter to clean up local refs when you are either bailing from native .. or the entire process .. cleaning up when you are continuing on (as the code does) is perhaps more important since you might need more local refs before you are done.
Or file a separate bug against the launcher and JNI clean up after error handling.
java.base/share/native/libjli/java.c is a good example of where the same pattern exists.

}

void
Expand Down
17 changes: 1 addition & 16 deletions src/java.desktop/macosx/native/libosxapp/NSApplicationAWT.m
Original file line number Diff line number Diff line change
Expand Up @@ -180,26 +180,11 @@ - (void) registerWithProcessManager
}

// If it wasn't specified as an argument, see if it was specified as a system property.
// The launcher code sets this if it is not already set on the command line.
if (fApplicationName == nil) {
fApplicationName = [PropertiesUtilities javaSystemPropertyForKey:@"apple.awt.application.name" withEnv:env];
}

// If we STILL don't have it, the app name is retrieved from an environment variable (set in java.c) It should be UTF8.
if (fApplicationName == nil) {
char mainClassEnvVar[80];
snprintf(mainClassEnvVar, sizeof(mainClassEnvVar), "JAVA_MAIN_CLASS_%d", getpid());
char *mainClass = getenv(mainClassEnvVar);
if (mainClass != NULL) {
fApplicationName = [NSString stringWithUTF8String:mainClass];
unsetenv(mainClassEnvVar);

NSRange lastPeriod = [fApplicationName rangeOfString:@"." options:NSBackwardsSearch];
if (lastPeriod.location != NSNotFound) {
fApplicationName = [fApplicationName substringFromIndex:lastPeriod.location + 1];
}
}
}

// The dock name is nil for double-clickable Java apps (bundled and Web Start apps)
// When that happens get the display name, and if that's not available fall back to
// CFBundleName.
Expand Down
68 changes: 68 additions & 0 deletions test/jdk/tools/launcher/MacOSAppNamePropertyTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/**
* @test
* @bug 8274397
* @summary Ensure the app name system property is set on macOS
* @requires os.family == "mac"
* @compile MacOSAppNamePropertyTest.java SystemPropertyTest.java
* @run main MacOSAppNamePropertyTest
*/

import java.util.ArrayList;
import java.util.List;
/*
* If the system property apple.awt.application.name is unset, it should default
* to the name of this test class.
* If it is set, then it should be used instead of the class.
* The arg. to the test indicates the *expected* name.
* The test will fail if the property is not set or does not match
*/
public class MacOSAppNamePropertyTest extends TestHelper {

static final String APPNAME = "SystemPropertyTest";

public static void main(String[]args) {
if (!isMacOSX) {
return;
}
execTest(null, APPNAME);
execTest("-Dapple.awt.application.name=Foo", "Foo");
}

static void execTest(String propSetting, String expect) {
List<String> cmdList = new ArrayList<>();
cmdList.add(javaCmd);
if (propSetting != null) {
cmdList.add(propSetting);
}
cmdList.add(APPNAME);
cmdList.add(expect);
TestResult tr = doExec(cmdList.toArray(new String[cmdList.size()]));
if (!tr.isOK()) {
System.err.println(tr.toString());
throw new RuntimeException("Test Fails");
}
}
}
44 changes: 44 additions & 0 deletions test/jdk/tools/launcher/SystemPropertyTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/


/*
* Child launched by MacOSAppNamePropertyTest.java
* If the system property apple.awt.application.name is unset, it should default
* to the name of this main program class, less any package name.
* If it is set, then it should be used instead of the class name.
* The arg. to the test indicates the *expected* name.
* The test will fail if the property is not set or does not match
*/
public class SystemPropertyTest {

public static void main(String[]args) {
String prop = System.getProperty("apple.awt.application.name");
if (prop == null) {
throw new RuntimeException("Property not set");
}
if (!prop.equals(args[0])) {
throw new RuntimeException("Got " + prop + " expected " + args[0]);
}
}
}
6 changes: 1 addition & 5 deletions test/jdk/tools/launcher/TestSpecialArgs.java
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,11 @@ void testDocking() {
Set<String> envToRemove = new HashSet<>();
Map<String, String> map = System.getenv();
for (String s : map.keySet()) {
if (s.startsWith("JAVA_MAIN_CLASS_")
|| s.startsWith("APP_NAME_")
if (s.startsWith("APP_NAME_")
|| s.startsWith("APP_ICON_")) {
envToRemove.add(s);
}
}
runTest(envToRemove, javaCmd, "-cp", TEST_CLASSES_DIR.getAbsolutePath(),
"EnvironmentVariables", "JAVA_MAIN_CLASS_*",
"EnvironmentVariables");

runTest(envToRemove, javaCmd, "-cp", TEST_CLASSES_DIR.getAbsolutePath(),
"-Xdock:name=TestAppName", "EnvironmentVariables",
Expand Down