Container ops fork child processes that inherit file descriptors about go-lxc HOT 6 CLOSED

Hey @dijkstracula,

Thank you so much for the super detailed report and also for the sample code to reproduce it!

I'll try to take a look at it but unfortunately I'm crazy busy nowadays :/

Not sure whether double fork would be a solution, need to try it :) @tych0 do you have any suggestions?

from go-lxc.

You can set the container's daemonize flag via container.WantDaemonize(true), which should invoke liblxc's code to close these fds. It also implies a few other things about forking that you might or might not want, I don't remember exactly the specifics.

We could also replicate lxc_check_inherited in golang, but it might make more sense to somehow expose it in a nicer way to liblxc. But maybe just setting daemonize will be enough, so I'd suggest you try that first :)

from go-lxc.

Thank you, @tych0! It appears that in the repro case setting WantDaemonize(true) seems to avoid the race. Today I'll backport the "fix" in the service at work (which was, unfortunately, just to shell out to the LXC CLI :( ) and see if that also solves our issue.

Do you reckon that this ought to be the default setting for go-lxc containers? Is there a reason why application developers would not want this flag enabled by default?

from go-lxc.

On Mon, Sep 26, 2016 at 08:46:16AM -0700, Nathan Taylor wrote:

Thank you, @tych0! It appears that in the repro case setting WantDaemonize(true) seems to avoid the race. Today I'll backport the "fix" in the service at work (which was, unfortunately, just to shell out to the LXC CLI :( ) and see if that also solves our issue.

Do you reckon that this ought to be the default setting for go-lxc containers? Is there a reason why application developers would not want this flag enabled by default?

Possibly, although someone should look at what the repercussions are
for setting it by default. It implies a bunch of things, and I don't
remember exactly what they are :).

Probably it shouldn't be go-lxc making this decision to change the
default, but liblxc itself, since it is the one providing the C api.

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#66 (comment)

from go-lxc.

@tych0 @dijkstracula ah sorry for not mentioning that, for some reason I assumed you already tried WantDaemonize. You should also be able set WantCloseAllFds to instruct liblxc to do the same.

from go-lxc.

Apologies for the delay!

Yes, it turns out WantDaemonize was the thing I wanted; unclear how I kept missing it.

from go-lxc.