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.

Sign up for GitHub

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

Jump to bottom

GG-36475 Fix the illegal reflective access warnings issue #3070

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

GG-36475 Fix the illegal reflective access warnings issue #3070

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2543845
gg-36475 Fix the illegal reflective access warnings issue
dy2285 Feb 1, 2024
b7386ce
To correct the mistake of adding a wrong file controlsh
dy2285 Mar 4, 2024
3311898
Remove unwanted file controlsh
dy2285 Mar 6, 2024
6f3416b
Update bin/control.sh and bin/include/jvmdefaults.sh
dy2285 Mar 13, 2024
51bb31e
Merge branch 'master' into gg-36475
dy2285 Mar 13, 2024
0f58a07
Update control scripts and JVM defaults to handle Java version compat…
dy2285 May 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions bin/control.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,20 @@ fi
# Final CONTROL_JVM_OPTS for Java 9+ compatibility
#
CONTROL_JVM_OPTS=$(getJavaSpecificOpts $version "$CONTROL_JVM_OPTS")
CONTROL_JVM_OPTS="${CONTROL_JVM_OPTS} --add-exports=java.base/jdk.internal.misc=ALL-UNNAMED"
CONTROL_JVM_OPTS="${CONTROL_JVM_OPTS} --add-exports=java.base/sun.nio.ch=ALL-UNNAMED"
CONTROL_JVM_OPTS="${CONTROL_JVM_OPTS} --add-opens=java.base/jdk.internal.misc=ALL-UNNAMED"
CONTROL_JVM_OPTS="${CONTROL_JVM_OPTS} --add-opens=java.base/sun.nio.ch=ALL-UNNAMED"
CONTROL_JVM_OPTS="${CONTROL_JVM_OPTS} --add-opens=java.base/sun.nio.ch=ALL-UNNAMED"
CONTROL_JVM_OPTS="${CONTROL_JVM_OPTS} --add-opens=java.management/com.sun.jmx.mbeanserver=ALL-UNNAMED"
CONTROL_JVM_OPTS="${CONTROL_JVM_OPTS} --add-opens=jdk.internal.jvmstat/sun.jvmstat.monitor=ALL-UNNAMED"
CONTROL_JVM_OPTS="${CONTROL_JVM_OPTS} --add-opens=java.base/sun.reflect.generics.reflectiveObjects=ALL-UNNAMED"
CONTROL_JVM_OPTS="${CONTROL_JVM_OPTS} --add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED"
CONTROL_JVM_OPTS="${CONTROL_JVM_OPTS} --add-opens=java.base/java.io=ALL-UNNAMED"
CONTROL_JVM_OPTS="${CONTROL_JVM_OPTS} --add-opens=java.base/java.nio=ALL-UNNAMED"
CONTROL_JVM_OPTS="${CONTROL_JVM_OPTS} --add-opens=java.base/java.util=ALL-UNNAMED"
CONTROL_JVM_OPTS="${CONTROL_JVM_OPTS} --add-opens=java.base/java.lang=ALL-UNNAMED"
CONTROL_JVM_OPTS="${CONTROL_JVM_OPTS} --illegal-access=warn"
Copy link
Contributor

Choose a reason for hiding this comment

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

If I get it right this changes have to be reverted since you moved options to a separate file


if [ -n "${JVM_OPTS}" ] ; then
echo "JVM_OPTS environment variable is set, but will not be used. To pass JVM options use CONTROL_JVM_OPTS"
Expand Down
178 changes: 178 additions & 0 deletions bin/controlsh
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

This file should not exist.

Copy link
Author

Choose a reason for hiding this comment

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

removed.

Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
#!/usr/bin/env bash
if [ ! -z "${IGNITE_SCRIPT_STRICT_MODE:-}" ]
then
set -o nounset
set -o errexit
set -o pipefail
set -o errtrace
set -o functrace
fi

#
# Copyright 2019 GridGain Systems, Inc. and Contributors.
#
# Licensed under the GridGain Community Edition License (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# https://www.gridgain.com/products/software/community-edition/gridgain-community-edition-license
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

#
# Grid cluster control.
#

#
# Import common functions.
#
if [ "${IGNITE_HOME:-}" = "" ];
then IGNITE_HOME_TMP="$(dirname "$(cd "$(dirname "$0")"; "pwd")")";
else IGNITE_HOME_TMP=${IGNITE_HOME};
fi

#
# Set SCRIPTS_HOME - base path to scripts.
#
SCRIPTS_HOME="${IGNITE_HOME_TMP}/bin"
. "${SCRIPTS_HOME}"/include/functions.sh

