Skip to content

Commit 2e8a54b

Browse files
os-zhuangclaude
andcommitted
fix(one): stop orphaned server, double menu dialogs, and update crash-loop
Four user-reported issues with the desktop shell: 1. Orphaned server on quit/restart. `node one.mjs` is only a launcher — it spawns `objectstack serve` as a grandchild. kill_current() used Child::kill() (SIGKILL), which the launcher can't forward, so the real server was orphaned and kept holding the port + SQLite DB. Now the launcher runs as a process-group leader and shutdown sends SIGTERM to the whole group (graceful, forwarded to the server) with a SIGKILL fallback; taskkill /T on Windows. Validated: port listeners drop to 0 after shutdown. 2. "Repeated refresh" on second launch was a consequence of #1 — the orphan held standalone.db, so the new server hit a lock conflict, crashed, and the supervisor restarted it in a loop, re-navigating the window each time. Fixed by #1 (no orphan -> no conflict). 3. Two windows per menu click. A single click is delivered to both the tray's on_menu_event and the global app handler, running every action twice (two update dialogs, two Finder windows for Open Data/Reveal Logs). Added a short dedup window in handle_menu_id. 4. "Check for updates" gave no feedback and could be clicked repeatedly. Added CHECKING/INSTALLING atomic guards (concurrent clicks are no-ops), an update-checking event ("Checking for updates…"), and real download progress via update-progress; splash + injected banner render both. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 7f66da6 commit 2e8a54b

7 files changed

Lines changed: 224 additions & 16 deletions

File tree

apps/objectos-one/src-tauri/Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

apps/objectos-one/src-tauri/Cargo.toml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ dirs = "5"
2727
tokio = { version = "1", features = ["time"] }
2828
url = "2"
2929

30+
[target.'cfg(unix)'.dependencies]
31+
# Used to signal the whole sidecar process group (launcher + grandchild
32+
# `objectstack serve`) so a graceful SIGTERM reaches the real server instead
33+
# of only SIGKILLing the launcher and orphaning the server.
34+
libc = "0.2"
35+
3036
[profile.release]
3137
panic = "abort"
3238
codegen-units = 1

apps/objectos-one/src-tauri/assets/update-banner.js

Lines changed: 55 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// Injected into every page of the main webview. Lets the user see update
2-
// prompts even after navigating away from the splash page.
2+
// status, progress and prompts even after navigating away from the splash page.
33
(function () {
44
if (window.__objectosUpdateBanner) return;
55
window.__objectosUpdateBanner = true;
@@ -23,9 +23,7 @@
2323
return root;
2424
}
2525

26-
function render(info) {
27-
const root = ensureRoot();
28-
root.innerHTML = '';
26+
function cardShell() {
2927
const card = document.createElement('div');
3028
Object.assign(card.style, {
3129
background: '#141826',
@@ -35,14 +33,55 @@
3533
padding: '14px 16px',
3634
boxShadow: '0 10px 30px rgba(0,0,0,0.35)',
3735
});
36+
return card;
37+
}
38+
39+
function currentKind() {
40+
const root = document.getElementById('__objectos_update_root');
41+
const el = root && root.firstElementChild;
42+
return el ? el.getAttribute('data-kind') : null;
43+
}
44+
45+
function setStatusText(text) {
46+
const root = document.getElementById('__objectos_update_root');
47+
if (!root) return;
48+
const status = root.querySelector('[data-status]');
49+
if (status) status.textContent = text;
50+
}
51+
52+
// Lightweight, non-actionable status toast (e.g. "Checking for updates…").
53+
// Never overrides the real, actionable update card.
54+
function showStatus(text) {
55+
if (currentKind() === 'card') return;
56+
const root = ensureRoot();
57+
root.innerHTML = '';
58+
const card = cardShell();
59+
card.setAttribute('data-kind', 'status');
60+
card.innerHTML = `<div data-status style="color:#8b93a7">${text}</div>`;
61+
root.appendChild(card);
62+
}
63+
64+
function clearStatus() {
65+
if (currentKind() === 'status') {
66+
const root = document.getElementById('__objectos_update_root');
67+
if (root) root.remove();
68+
}
69+
}
70+
71+
// The actionable "update available · restart to install" card.
72+
function render(info) {
73+
const root = ensureRoot();
74+
root.innerHTML = '';
75+
const card = cardShell();
76+
card.setAttribute('data-kind', 'card');
3877
card.innerHTML = `
39-
<div style="font-weight:600;margin-bottom:4px">Update available · v${info.version}</div>
78+
<div style="font-weight:600;margin-bottom:4px">Update available · v${info.version || '?'}</div>
4079
<div style="color:#8b93a7;font-size:12px;margin-bottom:10px">
41-
You're on v${info.current}. Restart to install.
80+
You're on v${info.current || '?'}. Restart to install.
4281
</div>
4382
<div style="display:flex;gap:8px;justify-content:flex-end">
4483
<button data-act="later" style="background:transparent;color:#8b93a7;border:1px solid #2c3247;border-radius:6px;padding:5px 10px;cursor:pointer">Later</button>
45-
<button data-act="install" style="background:#6ea8ff;color:#0b0d12;border:0;border-radius:6px;padding:5px 12px;cursor:pointer;font-weight:600">Install & Restart</button>
84+
<button data-act="install" style="background:#6ea8ff;color:#0b0d12;border:0;border-radius:6px;padding:5px 12px;cursor:pointer;font-weight:600">Install &amp; Restart</button>
4685
</div>
4786
<div data-status style="color:#8b93a7;font-size:12px;margin-top:8px"></div>
4887
`;
@@ -63,11 +102,15 @@
63102
};
64103
}
65104

105+
// "Checking for updates…" feedback for the user-initiated menu check.
106+
event.listen('objectos://update-checking', (e) => {
107+
if (e.payload) showStatus('Checking for updates…');
108+
else clearStatus();
109+
});
66110
event.listen('objectos://update-available', (e) => render(e.payload || {}));
67-
event.listen('objectos://update-installing', (e) => {
68-
const root = document.getElementById('__objectos_update_root');
69-
if (!root) return;
70-
const status = root.querySelector('[data-status]');
71-
if (status) status.textContent = 'Downloading v' + e.payload + '…';
111+
event.listen('objectos://update-installing', () => setStatusText('Starting update…'));
112+
event.listen('objectos://update-progress', (e) => {
113+
const p = e.payload || {};
114+
setStatusText(p.pct != null ? `Downloading ${p.pct}%…` : 'Downloading…');
72115
});
73116
})();

apps/objectos-one/src-tauri/src/menu.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@
44
//! reads the first submenu as the "application menu", so that's where the
55
//! standard About / Settings / Quit items live.
66
7+
use std::{
8+
sync::Mutex,
9+
time::{Duration, Instant},
10+
};
11+
712
use tauri::{
813
menu::{AboutMetadataBuilder, Menu, MenuItem, PredefinedMenuItem, Submenu},
914
tray::{MouseButton, MouseButtonState, TrayIconBuilder, TrayIconEvent},
@@ -13,8 +18,30 @@ use tauri_plugin_opener::OpenerExt;
1318

1419
use crate::{logger, paths, sidecar, updater, windows};
1520

21+
/// A single menu click is delivered to BOTH the tray's own `on_menu_event` and
22+
/// the global app `on_menu_event`, so every action would otherwise run twice —
23+
/// popping two dialogs, opening two Finder windows, etc. Track the last event
24+
/// and drop a repeat of the same id within a short window.
25+
static LAST_MENU_EVENT: Mutex<Option<(String, Instant)>> = Mutex::new(None);
26+
const MENU_DEDUP_WINDOW: Duration = Duration::from_millis(400);
27+
28+
fn is_duplicate_event(id: &str) -> bool {
29+
let now = Instant::now();
30+
let mut guard = LAST_MENU_EVENT.lock().unwrap();
31+
if let Some((last_id, last_at)) = guard.as_ref() {
32+
if last_id == id && now.duration_since(*last_at) < MENU_DEDUP_WINDOW {
33+
return true;
34+
}
35+
}
36+
*guard = Some((id.to_string(), now));
37+
false
38+
}
39+
1640
/// Central dispatch for both menus.
1741
pub fn handle_menu_id(app: &AppHandle, id: &str) {
42+
if is_duplicate_event(id) {
43+
return;
44+
}
1845
match id {
1946
"open" => windows::focus_main(app),
2047
"reload" => windows::reload_main(app),

apps/objectos-one/src-tauri/src/sidecar.rs

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,54 @@ const STABLE_AFTER: Duration = Duration::from_secs(60);
5454
/// pick-a-port / bind-a-port race the Node launcher used to have.
5555
const DEFAULT_PORT: u16 = 8787;
5656

57+
/// Max time we wait for a graceful shutdown before force-killing.
58+
const SHUTDOWN_GRACE: Duration = Duration::from_millis(3000);
59+
5760
pub fn kill_current(app: &AppHandle) {
5861
let state: tauri::State<Sidecar> = app.state();
5962
let taken = state.0.lock().unwrap().take();
6063
if let Some(mut child) = taken {
61-
let _ = child.kill();
64+
let pid = child.id();
65+
terminate_tree(pid, &mut child);
66+
}
67+
}
68+
69+
/// Terminate the sidecar *process tree*.
70+
///
71+
/// The thing we spawn (`node one.mjs`) is only a launcher — it spawns the real
72+
/// `objectstack serve` as a grandchild. A plain `Child::kill()` sends SIGKILL,
73+
/// which the launcher can't catch or forward, so the server is orphaned and
74+
/// keeps holding the port and the SQLite DB. Instead we signal the whole
75+
/// process group (we spawn the launcher as a group leader, so pgid == pid):
76+
/// SIGTERM first so the launcher forwards a clean shutdown to the server and
77+
/// it releases its resources, then SIGKILL as a fallback if it doesn't exit.
78+
fn terminate_tree(pid: u32, child: &mut Child) {
79+
#[cfg(unix)]
80+
{
81+
// Negative pid targets the entire process group.
82+
unsafe { libc::kill(-(pid as i32), libc::SIGTERM) };
83+
let deadline = Instant::now() + SHUTDOWN_GRACE;
84+
loop {
85+
match child.try_wait() {
86+
Ok(Some(_)) => return, // exited cleanly
87+
Ok(None) if Instant::now() < deadline => {
88+
thread::sleep(Duration::from_millis(100));
89+
}
90+
_ => break,
91+
}
92+
}
93+
// Didn't exit in time — force-kill the whole group, then reap.
94+
unsafe { libc::kill(-(pid as i32), libc::SIGKILL) };
95+
let _ = child.wait();
96+
}
97+
#[cfg(windows)]
98+
{
99+
// No process groups here; taskkill /T terminates the whole tree.
100+
let _ = Command::new("taskkill")
101+
.args(["/PID", &pid.to_string(), "/T", "/F"])
102+
.stdout(Stdio::null())
103+
.stderr(Stdio::null())
104+
.status();
62105
let _ = child.wait();
63106
}
64107
}
@@ -180,6 +223,15 @@ fn spawn_sidecar(app: &AppHandle) -> Result<Child, String> {
180223
// full deadline on a dead port and the UI would hang on the splash.
181224
.env("OBJECTOS_MANAGED", "1");
182225

226+
// Run the launcher in its own process group (pgid == its pid) so shutdown
227+
// can signal the whole tree — launcher + the `objectstack serve`
228+
// grandchild — at once instead of orphaning the server. See terminate_tree.
229+
#[cfg(unix)]
230+
{
231+
use std::os::unix::process::CommandExt;
232+
cmd.process_group(0);
233+
}
234+
183235
let mut child = cmd.spawn().map_err(|e| format!("spawn node: {e}"))?;
184236

185237
if let Some(stdout) = child.stdout.take() {

apps/objectos-one/src-tauri/src/updater.rs

Lines changed: 77 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@
1313
//! which WebView is currently loaded in the main window.
1414
1515
use std::{
16-
sync::atomic::{AtomicBool, Ordering},
16+
sync::{
17+
atomic::{AtomicBool, AtomicU64, Ordering},
18+
Arc,
19+
},
1720
time::Duration,
1821
};
1922

@@ -27,6 +30,36 @@ use crate::{logger::log_line, sidecar};
2730

2831
pub static UPDATE_PENDING: AtomicBool = AtomicBool::new(false);
2932

33+
/// True while an update check is running. Guards against a second click (or a
34+
/// background check) starting an overlapping check — which is what let the
35+
/// "Check for updates…" item fire repeatedly with no feedback.
36+
static CHECKING: AtomicBool = AtomicBool::new(false);
37+
38+
/// True while a download+install is running. This is the guard that makes it
39+
/// impossible to launch two installs at once (the cause of multiple installer
40+
/// windows on a slow network).
41+
static INSTALLING: AtomicBool = AtomicBool::new(false);
42+
43+
/// RAII guard over an atomic flag: acquires it via compare-exchange and clears
44+
/// it on drop, so any early return / error path releases it for a later retry.
45+
struct FlagGuard(&'static AtomicBool);
46+
47+
impl FlagGuard {
48+
/// `Some(guard)` only if we flipped the flag false→true; `None` if it was
49+
/// already held (i.e. an operation is already in flight).
50+
fn acquire(flag: &'static AtomicBool) -> Option<FlagGuard> {
51+
flag.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst)
52+
.ok()
53+
.map(|_| FlagGuard(flag))
54+
}
55+
}
56+
57+
impl Drop for FlagGuard {
58+
fn drop(&mut self) {
59+
self.0.store(false, Ordering::SeqCst);
60+
}
61+
}
62+
3063
#[derive(Serialize, Clone)]
3164
pub struct UpdateInfo {
3265
pub version: String,
@@ -48,17 +81,34 @@ pub fn schedule_update_check(app: AppHandle) {
4881
/// so we always show a native dialog with the outcome. The background check
4982
/// (verbose=false) stays silent on the no-news path.
5083
pub async fn check_for_update(app: &AppHandle, verbose: bool) {
84+
// Only one check at a time. A second click (or a background check racing a
85+
// manual one) is ignored rather than stacking another network call + dialog.
86+
let Some(_check_guard) = FlagGuard::acquire(&CHECKING) else {
87+
log_line("INFO", "update check already running; ignoring duplicate request");
88+
return;
89+
};
90+
// Give the user immediate feedback that the click registered.
91+
if verbose {
92+
let _ = app.emit("objectos://update-checking", true);
93+
}
94+
5195
let updater = match app.updater() {
5296
Ok(u) => u,
5397
Err(e) => {
5498
log_line("WARN", &format!("updater unavailable: {e}"));
5599
if verbose {
100+
let _ = app.emit("objectos://update-checking", false);
56101
show_error(app, &e.to_string());
57102
}
58103
return;
59104
}
60105
};
61-
match updater.check().await {
106+
let outcome = updater.check().await;
107+
// Clear the "checking" indicator before surfacing the result.
108+
if verbose {
109+
let _ = app.emit("objectos://update-checking", false);
110+
}
111+
match outcome {
62112
Ok(Some(update)) => {
63113
UPDATE_PENDING.store(true, Ordering::SeqCst);
64114
let info = UpdateInfo {
@@ -192,15 +242,39 @@ pub async fn install_update(app: AppHandle) -> Result<(), String> {
192242
/// Shared install path used by both the IPC command (called from the
193243
/// splash UI) and the verbose-check confirm dialog.
194244
async fn run_install(app: &AppHandle) -> Result<(), String> {
245+
// Single install ever. A second invocation (double-click, or both the
246+
// splash card and the injected banner firing) is a no-op while one runs —
247+
// this is what prevents two downloads / two installer windows.
248+
let Some(_install_guard) = FlagGuard::acquire(&INSTALLING) else {
249+
log_line("INFO", "install already in progress; ignoring duplicate request");
250+
return Ok(());
251+
};
252+
195253
let updater = app.updater().map_err(|e| e.to_string())?;
196254
let update = updater
197255
.check()
198256
.await
199257
.map_err(|e| e.to_string())?
200258
.ok_or_else(|| "no update available".to_string())?;
201259
let _ = app.emit("objectos://update-installing", &update.version);
260+
261+
// Stream download progress to the UI so the user can see it's working and
262+
// doesn't click again.
263+
let app_progress = app.clone();
264+
let downloaded = Arc::new(AtomicU64::new(0));
265+
let counter = downloaded.clone();
202266
update
203-
.download_and_install(|_, _| {}, || {})
267+
.download_and_install(
268+
move |chunk, total| {
269+
let so_far = counter.fetch_add(chunk as u64, Ordering::SeqCst) + chunk as u64;
270+
let pct = total.map(|t| if t > 0 { ((so_far * 100) / t).min(100) } else { 0 });
271+
let _ = app_progress.emit(
272+
"objectos://update-progress",
273+
serde_json::json!({ "downloaded": so_far, "total": total, "pct": pct }),
274+
);
275+
},
276+
|| {},
277+
)
204278
.await
205279
.map_err(|e| e.to_string())?;
206280
// Make sure the sidecar is gone before the relaunch swaps the binary.

apps/objectos-one/src/index.html

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,11 @@
129129
updInstall.disabled = true;
130130
updLater.disabled = true;
131131
});
132+
event.listen('objectos://update-progress', (e) => {
133+
const p = e.payload || {};
134+
updStatus.textContent =
135+
p.pct != null ? 'Downloading ' + p.pct + '%…' : 'Downloading…';
136+
});
132137
updInstall.addEventListener('click', async () => {
133138
updStatus.textContent = 'Starting update…';
134139
updInstall.disabled = true;

0 commit comments

Comments
 (0)