coreos/go-systemd

activation: allow multiple calls to `activation.Files(true)`

oliverpool opened this issue · 2 comments

Currently when calling activation.Files(true) the environment variables get unset

func Files(unsetEnv bool) []*os.File {
if unsetEnv {
defer os.Unsetenv("LISTEN_PID")
defer os.Unsetenv("LISTEN_FDS")
defer os.Unsetenv("LISTEN_FDNAMES")
}

This means that calling activation.Files(true) will return an empty listener slice.

I think it would be nice to allow for multiple calls to succeed.

Use Case

In my case I have multiple services and each check if there named-socket is present, to start listening. Since activation.Listeners calls activation.Files(true) this works only for the first caller.

Workaround

Make one call to activation.Listeners and pass this around.

Suggestion

Add a sync.Once in activation.Files to be able to reuse the listeners.
I think the unsetEnv bool parameter could be dropped and replace with a method RestoreEnvironment which would restore the LISTEN_* env variables when called.

I would be willing to craft a PR, if you think that this is a valid usecase and that this solution looks acceptable.

(as-is, it would be a breaking change)

Edit: this problem is actually quite complex, since all the Listeners function Close those files

lucab commented

Hi and welcome!
I'm having some troubles understanding what you are suggesting. The problem you mention ("not unsetting env") should be covered by activation.Files(false). However as the documentation says you almost never want to do that, as managing the lifecycles of FDs that way is very prone to bugs.

In my case I have multiple services and each check if there named-socket is present, to start listening. Since activation.Listeners calls activation.Files(true) this works only for the first caller.

If this is the whole scenario, then you should consider re-arranging the logic so that there is a single activation.Files(true) call. And then dispatching the file objects as needed. The listeners logic covers most of the simple cases, but it isn't well suited for more complex cases like yours.

You are right, trying to allow activation.Files to be called multiple time seems quite hard and not worth the trouble.

I will refactor my logic, to call activation.Files only once!