# Include JVM options from jvmdefaults.sh
#. "${SCRIPTS_HOME}"/include/jvmdefaults.sh ||exit 1
. ./jvmdefaults_withfunction.sh ||exit 1

# Call the function
addReflectiveAccessOptions

# Use the function
CONTROL_JVM_OPTS="${CONTROL_JVM_OPTS} $(addReflectiveAccessOptions)"

#
# Discover path to Java executable and check its version.
#
checkJava

#
# Discover IGNITE_HOME environment variable.
#
setIgniteHome

if [ "${DEFAULT_CONFIG:-}" == "" ]; then
DEFAULT_CONFIG=config/default-config.xml
fi

#
# Set IGNITE_LIBS.
#
. "${SCRIPTS_HOME}"/include/setenv.sh

CP="${IGNITE_LIBS}:${IGNITE_HOME}/libs/optional/ignite-zookeeper/*"

RANDOM_NUMBER=$("$JAVA" -cp "${CP}" org.apache.ignite.startup.cmdline.CommandLineRandomNumberGenerator)

# Mac OS specific support to display correct name in the dock.
osname=`uname`

if [ "${DOCK_OPTS:-}" == "" ]; then
DOCK_OPTS="-Xdock:name=Ignite Node"
fi

#
# JVM options. See http://java.sun.com/javase/technologies/hotspot/vmoptions.jsp for more details.
#
# ADD YOUR/CHANGE ADDITIONAL OPTIONS HERE
#
if [ -z "${CONTROL_JVM_OPTS:-}" ] ; then
if [[ `"$JAVA" -version 2>&1 | egrep "1\.[7]\."` ]]; then
CONTROL_JVM_OPTS="-Xms256m -Xmx1g"
else
CONTROL_JVM_OPTS="-Xms256m -Xmx1g"
fi
fi

#
# Uncomment to enable experimental commands [--wal]
#
# CONTROL_JVM_OPTS="${CONTROL_JVM_OPTS} -DIGNITE_ENABLE_EXPERIMENTAL_COMMAND=true"

#
# Uncomment the following GC settings if you see spikes in your throughput due to Garbage Collection.
#
# CONTROL_JVM_OPTS="$CONTROL_JVM_OPTS -XX:+UseG1GC"

#
# Uncomment if you get StackOverflowError.
# On 64 bit systems this value can be larger, e.g. -Xss16m
#
# CONTROL_JVM_OPTS="${CONTROL_JVM_OPTS} -Xss4m"

#
# Uncomment to set preference for IPv4 stack.
#
# CONTROL_JVM_OPTS="${CONTROL_JVM_OPTS} -Djava.net.preferIPv4Stack=true"

#
# Assertions are disabled by default since version 3.5.
# If you want to enable them - set 'ENABLE_ASSERTIONS' flag to '1'.
#
ENABLE_ASSERTIONS="0"

#
# Set '-ea' options if assertions are enabled.
#
if [ "${ENABLE_ASSERTIONS:-}" = "1" ]; then
CONTROL_JVM_OPTS="${CONTROL_JVM_OPTS} -ea"
fi

#
# Set main class to start service (grid node by default).
#
if [ "${MAIN_CLASS:-}" = "" ]; then
MAIN_CLASS=org.apache.ignite.internal.commandline.CommandHandler
fi

#
# Remote debugging (JPDA).
# Uncomment and change if remote debugging is required.
#
# CONTROL_JVM_OPTS="-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=8787 ${CONTROL_JVM_OPTS}"

#
# Final CONTROL_JVM_OPTS for Java 9+ compatibility
#



# Get Java specific options
CONTROL_JVM_OPTS=$(getJavaSpecificOpts $version "$CONTROL_JVM_OPTS")



if [ -n "${JVM_OPTS}" ] ; then
echo "JVM_OPTS environment variable is set, but will not be used. To pass JVM options use CONTROL_JVM_OPTS"
echo "JVM_OPTS=${JVM_OPTS}"
fi

case $osname in
Darwin*)
"$JAVA" ${CONTROL_JVM_OPTS} ${QUIET:-} "${DOCK_OPTS}" \
-DIGNITE_HOME="${IGNITE_HOME}" \
-DIGNITE_PROG_NAME="$0" ${JVM_XOPTS:-} -cp "${CP}" ${MAIN_CLASS} "$@"
;;
OS/390*)
"$JAVA" ${CONTROL_JVM_OPTS} ${QUIET:-} \
-DIGNITE_HOME="${IGNITE_HOME}" \
-Dcom.ibm.jsse2.overrideDefaultTLS=true -Dssl.KeyManagerFactory.algorithm=IbmX509 \
-DIGNITE_PROG_NAME="$0" ${JVM_XOPTS:-} -cp "${CP}" ${MAIN_CLASS} "$@"
;;
*)
"$JAVA" ${CONTROL_JVM_OPTS} ${QUIET:-} \
-DIGNITE_HOME="${IGNITE_HOME}" \
-DIGNITE_PROG_NAME="$0" ${JVM_XOPTS:-} -cp "${CP}" ${MAIN_CLASS} "$@"
;;
esac
22 changes: 21 additions & 1 deletion bin/include/jvmdefaults.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ getJavaSpecificOpts() {
--add-modules=java.xml.bind \
${current_value}"

elif [ "${version}" -ge 11 ] && [ "${version}" -lt 15 ]; then
elif [ "${version}" -ge 11 ] && [ "${version}" -lt 14 ]; then
value="\
--add-exports=java.base/jdk.internal.misc=ALL-UNNAMED \
--add-exports=java.base/sun.nio.ch=ALL-UNNAMED \
Expand All @@ -62,8 +62,21 @@ getJavaSpecificOpts() {
--illegal-access=permit \
${current_value}"

elif [ "${version}" -ge 14 ] && [ "${version}" -lt 15 ]; then
value="\
--add-exports=java.base/jdk.internal.misc=ALL-UNNAMED \
--add-exports=java.base/sun.nio.ch=ALL-UNNAMED \
--add-exports=java.management/com.sun.jmx.mbeanserver=ALL-UNNAMED \
--add-exports=jdk.internal.jvmstat/sun.jvmstat.monitor=ALL-UNNAMED \
--add-exports=java.base/sun.reflect.generics.reflectiveObjects=ALL-UNNAMED \
--add-opens=java.base/jdk.internal.access=ALL-UNNAMED \
--add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED \
--illegal-access=permit \
${current_value}"

elif [ "${version}" -ge 15 ] ; then
value="\
--add-opens=java.base/jdk.internal.access=ALL-UNNAMED \
--add-opens=java.base/jdk.internal.misc=ALL-UNNAMED \
--add-opens=java.base/sun.nio.ch=ALL-UNNAMED \
--add-opens=java.management/com.sun.jmx.mbeanserver=ALL-UNNAMED \
Expand All @@ -82,10 +95,17 @@ getJavaSpecificOpts() {
--add-opens=java.base/java.lang.invoke=ALL-UNNAMED \
--add-opens=java.base/java.math=ALL-UNNAMED \
--add-opens=java.base/java.time=ALL-UNNAMED \
--add-opens=java.base/sun.security.ssl=ALL-UNNAMED \
--add-opens=java.base/sun.security.x509=ALL-UNNAMED \
--add-opens=java.sql/java.sql=ALL-UNNAMED \
${current_value}"
fi

echo $value
}
# Reflective access options
addReflectiveAccessOptions() {
Copy link
Contributor

@sk0x50 sk0x50 Apr 22, 2024 •
edited

Choose a reason for hiding this comment

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

I think the exact list of options should be based on the JVM version. For example, using these opts with JVM8 leads to

Unrecognized option: --add-exports=java.base/jdk.internal.misc=ALL-UNNAMED
Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.

Perhaps, I am missing something.

It would be nice to check the script works as expected using Java 8, 11, and 17
https://www.gridgain.com/docs/latest/getting-started/quick-start/java

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for highlighting the compatibility issues with Java 8. I realize now that my updates to the jvmdefaults.sh script included Java 9+ specific options without checks for Java version compatibility. I will revise the script to dynamically adjust the JVM options based on the version it detects.

Yes as recommended I will also conduct testing across these Java versions to confirm that the script functions correctly in each environment.

Once you've confirmed my approach regarding the initial comment of the PR, I will make all necessary changes and conduct all tests at once.

CONTROL_JVM_OPTS+="--add-exports=java.base/jdk.internal.misc=ALL-UNNAMED --add-opens java.base/java.nio=ALL-UNNAMED --add-opens=java.base/sun.nio.ch=ALL-UNNAMED --add-exports=java.management/com.sun.jmx.mbeanserver=ALL-UNNAMED --add-exports=jdk.internal.jvmstat/sun.jvmstat.monitor=ALL-UNNAMED --add-exports=java.base/sun.reflect.generics.reflectiveObjects=ALL-UNNAMED --add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED --illegal-access=warn"; }

# Export the function
export -f addReflectiveAccessOptions