Skip to content
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

Preserve systemd provided socket after shutdown #20490

Closed
frankli0324 opened this issue Jul 26, 2022 · 9 comments · Fixed by #20499
Closed

Preserve systemd provided socket after shutdown #20490

frankli0324 opened this issue Jul 26, 2022 · 9 comments · Fixed by #20499
Labels

Comments

@frankli0324
Copy link
Contributor

frankli0324 commented Jul 26, 2022

Description

I struggle to understand why gitea unlinks the socket file on shutdown (L171):

for i, l := range providedListeners {
if isSameAddr(l.Addr(), address) {
providedListeners = append(providedListeners[:i], providedListeners[i+1:]...)
activeListeners = append(activeListeners, l)
unixListener := l.(*net.UnixListener)
unixListener.SetUnlinkOnClose(true)
return unixListener, nil
}
}

I believe this is unwanted behavior since it breaks systemctl restart gitea when using socket activation.

Gitea Version

a701fd3

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

# gitea.service
[Unit]
Description=Gitea (Git with a cup of tea)
After=syslog.target
After=network.target

Wants=postgresql.service
After=postgresql.service

After=gitea.socket
Requires=gitea.socket

[Service]
RestartSec=2s
Type=simple
User=git
Group=git
WorkingDirectory=/var/lib/gitea/
ExecStart=/usr/local/bin/gitea --config /etc/gitea/app.ini --custom-path /pool/git/custom --work-path /var/lib/gitea/ web
Restart=always
Environment=USER=git

[Install]
WantedBy=multi-user.target
# gitea.socket
[Unit]
Description=Gitea Web Socket

[Socket]
ListenStream=/run/gitea/gitea.sock
SocketMode=0666
SocketUser=git
SocketGroup=git

[Install]
WantedBy=sockets.target

Database

PostgreSQL

@zeripath
Copy link
Contributor

Have you tried using kill -1 to restart gitea?

@frankli0324
Copy link
Contributor Author

frankli0324 commented Jul 26, 2022

Have you tried using kill -1 to restart gitea?

I could, but why?
yes it indeed (maybe, haven't tried) reloads the gitea service without entirely shutting it down, but I believe that's irrelevant to what I'm saying here...

@frankli0324
Copy link
Contributor Author

frankli0324 commented Jul 26, 2022

from what I learned, SetUnlinkOnClose was designed exactly for preserving the socket file created by systemd by the golang team.

a reference from golang/go#11826 (comment):

if the UnixListener was not responsible for creating the socket file, it should not unlink the socket file on Close.

@silverwind
Copy link
Member

silverwind commented Jul 26, 2022

Isn't it normal to unlink a inactive socket, indicating it is not available? What other options are there? chmod 000?

@frankli0324
Copy link
Contributor Author

frankli0324 commented Jul 26, 2022

Set chmod 000?

Leave it alone.

https://www.freedesktop.org/software/systemd/man/systemd.socket.html
in short, systemd provides always active socket, and starts the service on connection if not started. both the creation and destruction of the socket file SHOULD be handled solely by systemd.

again,

if the UnixListener was not responsible for creating the socket file, it should not unlink the socket file on Close.

@silverwind
Copy link
Member

silverwind commented Jul 26, 2022

Interesting, yeah they mention it multiple times on that link:

A daemon listening on an AF_UNIX socket may, but does not need to, call close(2) on the received socket before exiting. However, it must not unlink the socket from a file system.

Unrelated to gitea, but normally, what I do in my apps is unlink,create and chmod 666 a socket on start, and do nothing with it on shutdown.

@frankli0324
Copy link
Contributor Author

frankli0324 commented Jul 26, 2022

further explanation on how it breaks "systemctl restart":

  • gitea.socket activates, systemd creates ipc file
  • gitea.service activates, listens on the ipc file sysd just created
  • invoke systemctl restart
  • gitea.service deactivates, deleting the ipc file
  • gitea.service activates, failing to find the appropriate ipc file to listen on

under such circumstances, users would have to first restart gitea.socket, then gitea.service. moreover, the gitea.service can't be automatically activated on incoming request, which is what systemd.socket is designed for.

@frankli0324
Copy link
Contributor Author

frankli0324 commented Jul 26, 2022

Unrelated to gitea, but normally, what I do in my apps is unlink,create and chmod 666 a socket on start, and do nothing with it on shutdown.

no, this is not about using ipc file, this is about using systemd.socket. it's two seperate(basically) things to consider.
in the code context I originally listed, google about LISTEN_FDS, see what getProvidedFDs does.

of course a socket file would ultimately be unlinked. but in this case, shouldn't be done by application. it happens when user explicitly stops gitea.socket, not gitea.service.

frankli0324 added a commit to frankli0324/gitea that referenced this issue Jul 27, 2022
@frankli0324
Copy link
Contributor Author

any thoughts?

zeripath added a commit that referenced this issue Aug 13, 2022
By default Gitea will always unlink any sockets that are provided using the `LISTEN_FDS` environment variable. This is because it uses this variable to handle passing when it is doing a graceful restart. However, this same mechanism is used by systemd - which explicitly expects that passed in sockets should not be unlinked by the receiving process. 

This PR adjusts Gitea's graceful restart mechanism to use an additional environment variable which tracks if a listening socket was opened by Gitea - and therefore should be unlinked on shutdown by Gitea.

Fix #20490

Co-authored-by: zeripath <art27@cantab.net>
vsysoev pushed a commit to IntegraSDL/gitea that referenced this issue Aug 28, 2022
By default Gitea will always unlink any sockets that are provided using the `LISTEN_FDS` environment variable. This is because it uses this variable to handle passing when it is doing a graceful restart. However, this same mechanism is used by systemd - which explicitly expects that passed in sockets should not be unlinked by the receiving process. 

This PR adjusts Gitea's graceful restart mechanism to use an additional environment variable which tracks if a listening socket was opened by Gitea - and therefore should be unlinked on shutdown by Gitea.

Fix go-gitea#20490

Co-authored-by: zeripath <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants