fix(service): start the Windows service after granting it its config (D157)
All checks were successful
PR Checks / rust-gates (pull_request) Successful in 1m8s
All checks were successful
PR Checks / rust-gates (pull_request) Successful in 1m8s
`sc start` ran inside registration, before `grant_service_access` let the new virtual account read `sidecar.toml` — which `protect_config` had locked to SYSTEM and Administrators. The first start died on "Access is denied (os error 5)" and the service sat STOPPED: the SCM's restart-on-failure policy never fires for a clean exit with an error code. A later manual start worked, because by then the grant existed. Found on the phase 18 walk (step 4, one Rust instance under the SCM), from the sidecar's own log. The code is shared, so the released ServUO installer has the same first-start failure; the org lead chose to fix both here (D157). Registration no longer starts the service; a new `start_registered` runs after the grant and refreshes the reported state. A no-op on Linux, where the service user exists before the config and registration starts the unit itself. Walked (Windows, elevated): install -> START_PENDING/running, /health on the first probe, clean stop and restart, doctor clean, uninstall clean. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E14m6SuuY6i1vASFeGDBeY
This commit is contained in:
@@ -576,8 +576,9 @@ fn install_sidecar(
|
||||
layout.relocated,
|
||||
)?;
|
||||
|
||||
let outcome = service::register(&prepared, layout, action.writes())?;
|
||||
let mut outcome = service::register(&prepared, layout, action.writes())?;
|
||||
service::grant_service_access(&config_path, &layout.data_dir, &outcome)?;
|
||||
service::start_registered(&mut outcome, &layout.sidecar_bin, &config_path)?;
|
||||
let service_record = match &outcome {
|
||||
service::Outcome::Registered {
|
||||
kind,
|
||||
|
||||
@@ -624,7 +624,7 @@ fn deploy_instance(
|
||||
prepared.user.as_deref(),
|
||||
layout.relocated,
|
||||
)?;
|
||||
let outcome = service::register_rust(
|
||||
let mut outcome = service::register_rust(
|
||||
prepared,
|
||||
layout,
|
||||
&plan.id,
|
||||
@@ -632,6 +632,7 @@ fn deploy_instance(
|
||||
binary_changed,
|
||||
)?;
|
||||
service::grant_service_access(&plan.config_path, &data_dir, &outcome)?;
|
||||
service::start_registered(&mut outcome, &layout.rust_sidecar_bin, &plan.config_path)?;
|
||||
let service_record = match &outcome {
|
||||
service::Outcome::Registered {
|
||||
kind,
|
||||
|
||||
@@ -399,22 +399,15 @@ pub fn windows_start_failure_for(
|
||||
|
||||
#[cfg(windows)]
|
||||
fn register_windows(binary: &Path, config: &Path, restart: bool) -> Result<Outcome> {
|
||||
register_windows_named(
|
||||
WINDOWS_SERVICE,
|
||||
DISPLAY_NAME,
|
||||
MIN_SERVICE_SIDECAR,
|
||||
binary,
|
||||
config,
|
||||
restart,
|
||||
)
|
||||
register_windows_named(WINDOWS_SERVICE, DISPLAY_NAME, binary, config, restart)
|
||||
}
|
||||
|
||||
/// Registers (or reconfigures), sets the restart policy of, and starts one SCM service.
|
||||
/// Registers (or reconfigures) one SCM service and sets its restart policy. It does not start it:
|
||||
/// [`start_registered`] does, after [`grant_service_access`].
|
||||
#[cfg(windows)]
|
||||
fn register_windows_named(
|
||||
name: &str,
|
||||
display: &str,
|
||||
min_sidecar: &str,
|
||||
binary: &Path,
|
||||
config: &Path,
|
||||
restart: bool,
|
||||
@@ -473,17 +466,11 @@ fn register_windows_named(
|
||||
if restart && windows_service_state(name).contains("RUNNING") {
|
||||
stop_windows_service(name)?;
|
||||
}
|
||||
// 1056 is ERROR_SERVICE_ALREADY_RUNNING, which is the desired end state, not a failure.
|
||||
let start = run("sc.exe", &["start", name])?;
|
||||
if !start.status.success() && start.status.code() != Some(1056) {
|
||||
anyhow::bail!(windows_start_failure_for(
|
||||
name,
|
||||
min_sidecar,
|
||||
start.status.code().unwrap_or(-1),
|
||||
binary,
|
||||
config
|
||||
));
|
||||
}
|
||||
// NOT started here: the virtual account `sc create` just made cannot read the config yet —
|
||||
// `protect_config` locked it to SYSTEM and Administrators, and `grant_service_access` is what
|
||||
// lets the account in. Started first, the sidecar died on `Access is denied` and the service
|
||||
// sat STOPPED; the restart policy never fires for a clean exit with an error code. The phase 18
|
||||
// walk found it (D157). The caller grants, then calls `start_registered`.
|
||||
|
||||
Ok(Outcome::Registered {
|
||||
kind: "windows-scm",
|
||||
@@ -659,7 +646,6 @@ pub fn register_rust(
|
||||
Manager::WindowsScm => register_windows_named(
|
||||
&rust_windows_service(server_id),
|
||||
&format!("Runic Gateway rust-link sidecar ({server_id})"),
|
||||
MIN_RUST_SERVICE_SIDECAR,
|
||||
binary,
|
||||
config,
|
||||
restart,
|
||||
@@ -1180,6 +1166,47 @@ pub fn protect_config(
|
||||
protect_config_platform(config, data_dir, user, relocated)
|
||||
}
|
||||
|
||||
/// Starts a service that [`register`] or [`register_rust`] has just registered, once
|
||||
/// [`grant_service_access`] has let its account read the config.
|
||||
///
|
||||
/// A no-op on Linux, where registration starts the unit itself: the service user was known before
|
||||
/// the config existed, so `protect_config` had already handed the file to it. On Windows the order
|
||||
/// is the whole point — see `register_windows_named`. The outcome's `state` is refreshed so the
|
||||
/// run reports what the service is doing now, not what it was doing before it was started.
|
||||
pub fn start_registered(outcome: &mut Outcome, binary: &Path, config: &Path) -> Result<()> {
|
||||
start_registered_platform(outcome, binary, config)
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn start_registered_platform(_outcome: &mut Outcome, _binary: &Path, _config: &Path) -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
fn start_registered_platform(outcome: &mut Outcome, binary: &Path, config: &Path) -> Result<()> {
|
||||
let Outcome::Registered { name, state, .. } = outcome else {
|
||||
return Ok(());
|
||||
};
|
||||
let min_sidecar = if name.as_str() == WINDOWS_SERVICE {
|
||||
MIN_SERVICE_SIDECAR
|
||||
} else {
|
||||
MIN_RUST_SERVICE_SIDECAR
|
||||
};
|
||||
// 1056 is ERROR_SERVICE_ALREADY_RUNNING, which is the desired end state, not a failure.
|
||||
let start = run("sc.exe", &["start", name])?;
|
||||
if !start.status.success() && start.status.code() != Some(1056) {
|
||||
anyhow::bail!(windows_start_failure_for(
|
||||
name,
|
||||
min_sidecar,
|
||||
start.status.code().unwrap_or(-1),
|
||||
binary,
|
||||
config
|
||||
));
|
||||
}
|
||||
*state = format!("{}, automatic start", windows_service_state(name));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Grants the registered service account access to what it must read and write.
|
||||
///
|
||||
/// A no-op on Linux, where the account was known before the config existed and `protect_config`
|
||||
|
||||
Reference in New Issue
Block a user