feat!: propagate service exit code to the caller#36
feat!: propagate service exit code to the caller#36Benoît Cortier (CBenoit) wants to merge 1 commit intomasterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes propagation of service_main’s u32 return value so non-zero service exit codes aren’t silently discarded (which previously caused dispatch() to effectively always exit 0 on all supported platforms, impacting service managers like systemd).
Changes:
- Capture
service_mainreturn value indispatch()on Linux/macOS/Windows. - Exit the process with a non-zero code after platform-specific shutdown steps (macOS monitor stop, Windows STOPPED status).
- Bump crate version to
0.7.1.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/controller/windows.rs |
Capture service_main exit code and attempt to propagate it via process exit after setting STOPPED. |
src/controller/macos.rs |
Capture service_main exit code, stop the session monitor, then exit non-zero. |
src/controller/linux.rs |
Capture service_main exit code and exit non-zero. |
Cargo.toml |
Version bump to reflect behavioral change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c0fd8f8 to
3f2eb82
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | SERVICE_ACCEPT_SHUTDOWN | ||
| | SERVICE_ACCEPT_PAUSE_CONTINUE | ||
| | SERVICE_ACCEPT_SESSIONCHANGE, | ||
| dwWin32ExitCode: 0, | ||
| dwServiceSpecificExitCode: 0, | ||
| dwWin32ExitCode: win32_exit_code, | ||
| dwServiceSpecificExitCode: service_specific_exit_code, |
There was a problem hiding this comment.
dwControlsAccepted is currently set to the same flags regardless of current_state. Per the Windows service API contract, dwControlsAccepted should be 0 in SERVICE_START_PENDING/SERVICE_STOP_PENDING and SERVICE_STOPPED states. Consider setting it conditionally based on current_state to avoid reporting unsupported controls to the SCM.
ServiceMainFn<T> now returns u8, matching POSIX's 0-255 exit code range. dispatch and the Service! macro wrapper return u8 accordingly. register returns Result<ExitCode, Error> on all platforms. On Linux/macOS, the caller receives the exit code and is responsible for propagating it to the OS. On Windows, dispatch reports the exit code to the SCM via SetServiceStatus with dwWin32ExitCode = ERROR_SERVICE_SPECIFIC_ERROR and dwServiceSpecificExitCode = u32::from(exit_code).
3f2eb82 to
956430c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| match StartServiceCtrlDispatcherW(*service_table.as_ptr()) { | ||
| 0 => Err(Error::new("StartServiceCtrlDispatcher")), | ||
| _ => Ok(()), | ||
| _ => Ok(std::process::ExitCode::SUCCESS), | ||
| } |
There was a problem hiding this comment.
WindowsController::register now returns ExitCode, but it always returns ExitCode::SUCCESS on success from StartServiceCtrlDispatcherW, discarding the service's actual exit code. If the goal is to propagate the service exit code to the caller, consider capturing the exit_code produced in dispatch (e.g., via a static atomic or other shared state) and returning it once StartServiceCtrlDispatcherW unblocks; otherwise, this API should be documented as always returning SUCCESS on Windows to avoid surprising callers.
| set_service_status(ctrl_handle, SERVICE_START_PENDING, 0, 0); | ||
| set_service_status(ctrl_handle, SERVICE_RUNNING, 0, 0); | ||
| let exit_code = service_main(rx, _tx, args, false); | ||
| set_service_status(ctrl_handle, SERVICE_STOPPED, 0, u32::from(exit_code)); | ||
| } |
There was a problem hiding this comment.
The PR description says dispatch (and the Service! wrapper) return a u8 exit code, but on Windows dispatch still returns () and the Service!-generated service_main_wrapper cannot return an exit code due to the Windows service ABI. Either update the Windows API to expose the exit code to the caller (e.g., make dispatch return u8 and/or stash it for register to return), or clarify the PR description/docs that Windows is an exception and the exit code is only reported to the SCM.
| /// Signature of the service main function. | ||
| /// `rx` receives the events that are sent to the service. `tx` can be used to send custom events on the channel. | ||
| /// `args` is the list or arguments that were passed to the service. When `standalone_mode` is true, the service | ||
| /// main function is being called directly (outside of the system service support). | ||
| pub type ServiceMainFn<T> = fn( | ||
| rx: mpsc::Receiver<ServiceEvent<T>>, | ||
| tx: mpsc::Sender<ServiceEvent<T>>, | ||
| args: Vec<String>, | ||
| standalone_mode: bool, | ||
| ) -> u32; | ||
| ) -> u8; |
There was a problem hiding this comment.
Now that ServiceMainFn returns u8, the doc comment should document what the return value represents (process/service exit code) and the expected range/semantics (e.g., 0 = success, 1–255 = failure). This will also help downstream users update their service main implementations correctly after the breaking change.
ServiceMainFn now returns u8, matching POSIX's 0-255 exit code range. dispatch and the Service! macro wrapper return u8 accordingly. register returns Result<ExitCode, Error> on all platforms.
On Linux/macOS, the caller receives the exit code and is responsible for propagating it to the OS.
On Windows, dispatch reports the exit code to the SCM via SetServiceStatus with dwWin32ExitCode = ERROR_SERVICE_SPECIFIC_ERROR and dwServiceSpecificExitCode = u32::from(exit_code).