do not remove the uds socket on pause

[hommeabeil]

PR Type

Bug Fix

PR Checklist

Check your PR fulfills the following:

• Tests for the changes have been added / updated.
• Documentation comments have been added / updated.
• A changelog entry has been made for the appropriate packages.
• Format code with the latest stable rustfmt

Overview

Add a terminate function to the listener socket so the socket file is not remove in the deregister, but really when it is necessary. For a tcp socket, we have nothing to do and in the deregister, there is nothing left special to do for a UDS.

Closes #364 #97

[hommeabeil]

DO you think the CI error, might be related to this ?

[robjtede]

DO you think the CI error, might be related

No idea, usually I ignore linker errors if they're not happening on all versions. It'll go away eventually.

[fakeshadow]

You should use Drop trait.

And my question in #97 still stands. The desired behaviour is still unclear to me. Does the file need to be cleared on server shutdown?

I'm asking this because the accept thread is notified to shutdown in non blocking manner and other part of the server would race to exit the program which means the code you write may not get a chance to run at all.

This lead to two possible solution:

1. Do not remove the file at all.
2. Remove the file in "blocking" manner to make sure it's always removed.

Either is better than an inconsistent removal.

[hommeabeil]

The server should try to always clean is file when it is closing. This is what most server does and it leave the system in a cleaner state. However, we can't be ensure the socket file will be clean, because unlike a tcp socket, the kernel will not remove the file when sending a SIGKILL. So the server should handle this on startup (right now it does).

Does implementing the Drop trait will make the file remove blocking when stopping in a graceful manner (SIGTERM) ? I think this would be the desired behaviour.

[fakeshadow]

Drop trait is the idiomatic way of clean up on resource go out of scope. Drop is not guaranteed to run either it's just more clear on the purpose and it's an implicit called compared to a homebrew method. So it's more about keep the code clean and readable.

As for block and wait for accept thread to exit you can look into this part of the code:

actix-net/actix-server/src/server.rs

Line 247 in 7248131

self.waker_queue.wake(WakerInterest::Stop);

There are two possible ways to achieve a blocking cleanup:

1. Let ServerInner holds the JoinHandle of accept thread and block join when stop the server.
2. Make the WakerInterest::Stop notifier stateful where it can notify the ServerEventMultiplexer to proceed after the clean up and stop process is done.(For example using a channel)

[fakeshadow]

#443

I made a PR for solution 1 in previous reply. If you don't mind you can rebase on it when it's accepted.

If you like to implement a different way in your PR it's also fine.

[hommeabeil]

Thanks for your suggestion it is clear, I will look at your PR and look to rebase on it. I also re-think about if we should clean the file and I think their is some case where the server is not the owner of the file. One use case I have in mind is systemd socket activation, you receive a file descriptor not path, so you should let systemd manage the file.

Maybe we should clean the file only when the server receive a path, but if we receive a socket, we should not remove it since the server is not technically the owner... I will look at the different way to bind the server (I do not put too much attention the last time).

[hommeabeil]

By looking around, I think we should leave the socket here. It would be cleaner to remove it on clean shutdown, but it will not be armful to leave it here. In fact this is what systemd does and even syslog. For a client this is not a problem because if you try to connect on a close socket, even if the file exist, you receive a Connection refused. If we need more we can always try to do the cleanup later, but for now, leaving the socket file will close the issue for everyone.

[hommeabeil]

This is still affecting us, any news about this ? This prevent the use of systemd socket activation.

[robjtede]

temporarily marking as draft due to conflicts