search

machine-drivers / machine

Machine management for a container-centric world
Apache License 2.0
25 stars 16 forks source link

plugin/localbinary: Exit output stream goroutines when plugin closes #12

Closed cezarsa closed 5 years ago

cezarsa commented 5 years ago

A similar PR was sent to the official repo in docker/machine#4629 but since it's in maintenance mode I'm also sending it here.

This commit ensures that when a plugin instance is closed the goroutines responsible for streaming stdout and stderr of the called binary will also exit, preventing a goroutines leak.

Before this commit these goroutines could stay blocked forever if Close() was called while the binary still had some pending output.

I found this bug after debugging a real world goroutine leak, the goroutines dump would show thousands of goroutines stuck at:

github.com/tsuru/tsuru/vendor/github.com/docker/machine/libmachine/drivers/plugin/localbinary.stream(0xc023013d80, 0xc0000aad20)
    /home/travis/gopath/src/github.com/tsuru/tsuru/vendor/github.com/docker/machine/libmachine/drivers/plugin/localbinary/plugin.go:177 +0x7c
created by github.com/tsuru/tsuru/vendor/github.com/docker/machine/libmachine/drivers/plugin/localbinary.(*Plugin).AttachStream
    /home/travis/gopath/src/github.com/tsuru/tsuru/vendor/github.com/docker/machine/libmachine/drivers/plugin/localbinary/plugin.go:183 +0x67
gbraad commented 5 years ago

Can you rebase and check if this is still needed?

afbjorklund commented 5 years ago

Seems to be merged (and in v0.16.1)

https://github.com/docker/machine/commit/55bfb59b67e7db9c058d6a973a46c7a2d32f0d17

praveenkumar commented 5 years ago

Yes, it is merged and also it is rebased so I am going to close it.