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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix possible garbage in status command output #1872

Merged
merged 2 commits into from Aug 31, 2021

Conversation

dominiklohmann
Copy link
Member

馃摂 Description

This had a slight race condition: We captured a string_view to every component's type, and used that as a key in the status output. However, when small string optimization is used, the addition/removal of components caused the string views to be invalidated. This change makes it so the status handler copies the keys.

馃摑 Checklist

  • All user-facing changes have changelog entries.
  • The changes are reflected on docs.tenzir.com/vast, if necessary.
  • The PR description contains instructions for the reviewer, if necessary.

馃幆 Review Instructions

Reasoning about the change should suffice, it's really simple in hindsight.

@dominiklohmann dominiklohmann added the bug Incorrect behavior label Aug 30, 2021
@dominiklohmann dominiklohmann requested a review from a team August 30, 2021 10:05
This had a slight race condition: We captured a `string_view` to every
component's type, and used that as a key in the status output. However,
when small string optimization is used, the addition/removal of
components caused the string views to be invalidated. This change makes
it so the status handler copies the keys.
@dominiklohmann dominiklohmann force-pushed the story/ch28145/status-output-garbage branch from 8b87a6c to 5934bf5 Compare August 30, 2021 10:06
@dominiklohmann dominiklohmann requested a review from a team August 30, 2021 11:34
Copy link
Member

@tobim tobim left a comment

Choose a reason for hiding this comment

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

I verified that the issue is resolved by testing locally.

@dominiklohmann dominiklohmann merged commit c977cb6 into master Aug 31, 2021
@dominiklohmann dominiklohmann deleted the story/ch28145/status-output-garbage branch August 31, 2021 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior
Projects
None yet
2 participants