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

refactor(userspace/falco): make signal handlers thread safe #2091

Merged
merged 3 commits into from
Aug 26, 2022

Conversation

jasondellaluce
Copy link
Contributor

What type of PR is this?

/kind design

kind feature

Any specific area of the project related to this PR?

/area engine

What this PR does / why we need it:

This PR refactors the signal handlers, and their effects, to be thread-safe. Thread-safety is not something we need right now, but it is preparatory for #2074.

Also, this defines a termination condition for the Falco output engine. Basically, a full queue would generally imply a deadlock for Falco, so we exit with an error in that case.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

refactor(userspace/falco): make signal handlers thread safe

@jasondellaluce
Copy link
Contributor Author

/milestone 0.33.0

@poiana poiana added this to the 0.33.0 milestone Jun 23, 2022
leogr
leogr previously approved these changes Aug 25, 2022
Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@poiana
Copy link
Contributor

poiana commented Aug 25, 2022

LGTM label has been added.

Git tree hash: bb7260a566fc78981eb218a7f87b1309a213dc94

FedeDP
FedeDP previously approved these changes Aug 25, 2022
Copy link
Contributor

@FedeDP FedeDP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

…processing loop

Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
…define exiting condition

Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
Signed-off-by: Jason Dellaluce <jasondellaluce@gmail.com>
@jasondellaluce jasondellaluce dismissed stale reviews from FedeDP and leogr via 281d012 August 26, 2022 07:22
@jasondellaluce jasondellaluce force-pushed the refactor/signals-thread-safety branch from eba0cfa to 281d012 Compare August 26, 2022 07:22
@poiana poiana removed the lgtm label Aug 26, 2022
@poiana poiana requested review from FedeDP and leogr August 26, 2022 07:22
Copy link
Contributor

@FedeDP FedeDP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

@poiana poiana added the lgtm label Aug 26, 2022
@poiana
Copy link
Contributor

poiana commented Aug 26, 2022

LGTM label has been added.

Git tree hash: cface11b9ddbd6c6f31c961bb500a70d45ec5acb

@poiana
Copy link
Contributor

poiana commented Aug 26, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, jasondellaluce

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [FedeDP,jasondellaluce]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@poiana poiana merged commit 34ca787 into master Aug 26, 2022
@poiana poiana deleted the refactor/signals-thread-safety branch August 26, 2022 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants