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

fix crashes on kqueue-platforms when built with go-1.14 #103

Merged
merged 1 commit into from
Jul 5, 2020

Conversation

asomers
Copy link
Contributor

@asomers asomers commented May 7, 2020

Go 1.14 introduces asynchronous preemption, which interrupts running
goroutines by sending a SIGURG signal. Some syscalls, like kevent(2),
will return EINTR when that happens. grok_exporter was treating EINTR
like an error, even though no error code was set. The result was an
eventual nil dereference. This patch fixes the bug by restarting kevent
on EINTR.

Submitted by: @a1exanderpetrov
Sponsored by: Axcient
Fixes: 90

Go 1.14 introduces asynchronous preemption, which interrupts running
goroutines by sending a SIGURG signal.  Some syscalls, like kevent(2),
will return EINTR when that happens.  grok_exporter was treating EINTR
like an error, even though no error code was set.  The result was an
eventual nil dereference.  This patch fixes the bug by restarting kevent
on EINTR.

Submitted by:	@a1exanderpetrov
Sponsored by:	Axcient
Fixes:		fstab#90
@coveralls
Copy link

coveralls commented May 7, 2020

Coverage Status

Coverage remained the same at 67.135% when pulling 6a3c1c0 on asomers:issue_90 into 44b195e on fstab:master.

@asomers
Copy link
Contributor Author

asomers commented Jul 3, 2020

ping. This PR is ready for review.

@fstab fstab merged commit 46bd33d into fstab:master Jul 5, 2020
@fstab
Copy link
Owner

fstab commented Jul 5, 2020

Thanks for reminding me. Great finding, thanks a lot for the fix!

@fstab fstab mentioned this pull request Jul 5, 2020
@asomers asomers deleted the issue_90 branch July 29, 2020 02:12
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.

3 participants