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

Clean up the docker socket on termination of the daemon. #6962

Merged
merged 1 commit into from
Jul 30, 2014
Merged

Clean up the docker socket on termination of the daemon. #6962

merged 1 commit into from
Jul 30, 2014

Conversation

erikh
Copy link
Contributor

@erikh erikh commented Jul 10, 2014

Please review. I feel like there's more cases here I'm missing, but I'm not sure where they might be.

Thanks!

@erikh
Copy link
Contributor Author

erikh commented Jul 10, 2014

closes #3349

@cyphar
Copy link
Contributor

cyphar commented Jul 11, 2014

Doesn't closing the socket remove the "file" on *nix systems? Couldn't we just close the sockets instead of path splitting (or do we not store them in the daemon structure)?

@erikh
Copy link
Contributor Author

erikh commented Jul 11, 2014

@cyphar the problem is it needs to be handled there -- api/server/server.go does the same thing when setting up the listening endpoints.

I can investigate the close, but I have not heard this before, nor does the socket go away on server termination, which this fixes. I would think that the socket would be closed at that point.

@@ -98,6 +110,7 @@ func InitServer(job *engine.Job) engine.Status {
log.Printf("Received signal '%v', starting shutdown of docker...\n", sig)
switch sig {
case os.Interrupt, syscall.SIGTERM:
srv.removeUnixSockets()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be one of the last things that you do before calling os.Exit?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(as usual)

@vieux vieux self-assigned this Jul 12, 2014
@vieux vieux added this to the 1.1.2 milestone Jul 12, 2014
@erikh
Copy link
Contributor Author

erikh commented Jul 12, 2014

Fixed according to @crosbymichael's comments. Please review.

@SvenDowideit
Copy link
Contributor

excellent - could someone please push this patch into the dockerserver libswarm backend?

@erikh
Copy link
Contributor Author

erikh commented Jul 13, 2014

Yeah, I can apply it later. Will ping then.

-Erik

On Jul 12, 2014, at 5:30 PM, Sven Dowideit notifications@github.com wrote:

excellent - could someone please push this patch into the dockerserver libswarm backend?


Reply to this email directly or view it on GitHub.

@erikh
Copy link
Contributor Author

erikh commented Jul 15, 2014

I can address this independently. Can we review this standalone? I think it's ready.

@vieux
Copy link
Contributor

vieux commented Jul 16, 2014

LGTM

@tiborvass
Copy link
Contributor

@erikh needs a simple rebase.

Docker-DCO-1.1-Signed-off-by: Erik Hollensbe <github@hollensbe.org> (github: erikh)
@erikh
Copy link
Contributor Author

erikh commented Jul 26, 2014

@tiborvass done

@vieux
Copy link
Contributor

vieux commented Jul 30, 2014

ping @tiborvass

@tiborvass
Copy link
Contributor

LGTM

1 similar comment
@unclejack
Copy link
Contributor

LGTM

unclejack added a commit that referenced this pull request Jul 30, 2014
Clean up the docker socket on termination of the daemon.
@unclejack unclejack merged commit c3045f5 into moby:master Jul 30, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants