imbajin commented on code in PR #3258:
URL: https://github.com/apache/hugegraph/pull/3258#discussion_r4174903922


##########
hugegraph-server/hugegraph-dist/src/assembly/travis/test-java-security-properties.sh:
##########
@@ -432,6 +450,144 @@ assert_argument "true" "$ENABLED_CAPTURE"
 assert_argument "-Doperator.marker=preserved" "$ENABLED_CAPTURE"
 assert_no_argument '^-D(networkaddress\.cache\.ttl|sun\.net\.inetaddr\.ttl)=' \
                    "$ENABLED_CAPTURE"
+# Heap dump and crash log defaults go first in JAVA_TOOL_OPTIONS, so the JVM's
+# own precedence lets every operator source override them. Check the values a
+# real JVM settles on when started the way a captured launcher run would start
+# it: the captured JAVA_TOOL_OPTIONS, the crash-related -XX: arguments, and
+# JDK_JAVA_OPTIONS.
+effective_flag() {
+    local capture="$1"
+    local flag="$2"
+    local jdk_java_options="${3:-}"
+    local args=()
+    local line
+    while IFS= read -r line; do
+        if [[ "$line" =~ 
^-XX:([+-]HeapDumpOnOutOfMemoryError|HeapDumpPath=|ErrorFile=) ]]; then
+            args+=("$line")
+        fi
+    done < "$capture"
+    JAVA_TOOL_OPTIONS="$(cat "${capture}.tool-options")" \
+        JDK_JAVA_OPTIONS="$jdk_java_options" \
+        "$JAVA_BIN" ${args[@]+"${args[@]}"} -XX:+PrintFlagsFinal -version \
+        2>/dev/null | awk -v flag="$flag" '$2 == flag { print $4; exit }'
+}
+
+assert_effective_flag() {
+    local capture="$1"
+    local flag="$2"
+    local pattern="$3"
+    local jdk_java_options="${4:-}"
+    local value
+    value=$(effective_flag "$capture" "$flag" "$jdk_java_options")
+    [[ "$value" =~ $pattern ]] ||
+        fail "effective ${flag} is '${value}', expected to match ${pattern}"
+}
+
+LOGS_PATTERN=$(printf '%s' "${SERVER_ROOT}/logs" | sed 
's/[][\.*^$+?(){}|]/\\&/g')
+# The names carry the launch time and, if needed, a counter, since a restarted
+# container often reuses the PID and HotSpot will not overwrite an existing
+# crash log or heap dump.
+HEAP_DUMP_PATTERN="^${LOGS_PATTERN}/java_pid[0-9]+_[0-9]{8}-[0-9]{6}(-[0-9]+)?\.hprof$"
+ERROR_FILE_PATTERN="^${LOGS_PATTERN}/hs_err_pid%p_[0-9]{8}-[0-9]{6}(-[0-9]+)?\.log$"
+
+assert_no_argument 
'^-XX:([+-]HeapDumpOnOutOfMemoryError|HeapDumpPath=|ErrorFile=)' \
+                   "$ENABLED_CAPTURE"
+assert_effective_flag "$ENABLED_CAPTURE" HeapDumpOnOutOfMemoryError '^true$'
+assert_effective_flag "$ENABLED_CAPTURE" HeapDumpPath "$HEAP_DUMP_PATTERN"
+assert_effective_flag "$ENABLED_CAPTURE" ErrorFile "$ERROR_FILE_PATTERN"
+
+# JAVA_OPTIONS replaces the default heap options; the crash defaults stay, and
+# paths given there win.
+CUSTOM_OPTIONS_CAPTURE="${TEMP_DIR}/custom-java-options.args"
+CAPTURE_FILE="$CUSTOM_OPTIONS_CAPTURE" JAVA_HOME="$MOCK_JAVA_HOME" \
+    JAVA_OPTIONS="-Xmx1g -XX:ErrorFile=/operator/hs_err.log \
+                  -XX:HeapDumpPath=/operator/dumps" \
+    STDOUT_MODE=true "$SERVER_SCRIPT" \
+    "${CONF}/gremlin-server.yaml" "${CONF}/rest-server.properties" true 
>/dev/null
+
+assert_argument "-Xmx1g" "$CUSTOM_OPTIONS_CAPTURE"
+assert_effective_flag "$CUSTOM_OPTIONS_CAPTURE" HeapDumpOnOutOfMemoryError 
'^true$'
+assert_effective_flag "$CUSTOM_OPTIONS_CAPTURE" HeapDumpPath 
'^/operator/dumps$'
+assert_effective_flag "$CUSTOM_OPTIONS_CAPTURE" ErrorFile 
'^/operator/hs_err\.log$'
+
+# The JVM, not the launcher, parses the operator's JAVA_TOOL_OPTIONS: a quoted
+# opt-out works, and flag-like text inside a property value changes nothing.
+QUOTED_TOOL_CAPTURE="${TEMP_DIR}/quoted-tool-options.args"
+CAPTURE_FILE="$QUOTED_TOOL_CAPTURE" JAVA_HOME="$MOCK_JAVA_HOME" \
+    JAVA_TOOL_OPTIONS='"-XX:-HeapDumpOnOutOfMemoryError" 
"-Dmarker=-XX:ErrorFile=/nope"' \
+    STDOUT_MODE=true "$SERVER_SCRIPT" \
+    "${CONF}/gremlin-server.yaml" "${CONF}/rest-server.properties" true 
>/dev/null
+
+assert_effective_flag "$QUOTED_TOOL_CAPTURE" HeapDumpOnOutOfMemoryError 
'^false$'
+assert_effective_flag "$QUOTED_TOOL_CAPTURE" ErrorFile "$ERROR_FILE_PATTERN"
+
+# JDK_JAVA_OPTIONS, including an @argfile, also overrides the defaults.
+ARGFILE="${TEMP_DIR}/jdk-java-options.args"
+echo '-XX:ErrorFile=/operator/argfile-hs_err.log' > "$ARGFILE"
+ARGFILE_CAPTURE="${TEMP_DIR}/argfile.args"
+CAPTURE_FILE="$ARGFILE_CAPTURE" JAVA_HOME="$MOCK_JAVA_HOME" \
+    JDK_JAVA_OPTIONS="@${ARGFILE}" \
+    STDOUT_MODE=true "$SERVER_SCRIPT" \
+    "${CONF}/gremlin-server.yaml" "${CONF}/rest-server.properties" true 
>/dev/null
+
+assert_effective_flag "$ARGFILE_CAPTURE" ErrorFile 
'^/operator/argfile-hs_err\.log$' \
+                      "@${ARGFILE}"
+assert_effective_flag "$ARGFILE_CAPTURE" HeapDumpPath "$HEAP_DUMP_PATTERN" 
"@${ARGFILE}"
+
+# A launch never reuses a name already taken in logs/, even within the same
+# second: with a fixed clock, the counter moves past both existing files.
+MOCK_DATE_BIN="${TEMP_DIR}/mock-date-bin"
+mkdir -p "$MOCK_DATE_BIN"
+printf '#!/bin/bash\necho 20200101-000000\n' > "${MOCK_DATE_BIN}/date"
+chmod +x "${MOCK_DATE_BIN}/date"
+mkdir -p "${SERVER_ROOT}/logs"
+CRASH_NAME_FIXTURES=("${SERVER_ROOT}/logs/java_pid1_20200101-000000.hprof"
+                     "${SERVER_ROOT}/logs/hs_err_pid1_20200101-000000-1.log")
+touch "${CRASH_NAME_FIXTURES[@]}"

Review Comment:
   🧹 Blocking: no. Summary: This test touches two fixed crash-file names under 
the caller's distribution logs/ and unconditionally removes them, including 
from the EXIT cleanup. If either file existed before the test, its crash 
artifact is deleted. Please use a temporary logs directory or track which 
fixtures this run created before removing them. Evidence: the fixed paths are 
touched here and removed in both cleanup() and the success path.



-- 
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]

Reply via email to