studio/install: fix mac desktop shortcut spawning and lifecycle (#5496)
* studio/install: fix mac desktop shortcut spawning and lifecycle
The macOS .app generated by install.sh ships a shell-shim wrapper that
is unsigned and has no NSAppleEventsUsageDescription in its Info.plist,
so AppleEvents from the bundle are denied by TCC. The launcher's
`osascript ... tell application "Terminal" to do script ...` call
silently fails and the script falls back to the headless nohup branch,
where the user sees no Terminal window at all. Each click of the Desktop
shortcut then leaks an unattached server (no PID file, no cleanup) and
the launcher times out after 60s without ever opening a browser.
Replace the AppleScript spawn with a `.command` file + `open -a Terminal`.
Terminal handles `.command` natively through Launch Services, no
AppleEvents permission required, works with unsigned bundles.
The new design also decouples the studio server from the Terminal:
- Server is started via nohup, detached from any TTY. Warm relaunches
(server still alive) hit the existing fast path: the launcher's
`_find_healthy_port` returns the running port and the browser opens
in ~80ms with no Terminal involvement.
- The `.command` file is a log viewer (`tail -F` of studio.log), not
the server's parent. It also runs a watcher subshell that polls the
server PID and kills `tail` when the server exits. This means
clicking "Stop server" in the UI causes the Terminal window to drop
to no-running-processes state, so the user can close the window
without the "Do you want to terminate running processes" dialog.
- A trap on HUP/INT/TERM/EXIT in the `.command` file sends SIGTERM
(then SIGKILL at +0.5s) to the server PID, so closing the Terminal
window also stops Studio. Best of both worlds: fast warm relaunch
AND "close terminal == quit Studio".
Also:
- Drop POLL_INTERVAL_SEC from 1 to 0.25. With Python studio startup
at ~2s, the 1s poll added up to 1s of slack between server-ready
and browser-open. 0.25s tightens cold-launch latency at no
meaningful CPU cost.
- Refuse to install the `.app` bundle through a symlink. If a prior
install (e.g. a --tauri build) left $HOME/Applications/Unsloth\\ Studio.app
as a symlink, mkdir -p follows it and writes the new bundle contents
through to the target. Detect and rm the symlink before mkdir -p.
Test plan:
- Existing studio-mac-update-smoke.yml CI runs install.sh end-to-end
on macos-14 and asserts /api/health returns healthy.
- Manual: click Desktop shortcut from cold state, Terminal opens with
logs streaming, browser opens at ~2s. Re-click while Studio still
running, browser opens in <200ms, no new Terminal. Click "Stop
server" in the UI, Terminal closes cleanly with no prompt. Close
Terminal via Cmd+W, server stops within 1s.
* studio/install: trim verbose comments in _spawn_terminal
* studio/install: harden trap quoting in generated .command
The trap bodies in the .command file were written with broken
quoting:
trap "rm -f "$PID_FILE" 2>/dev/null" EXIT
Shell parses this as three concatenated tokens ("rm -f " + unquoted
$PID_FILE + " 2>/dev/null") then runs the trap. With paths that
contain spaces, the unquoted expansion word-splits and the rm
either no-ops or removes the wrong path. Default $HOME has no
spaces so the bug is latent, but it should be space-safe.
Switch both trap bodies to single-quoted form so $WATCHER_PID,
$TAIL_PID, and $PID_FILE expand at signal time inside properly
quoted positions. Shellcheck-clean on the generated .command.
* studio/install: exec studio in nohup wrapper so PID is the server
Without the explicit exec, `nohup sh -c "$_cmd"` runs `_cmd` as a
child of the wrapper shell. Whether sh exec-optimizes that single
command is shell-specific (macOS /bin/sh does, dash does, some bash
configurations do not). When the optimization does not fire, `$!`
records the wrapper PID rather than the studio PID, so:
- the watcher in the generated .command monitors the wrapper, not
the actual studio process; closing the Terminal can leave studio
running if the wrapper exits first
- SIGTERM from shutdown_studio goes to the wrapper rather than the
server
Force the replacement with exec so the recorded PID is always the
studio process regardless of shell version.
Flagged by both gemini-code-assist and codex in PR review; verified
correct.
* Fix orphan-on-spawn-failure, graceful kill, and nested symlink for PR #5496
Three issues found while testing the new macOS spawn path:
1. _spawn_terminal returned 0 even when 'open -a Terminal' failed, so
the nohup'd server was left orphaned with no Terminal owner. Wrap
the .command write + chmod + open chain in 'if {...}; then return 0;
fi', and on failure SIGTERM the orphan (with a 3s grace) before
falling through to the generic terminal-spawn fallback.
2. The generated .command sent SIGKILL only 0.5s after SIGTERM, shorter
than studio/backend/run.py's _graceful_shutdown windows (5s inference
+ 5s export). Wait up to 12s for the server to exit on its own.
3. The .app symlink guard only checked the top-level path. If a prior
corrupted install left Unsloth Studio.app/Contents (or its MacOS or
Resources children) as a symlink, mkdir -p still wrote through them.
Check all four bundle paths, and refuse to continue if the bundle
path exists as a regular file.
---------
Co-authored-by: Daniel Han <info@unsloth.ai>
This commit is contained in:
parent
4c14038fe4
commit
8c335b8da6
1 changed files with 71 additions and 4 deletions
75
install.sh
75
install.sh
|
|
@ -572,7 +572,7 @@ fi
|
|||
BASE_PORT=8888
|
||||
MAX_PORT_OFFSET=20
|
||||
TIMEOUT_SEC=60
|
||||
POLL_INTERVAL_SEC=1
|
||||
POLL_INTERVAL_SEC=0.25
|
||||
LOG_FILE="$DATA_DIR/studio.log"
|
||||
# why: in env-override mode multiple installs share an OS user; namespace the
|
||||
# lock and remember our own healthy port so we never attach to an unrelated
|
||||
|
|
@ -727,9 +727,65 @@ _spawn_terminal() {
|
|||
_cmd="$1"
|
||||
_os=$(uname)
|
||||
if [ "$_os" = "Darwin" ]; then
|
||||
# Escape backslashes and double-quotes for AppleScript string
|
||||
_cmd_escaped=$(printf '%s' "$_cmd" | sed 's/\\/\\\\/g; s/"/\\"/g')
|
||||
osascript -e "tell application \"Terminal\" to do script \"$_cmd_escaped\"" >/dev/null 2>&1 && return 0
|
||||
# AppleEvents are TCC-denied from unsigned .app bundles; spawn
|
||||
# Terminal via a .command file + Launch Services instead. Server
|
||||
# is nohup'd so warm relaunches hit the fast-path; watcher + trap
|
||||
# in the .command couple Terminal close <-> server shutdown.
|
||||
# `exec` keeps the recorded PID equal to the studio process so
|
||||
# signals reach studio directly rather than a wrapper shell.
|
||||
nohup sh -c "exec $_cmd" >> "$LOG_FILE" 2>&1 &
|
||||
_server_pid=$!
|
||||
_pid_file="$DATA_DIR/studio-$_launch_port.pid"
|
||||
printf '%d\n' "$_server_pid" > "$_pid_file" 2>/dev/null || true
|
||||
|
||||
_cmd_file="$DATA_DIR/launch-terminal.command"
|
||||
_logfile_q=$(printf '%s' "$LOG_FILE" | sed "s/'/'\\\\''/g")
|
||||
_pidfile_q=$(printf '%s' "$_pid_file" | sed "s/'/'\\\\''/g")
|
||||
if {
|
||||
{
|
||||
printf '#!/bin/bash\n'
|
||||
printf "SERVER_PID=%s\n" "$_server_pid"
|
||||
printf "PID_FILE='%s'\n" "$_pidfile_q"
|
||||
# Wait up to 12s for graceful shutdown before SIGKILL.
|
||||
printf 'shutdown_studio() {\n'
|
||||
printf ' kill -TERM "$SERVER_PID" 2>/dev/null\n'
|
||||
printf ' _i=0\n'
|
||||
printf ' while kill -0 "$SERVER_PID" 2>/dev/null && [ "$_i" -lt 24 ]; do\n'
|
||||
printf ' sleep 0.5\n'
|
||||
printf ' _i=$((_i + 1))\n'
|
||||
printf ' done\n'
|
||||
printf ' kill -0 "$SERVER_PID" 2>/dev/null && kill -KILL "$SERVER_PID" 2>/dev/null\n'
|
||||
printf ' rm -f "$PID_FILE" 2>/dev/null\n'
|
||||
printf '}\n'
|
||||
printf "tail -n 100 -F '%s' &\n" "$_logfile_q"
|
||||
printf 'TAIL_PID=$!\n'
|
||||
# Server gone -> kill tail so bash exits cleanly.
|
||||
printf '(\n'
|
||||
printf ' while kill -0 "$SERVER_PID" 2>/dev/null; do sleep 1; done\n'
|
||||
printf ' kill "$TAIL_PID" 2>/dev/null\n'
|
||||
printf ') &\n'
|
||||
printf 'WATCHER_PID=$!\n'
|
||||
printf "trap 'shutdown_studio; kill \"\$WATCHER_PID\" \"\$TAIL_PID\" 2>/dev/null; exit' HUP INT TERM\n"
|
||||
printf "trap 'rm -f \"\$PID_FILE\" 2>/dev/null' EXIT\n"
|
||||
printf 'wait "$TAIL_PID" 2>/dev/null\n'
|
||||
} > "$_cmd_file" 2>/dev/null \
|
||||
&& chmod +x "$_cmd_file" 2>/dev/null \
|
||||
&& open -a Terminal "$_cmd_file" 2>/dev/null
|
||||
}; then
|
||||
# Foreground Terminal (Launch Services spawns us backgrounded).
|
||||
osascript -e 'tell application "Terminal" to activate' >/dev/null 2>&1 || true
|
||||
return 0
|
||||
fi
|
||||
# .command/open failed: kill orphan, fall through to generic fallback.
|
||||
kill -TERM "$_server_pid" 2>/dev/null || true
|
||||
_i=0
|
||||
while kill -0 "$_server_pid" 2>/dev/null && [ "$_i" -lt 6 ]; do
|
||||
sleep 0.5
|
||||
_i=$((_i + 1))
|
||||
done
|
||||
kill -0 "$_server_pid" 2>/dev/null && kill -KILL "$_server_pid" 2>/dev/null || true
|
||||
rm -f "$_pid_file" 2>/dev/null || true
|
||||
echo "[WARN] Could not open Terminal; falling back to background launch" >&2
|
||||
else
|
||||
for _term in gnome-terminal konsole xfce4-terminal mate-terminal lxterminal xterm; do
|
||||
if command -v "$_term" >/dev/null 2>&1; then
|
||||
|
|
@ -1005,6 +1061,17 @@ DESKTOP_EOF
|
|||
_css_contents="$_css_app/Contents"
|
||||
_css_macos_dir="$_css_contents/MacOS"
|
||||
_css_res_dir="$_css_contents/Resources"
|
||||
# Recreate bundle if root or any subpath is a symlink (mkdir -p follows them).
|
||||
if [ -L "$_css_app" ] || [ -L "$_css_contents" ] \
|
||||
|| [ -L "$_css_macos_dir" ] || [ -L "$_css_res_dir" ]; then
|
||||
rm -rf "$_css_app" 2>/dev/null || {
|
||||
echo "[ERROR] $_css_app contains a symlinked bundle path; remove manually and re-run install" >&2
|
||||
return 1
|
||||
}
|
||||
elif [ -e "$_css_app" ] && [ ! -d "$_css_app" ]; then
|
||||
echo "[ERROR] $_css_app exists but is not a directory; remove manually and re-run install" >&2
|
||||
return 1
|
||||
fi
|
||||
mkdir -p "$_css_macos_dir" "$_css_res_dir"
|
||||
|
||||
# Info.plist
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue