-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update daemonize crate #81
Comments
The goal is to get rid of the custom fork if possible, maybe pin daemonize to the |
Did the author disable the issues? The PR linked to |
Yes, @polarathene, you are right, the author disabled the issues. This is the commit that made use of the |
This turned out a bit longer than originally expected 😂 No expectation for a response, I don't have that much experience with this particular task itself so I could be misunderstanding the issue you were facing with the EDIT: If you're familiar with the whole forking/daemonization process going on and the changes between
The Whereas with
Do you actually need
|
let daemonize = Daemonize::new().exit_action(move || { | |
let mut read_buffer = [0]; | |
if let Err(e) = recv.read_exact(&mut read_buffer) { | |
info!("error reading from pipe {e}") | |
} else if read_buffer[0] == b'f' { | |
// in case of failure, 'f' is written into the pipe | |
// we explicitly exit with an error code, otherwise exit(0) is done by daemonize | |
exit(1); | |
} | |
}); | |
match daemonize.start() { |
Here you're using read_exact()
to block on receiving a single byte before continuing. And that failure condition depends upon performing daemonize.start()
to handle the returned result before returning for the error handling of the caller that would write f
?:
Lines 245 to 261 in ab7a74f
let (recv, mut init_notify) = os_pipe::pipe()?; | |
if let Err(e) = mount_background( | |
image, | |
&m.tag, | |
&mountpoint, | |
m.options, | |
manifest_verity, | |
recv, | |
&init_notify, | |
) { | |
if let Err(e) = init_notify.write_all(b"f") { | |
error!("puzzlefs will hang because we couldn't write to pipe, {e}"); | |
} | |
error!("mount_background failed: {e}"); | |
return Err(e); | |
} |
Perhaps you're a bit more familiar with what's going on in puzzlefs there, but I don't quite follow why you want to run this logic on the parent process before it exits with 0
as it's meant to. Wouldn't that belong better in privileged_action()
? (which provides a return value to start()
that you match on to handle the result?)
I have seen the related commits, but I'm probably not grokking something 😅
EDIT: I assume the intention is to avoid the parent process from exiting too soon, waiting until the child process has been successful after the daemonize.start()
call to return the status of mount()
/ error to then notify the parent via the pipe that's delaying exit until a byte arrives?
Comparing 0.4.1
to 0.5.0
and general control flow
From what I understand perform_fork()
(0.4.1
vs 0.5.0
) calls libc::fork()
which should return the child PID to the parent process if successful while the child process receives 0
, but if the fork failed you'd get -1
and need to lookup the appropriate errno code (unrelated to the -1
, it's a separate call).
In 0.4.1
the perform_fork()
method was handling each of those parent / child cases. Your exit_action
was for successful daemonization where the parent received a PID of the child, and as part of this process it's meant to exit the parent process with 0
status code. It looks like a failure scenario just returned a error for you to handle, but didn't handle this errno lookup for you to give more context.
In 0.5.0
, a check_err()
function was added, which instead performs the errno
lookup for you and returns that as the error. Now the next two conditions are handling PID for child (0 == None
) and parent (Some(pid)
).
With 0.5.0
when you call start()
it will not go directly to the perform_fork()
call, but instead match on execute()
where the parent will either be an error with errno
, or None
(successful) matching this block for the child and this block for the parent.
- Both branches run if no error due to how fork works, with the parent branch using
waitpid
via their ownwaitpid()
wrapper that blocks until the child branch completesexecute_child()
(which handles the 2ndperform_fork()
to create the grand child) which as you can see is where theexit(0)
remains, for reference the0.4.1
2ndperform_fork()
expected thisexit(0)
. - Now the parent branch unblocks from the
waitpid()
as the first child has terminated, it's done it's job and we get the exit status from that to bubble up (your rust code doesn't interact with this AFAIK, due tostd::process::exit
which terminates the parent and thus not possible to react to so the return type is!
). - The grand child continues to process the remainder of
execute_child()
, now orphaned with parent and child before it terminated. This does all the usual daemonization steps, and you can run some logic before privileges are dropped here (for reference same line in0.4.1
).
That's quite a bit to digest, but it's a bit less vague for control flow from parent to grand child?
Alternatives
Have you considered if that crate is still worthwhile for how you're leveraging it?
There's daemonize-me
that's an alternative that referenced daemonize
, it uses nix
instead of libc
calls directly, but hasn't been touched in 2 years. Still it appears to offer some hooks if you really need them (there are some closed issues and a 2.1 branch from a while ago intended to improve on the hook support, but it seems no progress is being made there).
nix::unistd::daemon(false, false)
would roughly match the defaults of daemonize
, but it's skips quite a bit of the daemonization process but you don't appear to need opt-in to any of that. For reference here is the daemon()
libc code that nix
would call (single fork()
followed by setsid()
).
// Return values if failing to fork described here:
// - https://man7.org/linux/man-pages/man3/daemon.3.html#RETURN_VALUE
//
// nix `daemon()` provides result with `errno`:
// - https://docs.rs/nix/latest/src/nix/unistd.rs.html#994-997
// - https://docs.rs/nix/latest/nix/errno/enum.Errno.html#method.result
let daemonized = nix::unistd::daemon(false, false);
The
Without the delay, the Secondly, if PuzzleFS is not able to mount the filesystem, I expect the exit code to reflect this. Without the exit_action handler, the mount command would return PS: the pipe mechanism is reused is this case because of this feature: https://github.com/project-machine/puzzlefs?tab=readme-ov-file#notification-when-the-mountpoint-is-ready |
There are some new commits in https://github.com/knsd/daemonize/commits/master that should be cherry picked into https://github.com/ariel-miculas/daemonize
The text was updated successfully, but these errors were encountered: