Skip to content

[Code Quality] ServiceManager.InstallServiceAsync — EnablePreShutdown only refreshes timeout on initial create, not on existing-service update #962

@Christophe-Rogiers

Description

@Christophe-Rogiers

Severity: Warning
File: src/Servy.Core/Services/ServiceManager.cs
Method: InstallServiceAsync (lines 278–528)

EnablePreShutdown(serviceHandle, finalTimeoutMs) is invoked only on the create branch (line 409). When CreateService fails because the service already exists, the code falls into the existing-service update branch at line 441 and calls UpdateServiceConfig + ChangeServiceConfig2 (delayed-autostart) — but it never re-applies SERVICE_CONFIG_PRESHUTDOWN_INFO.

Flow:

// line 318: try to create
serviceHandle = _windowsServiceApi.CreateService(...);
int createServiceError = _win32ErrorProvider.GetLastWin32Error();

// line 407: ONLY when create succeeded
if (serviceHandle != null && !serviceHandle.IsInvalid) {
    EnablePreShutdown(serviceHandle, finalTimeoutMs);  // <-- only path that sets it
    ...
}

// line 441: when create failed (service already exists)
if (serviceHandle == null || serviceHandle.IsInvalid) {
    if (IsServiceInstalled(options.ServiceName)) {
        UpdateServiceConfig(...);                     // updates basic config
        ChangeServiceConfig2(existingServiceHandle, ..);  // updates delayed-autostart
        await _serviceRepository.UpsertAsync(dto);    // updates DB
        // <-- EnablePreShutdown is NOT called here
        return OperationResult.Success();
    }
}

Impact: a user who installs a service with StopTimeout = 30s, then later edits StopTimeout to 300s and reinstalls, will see the new value in Servy's DB and Servy's runtime behavior — but the registry PreshutdownTimeout for the service still holds the original 30s, so when Windows actually shuts down, the SCM kills the service after the old timeout. The new one never reaches the OS until the service is uninstalled and freshly reinstalled.

Suggested fix: call EnablePreShutdown(existingServiceHandle, finalTimeoutMs) inside the update branch (around line 478), guarded the same way as the create branch and with rollback semantics matching the rest of that block.

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions