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

Unix socket not cleaned up on exit #14134

Closed
Daniel15 opened this issue Sep 19, 2019 · 27 comments
Closed

Unix socket not cleaned up on exit #14134

Daniel15 opened this issue Sep 19, 2019 · 27 comments
Assignees
Labels
affected-very-few This issue impacts very few customers area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions Docs This issue tracks updating documentation enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-kestrel severity-minor This label is used by an internal tool
Milestone

Comments

@Daniel15
Copy link
Contributor

Daniel15 commented Sep 19, 2019

Describe the bug

I hit a similar issue in 2015 (aspnet/KestrelHttpServer#419) and I think it's still lingering today. When I shutdown the systemd service for an ASP.NET site, it won't restart again as the UNIX socket is not deleted on shutdown. I can work around the issue by deleting the socket in Program.cs, but Kestrel should be doing this itself on shutdown.

To Reproduce

Steps to reproduce the behavior:

[Unit]
Description=Test ASP.NET Core Site

[Service]
WorkingDirectory=/var/www/test-site/
ExecStart=/var/www/test-site/Daniel15.Test
Restart=always
# Restart service after 10 seconds if the dotnet service crashes:
RestartSec=10
KillSignal=SIGINT
SyslogIdentifier=test-site
User=www-data
Environment=ASPNETCORE_ENVIRONMENT=Production
Environment=ASPNETCORE_URLS=http://unix:/tmp/test-site.sock

[Install]
WantedBy=multi-user.target
  • Enable and start the unit:
sudo service test-site enable
sudo service test-site start

It works fine.

  • Try to restart it:
sudo service test-site restart

It fails to start:

Sep 18 23:48:34 syd02.d.sb test-site[19486]: System.IO.IOException: Failed to bind to address http://unix:/tmp/test-site.sock: a
Sep 18 23:48:34 syd02.d.sb test-site[19486]:  ---> Microsoft.AspNetCore.Connections.AddressInUseException: Address already in use
Sep 18 23:48:34 syd02.d.sb test-site[19486]:  ---> System.Net.Sockets.SocketException (98): Address already in use
Sep 18 23:48:34 syd02.d.sb test-site[19486]:    at System.Net.Sockets.Socket.UpdateStatusAfterSocketErrorAndThrowException(SocketErr
Sep 18 23:48:34 syd02.d.sb test-site[19486]:    at System.Net.Sockets.Socket.DoBind(EndPoint endPointSnapshot, SocketAddress socketA
Sep 18 23:48:34 syd02.d.sb test-site[19486]:    at System.Net.Sockets.Socket.Bind(EndPoint localEP)
Sep 18 23:48:34 syd02.d.sb test-site[19486]:    at Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets.SocketConnectionListener.Bi
Sep 18 23:48:34 syd02.d.sb test-site[19486]:    --- End of inner exception stack trace ---

Expected behavior

Should restart cleanly

Additional context

dotnet --info on my computer:

λ dotnet --info
.NET Core SDK (reflecting any global.json):
 Version:   3.0.100-preview9-014004
 Commit:    8e7ef240a5

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.18362
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\3.0.100-preview9-014004\

Host (useful for support):
  Version: 3.0.0-preview9-19423-09
  Commit:  2be172345a

.NET Core SDKs installed:
  1.0.0-preview2-003121 [C:\Program Files\dotnet\sdk]
  1.0.0-preview2-1-003177 [C:\Program Files\dotnet\sdk]
  1.0.0-rc4-004771 [C:\Program Files\dotnet\sdk]
  1.0.0 [C:\Program Files\dotnet\sdk]
  1.0.4 [C:\Program Files\dotnet\sdk]
  1.1.0 [C:\Program Files\dotnet\sdk]
  1.1.9 [C:\Program Files\dotnet\sdk]
  1.1.11 [C:\Program Files\dotnet\sdk]
  1.1.12 [C:\Program Files\dotnet\sdk]
  2.0.0 [C:\Program Files\dotnet\sdk]
  2.0.2 [C:\Program Files\dotnet\sdk]
  2.1.201 [C:\Program Files\dotnet\sdk]
  2.1.202 [C:\Program Files\dotnet\sdk]
  2.1.503 [C:\Program Files\dotnet\sdk]
  2.1.504 [C:\Program Files\dotnet\sdk]
  2.1.602 [C:\Program Files\dotnet\sdk]
  2.2.103 [C:\Program Files\dotnet\sdk]
  2.2.202 [C:\Program Files\dotnet\sdk]
  3.0.100-preview9-014004 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.0.0-preview9.19424.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 1.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 1.0.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 1.0.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 1.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 1.0.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 1.0.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 1.0.14 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 1.1.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 1.1.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 1.1.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 1.1.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 1.1.10 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 1.1.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.0.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.0.0-preview9-19423-09 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.0.0-preview9-19423-09 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET Core runtimes or SDKs:
  https://aka.ms/dotnet-download

On the server, it's using a self-contained deployment so I don't think there's a dotnet ---info command I can run, but it's using 3.0.0-preview9-19423-09

@davidfowl
Copy link
Member

We hit the same thing in our tests. It does feel strange that we don't explicitly create it yet we're responsible for deleting it. We do the same in our tests 😄 https://github.com/aspnet/AspNetCore/blob/master/src/Servers/Kestrel/test/FunctionalTests/UnixDomainSocketsTests.cs#L113 (maybe that should have been a sign).

This seems like something we should fix in 3.1 cc @anurse

@Daniel15
Copy link
Contributor Author

Is it possible to mention this issue in the readme of ASP.NET Core 3.0, or perhaps just in the Kestrel docs? Anyone using it with UNIX sockets will hit this issue.

@davidfowl davidfowl added the Docs This issue tracks updating documentation label Sep 23, 2019
@analogrelay analogrelay added this to the 5.0.0-preview1 milestone Sep 24, 2019
@analogrelay
Copy link
Contributor

We should, at least, provide an option to control deletion behavior. If we're worried about the breaking change of always deleting, we can make the option off by default.

@Daniel15
Copy link
Contributor Author

This is how I'm working around it for now:

		public static void Main(string[] args)
		{
			var host = CreateWebHostBuilder(args).Build();
			DeleteOldSocketIfExists(host);
			host.Run();
		}

		private static void DeleteOldSocketIfExists(IWebHost host)
		{
			// Delete UNIX pipe if it exists at startup (eg. previous process crashed before cleaning it up)
			// Workaround for https://github.com/aspnet/AspNetCore/issues/14134
			var addressFeature = host.ServerFeatures.Get<IServerAddressesFeature>();
			var url = BindingAddress.Parse(addressFeature.Addresses.First());
			if (url.IsUnixPipe && File.Exists(url.UnixPipePath))
			{
				Console.WriteLine("UNIX pipe {0} already existed, deleting it.", url.UnixPipePath);
				File.Delete(url.UnixPipePath);
			}
		}

@Yuki-Nagato
Copy link

While @Daniel15 provided a workaround, it may not help if using Generic Host in ASP.NET Core 3.0.

@sherlock1982
Copy link

BTW I advice to cleanup on startup rather than on shutdown because your app might crash and than socket will remain there anyway. This can be done in systemd hooks.

@Daniel15
Copy link
Contributor Author

This can be done in systemd hooks.

Do you have an example of this?

@aramean
Copy link

aramean commented Mar 24, 2020

You could cleanup the socket like this:

[Service]
ExecStartPre=/usr/bin/rm -f /tmp/test-site.sock
ExecStart=/var/www/test-site/Daniel15.Test

@shirhatti
Copy link
Contributor

Triage decision: The workaround is good enough, and we won't be making a product change here.

@Daniel15
Copy link
Contributor Author

@shirhatti That's unfortunate because this is very clearly a bug. An app should clean up its allocated resources when they're no longer required. This would be similar to not fixing a memory leak because a workaround exists.

@davidfowl
Copy link
Member

Yea I agree this shouldn’t be closed. @Daniel15 can you submit a PR here?

@shirhatti shirhatti reopened this Mar 30, 2020
@analogrelay analogrelay added this to the Next sprint planning milestone Mar 30, 2020
@Daniel15
Copy link
Contributor Author

Daniel15 commented May 4, 2020

@davidfowl My free time is fairly limited these days but I can try to submit a PR if nobody else gets around to that.

@sherlock1982
Copy link

sherlock1982 commented May 4, 2020

EDITED:
Note: you will still need to clean it up in case your APP crashes. Or find another way to handle it from the APP itself.

@davidfowl
Copy link
Member

@sherlock1982 maybe, I'm wondering if we can create a delete on close file to handle to handle that scenario.

@BrennanConroy
Copy link
Member

Triage: We're trying to see how many folks are being hit by this issue.
If Kestrel deletes the socket, would anyone be broken by that?

@davidfowl
Copy link
Member

I wonder if this should be a BCL issue. I have some data about how this should be done and maybe it should be possible open an existing file

@davidfowl
Copy link
Member

https://troydhanson.github.io/network/Unix_domain_sockets.html

Note that, once created, this socket file will continue to exist, even after the server exits. If the server subsequently restarts, the file prevents re-binding:
% ./srv
% ./srv
bind error: Address already in use
So, servers should unlink the socket pathname prior to binding it.

@davidfowl
Copy link
Member

Then this interaction with systemd interesting:

coreos/go-systemd#71

@Daniel15
Copy link
Contributor Author

Daniel15 commented May 11, 2020

@davidfowl That page you linked (https://troydhanson.github.io/network/Unix_domain_sockets.html) is interesting. Sounds like the old socket should be cleaned up on start, not on exit. That'd also handle the case where the app doesn't exit cleanly (eg if you kill -9 it or it crashes) as @sherlock1982 mentioned in a comment.

Might be worth seeing what other frameworks do?

@BrennanConroy
Copy link
Member

@tmds Do you have experience/opinions on whether the server should cleanup the socket that's passed in?

@BrennanConroy BrennanConroy added affected-very-few This issue impacts very few customers enhancement This issue represents an ask for new feature or an enhancement to an existing one severity-minor This label is used by an internal tool labels Oct 26, 2020 — with ASP.NET Core Issue Ranking
@J5cott
Copy link

J5cott commented Dec 1, 2020

Thanks for the workaround @Daniel15!

But this may not quite work for others reading. I needed to make one little tweak.

This is how I'm working around it for now:

		public static void Main(string[] args)
		{
			var host = CreateWebHostBuilder(args).Build();
			DeleteOldSocketIfExists(host);
			host.Run();
		}

		private static void DeleteOldSocketIfExists(IWebHost host)
		{
			// Delete UNIX pipe if it exists at startup (eg. previous process crashed before cleaning it up)
			// Workaround for https://github.com/aspnet/AspNetCore/issues/14134
			var addressFeature = host.ServerFeatures.Get<IServerAddressesFeature>();
			var url = BindingAddress.Parse(addressFeature.Addresses.First());
			if (url.IsUnixPipe && File.Exists(url.UnixPipePath))
			{
				Console.WriteLine("UNIX pipe {0} already existed, deleting it.", url.UnixPipePath);
				File.Delete(url.UnixPipePath);
			}
		}

Grabbing the path to the socket from addressFeature.Addresses.First() was throwing a System.InvalidOperationException the sequence contained no elements exception. This was happening when trying to configure the unix socket path with KestrelServerOptions.ListenUnixSocket("/tmp/kestrel.sock") I ended up passing in a hardcoded string which worked....obviously.... Not sure why it wasn't grabbing the path that I configured.

    public class Program
    {
        private const string ServerSocket = "/tmp/kestrel.sock";
        public static void Main(string[] args)
        {
            var host = CreateHostBuilder(args).Build();
            DeleteOldSocketIfExists(host);
            host.Run();
        }

        private static void DeleteOldSocketIfExists(IWebHost host)
		{
			if (File.Exists(ServerSocket))
			{
				Console.WriteLine("UNIX pipe {0} already existed, deleting it.", ServerSocket);
				File.Delete(ServerSocket);
			}
		}

        public static IWebHostBuilder CreateHostBuilder(string[] args) =>
            WebHost.CreateDefaultBuilder(args)
            .UseKestrel(options => {options.ListenUnixSocket(ServerSocket);})
            .UseStartup<Startup>();
    }```

@tmds
Copy link
Member

tmds commented Dec 1, 2020

@tmds Do you have experience/opinions on whether the server should cleanup the socket that's passed in?

It is appropriate to delete the file on shutdown.

When you delete on start, the benefit is the app can restart when it crashed.
The potential drawback is you can delete a socket that is used by another application, effectively stealing that address.

@tmds
Copy link
Member

tmds commented Dec 3, 2020

It is appropriate to delete the file on shutdown.

I wrote an issue proposing the Socket class should delete this file on Dispose: dotnet/runtime#45537.

@Stoyan-Bukovich
Copy link

Another issue that I hit is that the socket permissions are always -rwxr-x-r-x and owned by the user and group under which Kestrel runs upon socket creation. That's an issue when hosting multiple web apps running under different user accounts, and having the reverse proxy server running under totally different account and trying to read each web app socket. See how messy it gets setting the correct permissions on each app socket, when restarting any of the web apps?!

@Stoyan-Bukovich
Copy link

@tmds Do you have experience/opinions on whether the server should cleanup the socket that's passed in?

It is appropriate to delete the file on shutdown.

When you delete on start, the benefit is the app can restart when it crashed.
The potential drawback is you can delete a socket that is used by another application, effectively stealing that address.

How about when you create the socket just giving unique name (Example: /var/run/websock/mysitedomain.sock)? That's how you can avoid deleting another apps socket by mistake.

@tmds
Copy link
Member

tmds commented May 31, 2021

.NET 6 will remove the unix socket file when the Socket gets disposed: dotnet/runtime#52103.

@davidfowl
Copy link
Member

Sweet, we can close this! Thanks @tmds !

@ghost ghost locked as resolved and limited conversation to collaborators Jun 30, 2021
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affected-very-few This issue impacts very few customers area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions Docs This issue tracks updating documentation enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-kestrel severity-minor This label is used by an internal tool
Projects
None yet
Development

No branches or pull requests