Skip to content

agent: remove data race in agent config#20200

Merged
rboyer merged 1 commit into
mainfrom
rboyer/fix-agent-config-race
Jan 12, 2024
Merged

agent: remove data race in agent config#20200
rboyer merged 1 commit into
mainfrom
rboyer/fix-agent-config-race

Conversation

@rboyer
Copy link
Copy Markdown
Member

@rboyer rboyer commented Jan 12, 2024

Description

To fix an issue displaying the current reloaded config in the v1/agent/self endpoint #18681 caused the agent's internal config struct member to be deepcopied and replaced on reload.

This is not safe because the field is not protected by a lock, nor should it be due to how it is accessed by the rest of the system.

This PR does the same deepcopy, but into a new field solely for the point of capturing the current reloaded values for display purposes. If there has been no reload then the original config is used.

@rboyer rboyer added pr/no-changelog PR does not need a corresponding .changelog entry pr/no-metrics-test backport/1.15 labels Jan 12, 2024
@rboyer rboyer self-assigned this Jan 12, 2024
Copy link
Copy Markdown
Contributor

@dhiaayachi dhiaayachi left a comment

Choose a reason for hiding this comment

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

LGTM!

@rboyer rboyer merged commit 7f9ed03 into main Jan 12, 2024
@rboyer rboyer deleted the rboyer/fix-agent-config-race branch January 12, 2024 21:11
rboyer added a commit that referenced this pull request Jan 12, 2024
To fix an issue displaying the current reloaded config in the
v1/agent/self endpoint #18681 caused the agent's internal
config struct member to be deepcopied and replaced on reload.

This is not safe because the field is not protected by a lock, nor
should it be due to how it is accessed by the rest of the system.

This PR does the same deepcopy, but into a new field solely for
the point of capturing the current reloaded values for display
purposes. If there has been no reload then the original config is used.
rboyer added a commit that referenced this pull request Jan 12, 2024
To fix an issue displaying the current reloaded config in the
v1/agent/self endpoint #18681 caused the agent's internal
config struct member to be deepcopied and replaced on reload.

This is not safe because the field is not protected by a lock, nor
should it be due to how it is accessed by the rest of the system.

This PR does the same deepcopy, but into a new field solely for
the point of capturing the current reloaded values for display
purposes. If there has been no reload then the original config is used.
@hc-github-team-consul-core
Copy link
Copy Markdown
Collaborator

@rboyer, a backport is missing for this PR [20200] for versions [1.15,1.16,1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

3 similar comments
@hc-github-team-consul-core
Copy link
Copy Markdown
Collaborator

@rboyer, a backport is missing for this PR [20200] for versions [1.15,1.16,1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@hc-github-team-consul-core
Copy link
Copy Markdown
Collaborator

@rboyer, a backport is missing for this PR [20200] for versions [1.15,1.16,1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@hc-github-team-consul-core
Copy link
Copy Markdown
Collaborator

@rboyer, a backport is missing for this PR [20200] for versions [1.15,1.16,1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

rboyer added a commit that referenced this pull request Jan 16, 2024
….x (#20202)

[1.16.x] agent: remove data race in agent config (#20200)

To fix an issue displaying the current reloaded config in the
v1/agent/self endpoint #18681 caused the agent's internal
config struct member to be deepcopied and replaced on reload.

This is not safe because the field is not protected by a lock, nor
should it be due to how it is accessed by the rest of the system.

This PR does the same deepcopy, but into a new field solely for
the point of capturing the current reloaded values for display
purposes. If there has been no reload then the original config is used.

Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>
rboyer added a commit that referenced this pull request Jan 16, 2024
….x (#20201)

[1.15.x] agent: remove data race in agent config (#20200)

To fix an issue displaying the current reloaded config in the
v1/agent/self endpoint #18681 caused the agent's internal
config struct member to be deepcopied and replaced on reload.

This is not safe because the field is not protected by a lock, nor
should it be due to how it is accessed by the rest of the system.

This PR does the same deepcopy, but into a new field solely for
the point of capturing the current reloaded values for display
purposes. If there has been no reload then the original config is used.

Co-authored-by: R.B. Boyer <4903+rboyer@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr/no-changelog PR does not need a corresponding .changelog entry pr/no-metrics-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants