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

Shutdown correctly if the extension is short lived #120

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

a-cognet
Copy link
Contributor

This is a follow up from #117

If the extension stops immediately after it starts, there could be a problem where shutdown happened before start. It's something we've seen in integration tests, but may happen or worsen the case where the extension is stuck in a restart loop with osqueryd.

I changed the Run function because it buried the original error and made the investigation harder.

Here is a stacktrace

Nov 16 18:56:40 ip-.compute.internal osqueryd[1992]: {"level":"warn","time":"2023-11-16T18:56:40.378Z","msg":"Shutting down client","extensionServer":"client","version":"1.29.1","pid":"2003","component":"osquery-plugin-server"}
Nov 16 18:56:40 ip-.compute.internal osqueryd[1992]: panic: runtime error: invalid memory address or nil pointer dereference
Nov 16 18:56:40 ip-.compute.internal osqueryd[1992]: [signal SIGSEGV: segmentation violation code=0x1 addr=0x58 pc=0x143f35b]
Nov 16 18:56:40 ip-.compute.internal osqueryd[1992]: goroutine 43 [running]:
Nov 16 18:56:40 ip-.compute.internal osqueryd[1992]: github.com/osquery/osquery-go.(*ExtensionManagerServer).Start.func1(0xc0001a2960, 0xc0002d7f88)
Nov 16 18:56:40 ip-.compute.internal osqueryd[1992]: /go/src/XXXX/vendor/github.com/osquery/osquery-go/server.go:201 +0x13b
Nov 16 18:56:40 ip-.compute.internal osqueryd[1992]: github.com/osquery/osquery-go.(*ExtensionManagerServer).Start(0x0?)
Nov 16 18:56:40 ip-.compute.internal osqueryd[1992]: /go/src/XXXX/vendor/github.com/osquery/osquery-go/server.go:237 +0x25
Nov 16 18:56:40 ip-.compute.internal osqueryd[1992]: github.com/osquery/osquery-go.(*ExtensionManagerServer).Run.func1()
Nov 16 18:56:40 ip-.compute.internal osqueryd[1992]: /go/src/XXXX/vendor/github.com/osquery/osquery-go/server.go:251 +0x26
Nov 16 18:56:40 ip-.compute.internal osqueryd[1992]: created by github.com/osquery/osquery-go.(*ExtensionManagerServer).Run
Nov 16 18:56:40 ip-.compute.internal osqueryd[1992]: /go/src/XXXX/vendor/github.com/osquery/osquery-
go/server.go:250 +0x85
Nov 16 18:56:40 ip-1.compute.internal systemd[1]: Stopped The osquery Daemon.

If the extension stops immediately after it starts, there could be a
problem where shutdown happened before start. It's something we've seen
in integration tests, but may happen or worsen the case where the
extension is stuck in a restart loop with osqueryd.
@a-cognet
Copy link
Contributor Author

@directionless would you have any guidance on how I can get this PR reviewed? I could not find a procedure to follow. Thanks.

@directionless
Copy link
Member

This seems reasonable. Though the subject here says "Panic" which I think is less desirable than the error return in the code. Is the PR subject out of date?

@a-cognet a-cognet changed the title Panic if the extension is short lived Shutdown correctly if the extension is short lived Nov 30, 2023
@a-cognet
Copy link
Contributor Author

I updated the PR title with the intent, instead of stating the issue.

@directionless directionless merged commit 61ac792 into osquery:master Nov 30, 2023
3 checks passed
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.

2 participants