Simplify code about pid file creation #299
Comments
fluca1978
added a commit
to fluca1978/pgagroal
that referenced
this issue
Aug 11, 2022
In `main` the pidfile is created, then it could happen that the application brutally exits because of a runtime error, like not being able to open a socket. In such cases, the pid file has to be removed. This commit introduces an `error` path that does remove the pidfile and exits with an error code. Invoking `remove_pidfile` is safe even when the file does not exists, and besides the pid file cannot be changed at runtime via a configuration reload. Moreove, it is not worth checking if the pidfile exists on disk before deleting it via `unlink(2)`, since the function will run smoothly if the file is not there. See agroal#299 Close agroal#298
fluca1978
added a commit
to fluca1978/pgagroal
that referenced
this issue
Aug 11, 2022
…tion Since `open(2)` is invoked with flags `O_CREAT | O_EXCL`, the function will fail if the file is already there. This means there is no need for `access(2)` to be invoked before the `open(2)` call. This commit catches errors related to `open` not being able to create the pid file because another one is there, or because the user does not have permissions. Other errors are managed in a generic way as it was before. See agroal#298 Close agroal#299.
fluca1978
added a commit
to fluca1978/pgagroal
that referenced
this issue
Aug 11, 2022
In `main` the pidfile is created, then it could happen that the application brutally exits because of a runtime error, like not being able to open a socket. In such cases, the pid file has to be removed. This commit introduces an `error` path that does remove the pidfile and exits with an error code. Invoking `remove_pidfile` is safe even when the file does not exists, and besides the pid file cannot be changed at runtime via a configuration reload. Moreover, it is not worth checking if the pidfile exists on disk before deleting it via `unlink(2)`, since the function will run smoothly if the file is not there. [agroal#298] Remove pidfile when reloading configuration fails. Change the permissions on the pid file: 0640. Since the configuration reload is issued within the main process, the process must remove the pidfile (if any) before existing in case of failure. This commit also removes `access(2)` call before `open(2)` in pidfile creation Since `open(2)` is invoked with flags `O_CREAT | O_EXCL`, the function will fail if the file is already there. This means there is no need for `access(2)` to be invoked before the `open(2)` call. This commit catches errors related to `open` not being able to create the pid file because another one is there, or because the user does not have permissions. Other errors are managed in a generic way as it was before. Close agroal#298 Close agroal#299
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
While working on #298 and re-reading the documentation of
open(2), I found thatopen(2)fails whenO_CREAT | O_EXCLare specified as flags if the file is already there.Therefore it is possible to remove the
access(2)check at https://github.com/agroal/pgagroal/blob/master/src/main.c#L1958 and catch specific errors to warn the user.The text was updated successfully, but these errors were encountered: