osquery / osquery-go

Go bindings for osquery
MIT License
388 stars 79 forks source link

Go extension processes do not die #56

Closed muffins closed 6 years ago

muffins commented 6 years ago

We’ve been experiencing go extensions in Windows not dying when the service restarts. We believe this is due to the shutdown method not being implemented. I will update with more details later, but wanted the issue for posterity and record that this is an issue.

groob commented 6 years ago

The shutdown method on the thrift interface exists in a few places. You're correct that it is a no-op within the table plugin implementation:

https://github.com/kolide/osquery-go/blob/e17dfe8f44e7ac06fb2bd1cccbf64dbfbc9e5757/plugin/table/table.go#L104

but it is implemented as a method on the extension manager server:

https://github.com/kolide/osquery-go/blob/e17dfe8f44e7ac06fb2bd1cccbf64dbfbc9e5757/server.go#L227-L243

The likely problem is that the shutdown method on the server has a bug that prevents the extension from shutting down.

cc @zwass

zwass commented 6 years ago

I know @groob has made some progress on edge cases where shutdown is called but doesn't complete successfully. I think there's also a separate issue in which we are not detecting "dirty shutdowns" (see https://github.com/osquery/osquery-python/blob/8536ff537085408ac7e7eed1a48feb1da436f717/osquery/management.py#L149-L160), which is more likely to be the problem in https://github.com/kolide/launcher/issues/293.

zwass commented 6 years ago

I just put up https://github.com/kolide/osquery-go/pull/57 to handle these dirty shutdowns similar to how osquery-python does so. This should fix the shutdown issue reported by Clippy.

groob commented 6 years ago

I'm going to close this issue. I was able to observe that it works well now on windows and Nate had some positive reports in Slack as well.