systemd notify support#999
Open
mbr wants to merge 18 commits into
Open
Conversation
Owner
|
This looks good in principle. A few things: Create a new utility pub struct FdBarrier(pub Rc<OwnedFd>);
pub struct FdBlocker(pub Rc<OwnedFd>);
pub fn create_fd_blocker() -> Result<(FdBarrier, FdBlocker), OsError> {
todo!()
}
impl FdBarrier {
pub fn wait_blocking(&self) -> Result<(), OsError> {
todo!()
}
}FdBarrier should contain the write end of the pipe and you can detect that the read end has been closed by polling the write end with no interest. POLLHUP will be delivered. Change the API of pub struct SdNotify {
fd: Rc<OwnedFd>,
}
impl SdNotify {
pub fn open() -> Result<Option<SdNotify>, OsError> {
todo!()
}
pub fn main_pid(&self, pid: c::pid) -> Result<(), OsError> {
todo!()
}
pub fn ready(&self) -> Result<(), OsError> {
todo!()
}
}open should consume the environment variable and open a connection, if possible. |
Owner
|
For a different reason, I've added the fd_blocker util in #1002 so you don't have to write it yourself. |
Author
|
Thanks. Feel free to do whatever with this, otherwise I'll check back in on this next week, as I'm traveling without access to the machine this normally runs on this week! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I was trying to setup
jaythrough systemd, along with other services depending on it (e.g. starting Signal through a user unit). The initial approach was simple enough, just make the Jay user serviceType=notifyand send the readiness notification manually inon-graphics-ready.This comes with a catch though: Since the compositor process forks from the main (reaper) process,
NotifyAccess=allis necessary, allowing other processes to signal readiness.Things took an unexpected turn when I started PostgreSQL in a terminal -- after ending it, the
NOTIFY_SOCKETenvvar was still intact, it dutifully sentSTOPPING=1down the shared(!) socket, dropping me back to the login greeter rather abruptly.The solution would be proper systemd notify integration, here's my take on it:
NOTIFY_SOCKETis set, after spawning the compositor, we sendMAINPID=..., which signals systemd to treat the compositor as the main process. This allows us to keepNotifyAccess=main.on-graphics-initialized, it sendsREADY=1, signaling systemd that it has started successfully.NOTIFY_SOCKETfrom its environment (and carry it forward).MAINPID=..., we install a barrier pipe and wait for it to close. Slim chance, but it would lead to a stuck service otherwise and a frustrating bug to find.The only potential side-effect for systemd
Type=notifyusers is that now the compositor instead of the reaper is considered the main process.Happy for any feedback, if this is not a feature you want in jay, feel free to close it. I also wasn't sure about the AI policy of this project, so for now, this PR consists of free-range, organic hand-written bytes only, although an LLM was used to explain the codebase initially, which I believe is in spirit with the generated docs. I think clarification on slop tolerance for this project couldn't hurt :)