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

[libbeat] Fix data race in convert processor #17032

Merged

Conversation

andrewkroh
Copy link
Member

What does this PR do?

If the convert processor was used in the global context it could lead to data races because there was a variable that was reused across executions. When processors are used in the global context they are shared across individual publisher clients so you could end up with a data race.

The fix here was to replace the shared state with a local variable. In testing this didn't make much of a difference in the number of allocations.

Why is it important?

Data races can lead to panics or incorrect events.

Checklist

  • My code follows the style guidelines of this project
  • I have added tests that prove my fix is effective or that my feature works

How to test this PR locally

cd libbeat/processors/convert && go test -race -cpu 4 -bench . -benchtime=5s -benchmem .

@andrewkroh andrewkroh added bug review libbeat needs_backport PR is waiting to be backported to other branches. labels Mar 16, 2020
If the convert processor was used in the global context it could lead to data races because there was a
variable that was reused across executions. When processors are used in the global context they are
shared across individual publisher clients so you could end up with a data race.

The fix here was to replace the shared state with a local variable. In testing this didn't make much of
a difference in the number of allocations.
@andrewkroh andrewkroh force-pushed the bugfix/libbeat-convert-proc-concurrency branch from f074ccc to 470fcd7 Compare March 16, 2020 16:10
@andresrc andresrc added [zube]: Inbox Team:Integrations Label for the Integrations team labels Mar 16, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/siem (Team:SIEM)

@andrewkroh andrewkroh merged commit b6167ae into elastic:master Mar 16, 2020
andrewkroh added a commit to andrewkroh/beats that referenced this pull request Mar 16, 2020
If the convert processor was used in the global context it could lead to data races because there was a
variable that was reused across executions. When processors are used in the global context they are
shared across individual publisher clients so you could end up with a data race.

The fix here was to replace the shared state with a local variable. In testing this didn't make much of
a difference in the number of allocations.

(cherry picked from commit b6167ae)
@andrewkroh andrewkroh added v7.7.0 and removed needs_backport PR is waiting to be backported to other branches. labels Mar 16, 2020
andrewkroh added a commit that referenced this pull request Mar 19, 2020
If the convert processor was used in the global context it could lead to data races because there was a
variable that was reused across executions. When processors are used in the global context they are
shared across individual publisher clients so you could end up with a data race.

The fix here was to replace the shared state with a local variable. In testing this didn't make much of
a difference in the number of allocations.

(cherry picked from commit b6167ae)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants