Skip to content

Commit 207d980

Browse files
akubiusaclaude
akubiusa
and
claude
authored
fix: update-*.jar の蓄積とコンテナ再起動ループを根本解決 (#464)
* fix: update.jar を固定ファイル名に変更し連鎖アップデートに対応 - Reload.kt: update-<timestamp>.jar → update.jar（固定名）に変更 タイムスタンプ付きファイル名が蓄積の根本原因であるため、 固定ファイル名 + overwrite=true により蓄積を構造的に排除する - docker-entrypoint.sh: 以下の改善を実施 - pgrep パターンを update-.*\.jar → update\.jar に更新 - is_zombie_pid / find_update_pid 関数を追加し ゾンビプロセスや無効プロセスを除外する - 連鎖アップデート対応の外側ループを追加 （update.jar が次の update.jar を起動してから終了する場合に追跡継続） Closes #463 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: コードレビュー指摘事項を自動修正 - docker-entrypoint.sh: ステップ 6 のコメントを英語から日本語に修正 - Reload.kt: updateTo() の docstring を更新し、固定ファイル名 ./update.jar への上書きコピーに関する説明を追記 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: Copilot レビューコメントへの対応 - docker-entrypoint.sh: wait で子プロセスの reap を試みた後、失敗時はポーリングに フォールバックし、ゾンビ化した場合はループを抜けるよう改善 - Reload.kt: update.jar 固定名上書きが Docker（Linux）環境で安全な理由をコメントに明記 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: update jar のファイル名をバージョン名ベースに変更 update.jar（固定名）から update-<version>.jar（バージョン名）に変更する。 同一バージョンでは同じファイル名になるため実質的に蓄積が抑えられ、 連鎖アップデート時には異なるバージョン間でファイル名が一致しないため 実行中ファイルを上書きしないという利点もある。 - Reload.kt: updateTo() でバージョンを manifest から読み取り update-<version>.jar 形式でコピー。バージョン読み取り失敗時は タイムスタンプをフォールバックとして使用する。 コピー前に古い update-*.jar を削除し蓄積を防ぐ。 - docker-entrypoint.sh: pgrep パターンを update-.*\.jar に更新 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: コメントを英語に統一 既存コードに合わせてコメントをすべて英語に変更する Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: バージョン文字列のサニタイズとコピー順序の改善 - バージョン文字列を英数字・ドット・ハイフン以外の文字を _ に置換してサニタイズ。 悪意ある jar manifest に ../../ 等が含まれていた場合のパストラバーサルを防止する。 - update-*.jar のクリーンアップ順序をコピー後に変更。 以前はコピー前に削除していたため、コピー失敗時に古い jar も失われる状態だった。 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: チェーン更新のプロセス検出ウィンドウを 5 秒に統一 連鎖アップデート時の max_attempts を 3 から 5 に変更し、初回検出と同じ 待機時間に揃える。高負荷環境での JVM 起動時間を考慮したマージンを確保する。 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: Path正規化によるパストラバーサル防止に変更 正規表現によるバージョン文字列サニタイズの代わりに、 Java NIO の Path.normalize() と startsWith() を使用して マニフェストファイル経由のパストラバーサルを防止する。 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: akubiusa <222856114+akubiusa@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 145cd72 commit 207d980

2 files changed

Lines changed: 78 additions & 55 deletions

File tree

‎docker-entrypoint.sh‎

Lines changed: 45 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -4,46 +4,49 @@
44
#
55
# Update flow:
66
# 1. Current (vcspeaker.jar) starts and runs normally
7-
# 2. When update is detected, Current spawns Latest (update-*.jar)
7+
# 2. When update is detected, Current spawns Latest (update-<version>.jar) via ProcessBuilder
88
# 3. Current and Latest communicate via API to transfer state
99
# 4. Current exits with code 0
1010
# 5. This script waits for Latest to finish (or keeps running)
11+
# 6. If Latest itself updates, the script tracks the new process (chain updates)
1112

13+
# Returns 0 if the given PID is a zombie process, 1 otherwise
1214
is_zombie_pid() {
1315
local pid=$1
1416

15-
# プロセスが存在しない場合は zombie ではない
17+
# If /proc/<pid>/stat is not readable, the process is not a zombie
1618
if [[ ! -r "/proc/$pid/stat" ]]; then
1719
return 1
1820
fi
1921

2022
local state
2123
state=$(awk '{print $3}' "/proc/$pid/stat" 2>/dev/null || true)
2224

23-
# state が取得できなかった場合も zombie ではない
25+
# If state cannot be read, treat as not a zombie
2426
if [[ -z "$state" ]]; then
2527
return 1
2628
fi
2729

2830
[[ "$state" == "Z" ]]
2931
}
3032

31-
find_update_pids() {
32-
update_pids=()
33+
# Returns the PID of the update process (update-<version>.jar), excluding zombies and invalid processes
34+
find_update_pid() {
3335
local pid
34-
3536
while IFS= read -r pid; do
3637
if [[ -z "$pid" ]]; then
3738
continue
3839
fi
3940

40-
# zombie プロセスはスキップ (is_zombie_pid 内でプロセスの存在確認も行われる)
41+
# Skip zombie processes
4142
if is_zombie_pid "$pid"; then
4243
continue
4344
fi
4445

45-
update_pids+=("$pid")
46-
done < <(pgrep -f "update-.*\.jar" || true)
46+
echo "$pid"
47+
return 0
48+
done < <(pgrep -f "update-.*\.jar" 2>/dev/null || true)
49+
return 1
4750
}
4851

4952
java -jar /app/vcspeaker.jar "$@"
@@ -52,57 +55,49 @@ exit_code=$?
5255
echo "VCSpeaker exited with code: $exit_code"
5356

5457
if [[ $exit_code -eq 0 ]]; then
55-
# Retry loop: wait up to 5 seconds for update process to appear
56-
update_pids=()
57-
for _ in {1..5}; do
58-
find_update_pids
59-
if [[ ${#update_pids[@]} -gt 0 ]]; then
58+
# Loop to track chain updates:
59+
# handles the case where update-<v1>.jar spawns update-<v2>.jar before exiting
60+
chain_update=0
61+
while true; do
62+
# Wait up to 5 seconds for an update process to appear (accounts for JVM startup time)
63+
update_pid=""
64+
for _ in $(seq 1 5); do
65+
update_pid=$(find_update_pid || true)
66+
if [[ -n "$update_pid" ]]; then
67+
break
68+
fi
69+
sleep 1
70+
done
71+
72+
if [[ -z "$update_pid" ]]; then
73+
if [[ $chain_update -eq 1 ]]; then
74+
echo "No further update process found. Normal shutdown."
75+
else
76+
echo "No update process found. Normal shutdown."
77+
fi
6078
break
6179
fi
62-
sleep 1
63-
done
64-
65-
if [[ ${#update_pids[@]} -gt 0 ]]; then
66-
echo "Update process detected (PID: ${update_pids[*]}). Waiting for it to complete..."
6780

68-
update_wait_timeout_seconds=${UPDATE_WAIT_TIMEOUT_SECONDS:-300}
69-
update_wait_start_epoch=$(date +%s)
70-
update_wait_timed_out=0
71-
72-
if [[ ! "$update_wait_timeout_seconds" =~ ^[0-9]+$ ]]; then
73-
update_wait_timeout_seconds=300
74-
fi
81+
echo "Update process detected (PID: $update_pid). Waiting for it to complete..."
7582

7683
# Wait for the update process to finish
7784
# This keeps the entrypoint alive so Docker doesn't restart the container
78-
while true; do
79-
find_update_pids
80-
81-
if [[ ${#update_pids[@]} -eq 0 ]]; then
82-
break
83-
fi
84-
85-
if [[ "$update_wait_timeout_seconds" -gt 0 ]]; then
86-
now_epoch=$(date +%s)
87-
elapsed_seconds=$((now_epoch - update_wait_start_epoch))
88-
if [[ $elapsed_seconds -ge $update_wait_timeout_seconds ]]; then
89-
update_wait_timed_out=1
85+
# Try wait first to properly reap child processes
86+
if ! wait "$update_pid" 2>/dev/null; then
87+
# Fall back to polling if wait fails (e.g. process is not a child)
88+
while kill -0 "$update_pid" 2>/dev/null; do
89+
# Break out of the loop if the process has become a zombie
90+
if is_zombie_pid "$update_pid"; then
91+
wait "$update_pid" 2>/dev/null || true
9092
break
9193
fi
92-
fi
93-
94-
sleep 1
95-
done
96-
97-
if [[ $update_wait_timed_out -eq 1 ]]; then
98-
echo "Update process wait timed out after ${elapsed_seconds}s (PID: ${update_pids[*]})."
99-
exit 1
94+
sleep 1
95+
done
10096
fi
10197

102-
echo "Update process finished."
103-
else
104-
echo "No update process found. Normal shutdown."
105-
fi
98+
echo "Update process (PID: $update_pid) finished. Checking for chain updates..."
99+
chain_update=1
100+
done
106101

107102
exit 0
108103
else

‎src/main/kotlin/com/jaoafa/vcspeaker/reload/Reload.kt‎

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -202,16 +202,44 @@ object Reload {
202202
}
203203

204204
/**
205-
* 指定された jar archive を使用して VCSpeaker を更新します。
206-
* 更新後は新しいプロセスが起動し、現在のプロセスは終了します。
205+
* Updates VCSpeaker using the given jar archive.
206+
* The jar is copied to the current working directory as `update-<version>.jar`.
207+
* Falls back to a timestamp-based name if the version cannot be read from the manifest.
208+
* Cleans up old `update-*.jar` files before copying to prevent accumulation.
209+
* After the update, a new process is launched and the current process exits.
207210
*
208-
* @param jar 更新先の jar archive
211+
* @param jar jar archive to update to
209212
*/
210213
fun updateTo(jar: File) {
211214
logger.info { "Updating to ${jar.name}..." }
212215

213-
// copy to the current working directory
214-
val updateJar = jar.copyTo(File("./update-${System.currentTimeMillis()}.jar"), overwrite = true)
216+
// Copy to a version-based filename so that different versions get different filenames,
217+
// avoiding overwriting a running update jar during chain updates.
218+
// Falls back to a timestamp if the version cannot be read from the manifest.
219+
val rawVersion = jar.jarVersion() ?: System.currentTimeMillis().toString()
220+
221+
// Use Path normalization to prevent path traversal via a malicious jar manifest.
222+
// Resolve the candidate path and verify it stays within the current directory.
223+
val baseDir = File(".").toPath().toAbsolutePath().normalize()
224+
val candidatePath = baseDir.resolve("update-$rawVersion.jar").normalize()
225+
val updateJarName = if (candidatePath.startsWith(baseDir) && candidatePath.parent == baseDir) {
226+
"update-$rawVersion.jar"
227+
} else {
228+
logger.warn { "Unsafe version string in manifest: '$rawVersion'. Falling back to timestamp-based name." }
229+
"update-${System.currentTimeMillis()}.jar"
230+
}
231+
232+
// Copy first, then clean up old update-*.jar files.
233+
// Copying before deletion ensures no downtime window if the copy fails.
234+
val updateJar = jar.copyTo(File("./$updateJarName"), overwrite = true)
235+
236+
// Delete old update-*.jar files (log a warning on failure, but continue)
237+
File(".").listFiles { f -> f.name.matches(Regex("update-.*\\.jar")) && f.name != updateJarName }
238+
?.forEach { old ->
239+
if (!old.delete()) {
240+
logger.warn { "Failed to delete old update jar: ${old.absolutePath}" }
241+
}
242+
}
215243

216244
val server = Server(ServerType.Current)
217245
VCSpeaker.apiServer?.stop()

0 commit comments

Comments
 (0)