bitflicker64 commented on code in PR #3253:
URL: https://github.com/apache/hugegraph/pull/3253#discussion_r4383328281
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/start-monitor.sh:
##########
@@ -37,6 +37,35 @@ fi
# Monitor HugeGraphServer every minute, if the server crashes then restart it.
# Modify the frequency according to actual needs carefully.
-CRONTAB_JOB="*/1 * * * * export JAVA_HOME=$JAVA_HOME &&
$TOP/bin/monitor-hugegraph.sh"
-crontab_append "$CRONTAB_JOB"
+# Persist any path overrides (-c/-l/-i/-o) that the caller
(start-hugegraph.sh) exported.
+# Every value goes through cron_quote: it validates (CR/LF, '%') and emits ONE
complete
+# POSIX shell word.
+JAVA_HOME_Q="$(cron_quote "$JAVA_HOME" "JAVA_HOME")" || exit 1
+MONITOR_RAW="$TOP/bin/monitor-hugegraph.sh"
+MONITOR_Q="$(cron_quote "$MONITOR_RAW" "monitor script path")" || exit 1
+
+CRONTAB_JOB="*/1 * * * * export JAVA_HOME=$JAVA_HOME_Q &&"
+if [ -n "$CONF_OVERRIDE" ]; then
+ CONF_Q="$(cron_quote "$CONF_OVERRIDE" "CONF_OVERRIDE")" || exit 1
+ CRONTAB_JOB="$CRONTAB_JOB export CONF_OVERRIDE=$CONF_Q &&"
+fi
+if [ -n "$LOGS_OVERRIDE" ]; then
+ LOGS_Q="$(cron_quote "$LOGS_OVERRIDE" "LOGS_OVERRIDE")" || exit 1
+ CRONTAB_JOB="$CRONTAB_JOB export LOGS_OVERRIDE=$LOGS_Q &&"
+fi
+if [ -n "$PID_FILE_OVERRIDE" ]; then
+ PID_Q="$(cron_quote "$PID_FILE_OVERRIDE" "PID_FILE_OVERRIDE")" || exit 1
+ CRONTAB_JOB="$CRONTAB_JOB export PID_FILE_OVERRIDE=$PID_Q &&"
+fi
+if [ -n "$PLUGINS_OVERRIDE" ]; then
+ PLUGINS_Q="$(cron_quote "$PLUGINS_OVERRIDE" "PLUGINS_OVERRIDE")" || exit 1
+ CRONTAB_JOB="$CRONTAB_JOB export PLUGINS_OVERRIDE=$PLUGINS_Q &&"
+fi
+CRONTAB_JOB="$CRONTAB_JOB $MONITOR_Q"
+
+# Keys cover both the current quoted form and the legacy unquoted form (older
+# start-monitor.sh versions wrote "... && $TOP/bin/monitor-hugegraph.sh"
+# without quoting), so an in-place upgrade detects an existing job as a
+# duplicate instead of installing a second one alongside it.
+crontab_append "$CRONTAB_JOB" "$MONITOR_Q" "$MONITOR_RAW"
Review Comment:
Important: `$MONITOR_RAW` matches as a substring, so with a monitor for
`/data/opt/hg` in the crontab, `-m true` for `/opt/hg` counts it as a duplicate
and installs nothing, and `stop-monitor.sh` for `/opt/hg` removes the other
install's job. Anchor the legacy key in both scripts, for example `"&&
$MONITOR_RAW"`.
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/util.sh:
##########
@@ -318,32 +318,323 @@ function check_port() {
return 0
}
-function crontab_append() {
- local job="$1"
- crontab -l | grep -F "$job" >/dev/null 2>&1
- if [ $? -eq 0 ]; then
+# Emit $1 as ONE complete POSIX shell word, INCLUDING the surrounding quotes.
+# Callers should NOT add another pair of quotes:
+# /data/team's-logs -> '/data/team'\''s-logs'
+#
+function shell_quote() {
+ local sq="'" repl="'\\''"
+ printf "'%s'" "${1//$sq/$repl}"
+}
+
+# Validate a path-like value. $1=value $2=label for the error
$3=crlf|cron|classpath
+# crlf : reject CR/LF. Call on the RAW value (e.g. $OPTARG) BEFORE
+# canonicalize_dir/canonicalize_file.
+# cron : crlf + reject '%'.
+# classpath : crlf + reject ':' and ';'. For a directory whose contents are
+# joined into a ':'-separated CLASSPATH (the plugins dir), where
+# a ':' in the path itself would split one entry into two bogus
+# ones. On Cygwin, hugegraph-server.sh converts that classpath
+# with 'cygpath -p -w', which re-delimits it on ';' instead, so a
+# ';' in the path would do the same splitting there.
+function reject_unsafe_path() {
+ case "$1" in
+ *[$'\n\r']*)
+ echo "ERROR: $2 must not contain CR/LF characters" >&2
+ return 1 ;;
+ esac
+
+ case "$3" in
+ cron)
+ case "$1" in
+ *%*)
+ echo "ERROR: $2 must not contain '%' (reserved by cron)"
>&2
+ return 1 ;;
+ esac ;;
+ classpath)
+ case "$1" in
+ *:*)
+ echo "ERROR: $2 must not contain ':' (CLASSPATH
separator)" >&2
+ return 1 ;;
+ *';'*)
+ echo "ERROR: $2 must not contain ';' (CLASSPATH separator
on Cygwin)" >&2
+ return 1 ;;
+ esac ;;
+ esac
+ return 0
+}
+
+# Validate (cron rules) then quote. $1=value $2=label. Prints the quoted word.
+function cron_quote() {
+ reject_unsafe_path "$1" "$2" cron || return 1
+ shell_quote "$1"
+}
+
+# Fail fast when a required config file is missing or unreadable.
+# read_property() returns an empty string in that case, which otherwise looks
+# exactly like "the property is not set" and lets startup continue with silent
+# defaults.
+# $1 = file $2 = for error message
+function ensure_file_readable() {
+ local file="$1" description="$2"
+ if [ ! -f "$file" ]; then
+ echo "ERROR: missing $description: '$file'" >&2
+ if [ -n "${CONF_OVERRIDE:-}" ]; then
+ echo " The conf dir passed with -c must contain the same
files" >&2
+ echo " as the packaged conf directory." >&2
+ fi
return 1
fi
- (crontab -l ; echo "$job") | crontab -
+ if [ ! -r "$file" ]; then
+ echo "ERROR: unreadable $description: '$file'" >&2
+ echo " Check the file's ownership and permissions." >&2
+ return 1
+ fi
+ return 0
}
-function crontab_remove() {
- local job="$1"
- # check exist before remove
- crontab -l | grep -F "$job" >/dev/null 2>&1
- if [ $? -eq 1 ]; then
+# A relative path inside a config file is resolved by the JVM against the
+# working directory, which the start scripts set to $TOP -- not against the
+# directory the config file itself lives in. So when -c points at an external
+# conf dir, a value such as 'graphs=./conf/graphs' still loads
+# $TOP/conf/graphs and the operator's copy is ignored with no message at all.
+# Refuse to start in such case when -c is set and say exactly what to change.
+# $1 = value read from the config $2 = property name $3 = config file
+function reject_relative_conf_path() {
+ local value="$1" property="$2" file="$3"
+ # Only an overridden conf dir can disagree with the working directory.
+ [ -z "${CONF_OVERRIDE:-}" ] && return 0
+ [ -z "$value" ] && return 0
+ case "$value" in
+ /*) return 0 ;;
+ esac
+ # Build the suggestion: the stock values are written relative to $TOP (e.g.
+ # './conf/graphs'), while $CONF_OVERRIDE already IS the conf dir, so drop a
+ # leading './' and a leading 'conf/' to avoid suggesting
'.../conf/conf/...'.
+ local suggestion="${value#./}"
+ suggestion="${suggestion#conf/}"
+ echo "ERROR: '$property' is the relative path '$value' in $file." >&2
+ echo " Relative paths resolve against the working directory, so
with" >&2
+ echo " -c '$CONF_OVERRIDE' this would silently keep using the
packaged" >&2
+ echo " directory instead of your copy. Set '$property' to an
absolute" >&2
+ echo " path (for example '$CONF_OVERRIDE/$suggestion')." >&2
+ return 1
+}
+
+# Serialize concurrent cron edits (across both processes and scripts: start,
+# stop and restart can all touch the crontab around the same time). Uses
+# flock(1) when available; falls back to an atomic mkdir-based lock with a
+# stale-lock timeout on systems without flock (e.g. stock macOS).
+# NOTE: neither path is reentrant. A process that already holds this lock
+# must not call crontab_lock_acquire (directly, or via crontab_append /
+# crontab_remove) again before releasing it: flock(1) keys locks off the open
+# file description, not the pid, so a second acquire from the SAME process
+# blocks on itself for the full CRONTAB_LOCK_TIMEOUT_S and then fails.
+CRONTAB_LOCK_FILE="${TMPDIR:-/tmp}/.hugegraph-crontab.lock"
Review Comment:
Important: every account shares this lock file in `/tmp`, and
`crontab_lock_acquire` fails without a message when it cannot open it (another
owner's file, or `fs.protected_regular`, which blocks even `root`), so
`stop-monitor.sh` returns 1 and `stop-hugegraph.sh` leaves the monitor job that
restarts the server within a minute. Key the lock to the user, for example
`.hugegraph-crontab-$(id -u).lock`, and print an error when it cannot be opened.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
