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

jetty.sh ignores JAVA_OPTIONS environment variable #7754

Closed
TimePerformance opened this issue Mar 20, 2022 · 4 comments · Fixed by #7819
Closed

jetty.sh ignores JAVA_OPTIONS environment variable #7754

TimePerformance opened this issue Mar 20, 2022 · 4 comments · Fixed by #7819
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@TimePerformance
Copy link

Jetty version(s)
10.0.8

Java version/vendor (use: java -version)
OpenJDK Runtime Environment Corretto-17.0.2.8.1

OS type/version
Amazon Linux 2

Description
Jetty failed to start after migrating from 10.0.6 to 10.0.8 (no jetty-start.log, no jetty.log)

What is strange is that in jetty.sh, JAVA_OPTIONS is mentioned in the doc in the header of the file but it is not used (except in dumpEnv).

I've replaced the following lines:

JETTY_SYS_PROPS=$(echo -ne "-Djetty.home=$JETTY_HOME" "-Djetty.base=$JETTY_BASE" "-Djava.io.tmpdir=$TMPDIR")
RUN_ARGS=$("$JAVA" -jar "$JETTY_START" --dry-run=opts,path,main,args ${JETTY_ARGS[*]})
RUN_CMD=("$JAVA" $JETTY_SYS_PROPS ${RUN_ARGS[@]})

(note: having "$JAVA" in RUN_ARGS is strange)

by the corresponding lines from jetty.sh in version 10.0.6:

JAVA_OPTIONS=(${JAVA_OPTIONS[]} "-Djetty.home=$JETTY_HOME" "-Djetty.base=$JETTY_BASE" "-Djava.io.tmpdir=$TMPDIR")
RUN_ARGS=(${JAVA_OPTIONS[@]} -jar "$JETTY_START" ${JETTY_ARGS[
]})
RUN_CMD=("$JAVA" ${RUN_ARGS[@]})

and now it starts.

@TimePerformance TimePerformance added the Bug For general bugs on Jetty side label Mar 20, 2022
@joakime
Copy link
Contributor

joakime commented Mar 20, 2022

RUN_ARGS must be built from --dry-run to avoid a forked JVM.

That means JAVA_OPTIONS must come from your ${jetty.base} configuration (see jvm module).

Your change allows for a forked JVM.

The other option you have is to use $JETTY_SYS_PROPS instead of $JAVA_OPTIONS. (this is because $JAVA_OPTIONS does not carry over to the forked JVM, but the $JETTY_SYS_PROPS will)

@TimePerformance
Copy link
Author

thanks a lot for the explanation

@TimePerformance
Copy link
Author

It seems that $JETTY_SYS_PROPS is redefined in the jetty.sh like that
JETTY_SYS_PROPS=$(echo -ne "-Djetty.home=$JETTY_HOME" "-Djetty.base=$JETTY_BASE" "-Djava.io.tmpdir=$TMPDIR"
Thus previous value of $JETTY_SYS_PROPS is ignored

I've tried to use the jvm module. I've set system properties in jvm.ini with the -DMY_VAR=VALUE syntax. However, in the RUN_CMD generated by the dry run, ${MY_VAR} are not expanded which causes troubles.

For instance,
in jvm.ini:
--exec
-DWORKERNAME=localhost

in sessions.ini:
jetty.sessionIdManager.workerName=${WORKERNAME}

The RUN_CMD contains -DWORKERNAME=localhost jetty.sessionIdManager.workerName=${WORKERNAME} but ${WORKERNAME} is not expanded even when it is run.

@joakime
Copy link
Contributor

joakime commented Apr 1, 2022

for property expansion in start modules (be it in *.mod, or *.ini, or *.xml), use simple post-jar properties, don't rely on system properties (that's last resort, backup solution, not the primary solution)

for your example, the use of JETTY_ARGS to set the simple property should be enough. (don't prepend with -D)

JETTY_ARGS="WORKERNAME=localhost"

@joakime joakime self-assigned this Apr 1, 2022
joakime added a commit that referenced this issue Apr 1, 2022
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime joakime added this to To do in Jetty 10.0.10/11.0.10 - 🧊 FROZEN 🥶 via automation Apr 1, 2022
@joakime joakime moved this from To do to In progress in Jetty 10.0.10/11.0.10 - 🧊 FROZEN 🥶 Apr 1, 2022
Jetty 10.0.10/11.0.10 - 🧊 FROZEN 🥶 automation moved this from In progress to Done May 26, 2022
joakime added a commit that referenced this issue May 26, 2022
…7819)

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants