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 corruption in templates that use title function #3278

Merged
merged 2 commits into from
Mar 7, 2023

Conversation

grobinson-grafana
Copy link
Contributor

This commit fixes data corruption in templates that use the title function as it used a shared cases.Title when casers should not be shared between goroutines. When templates that used the title function were executed at the same time, data corruption would occur in the text that was being cased. This is fixed using a separate caser for each function call.

Here is an example of the issue as explained here:

2023/03/03 16:40:51 Waltz, BaD
2023/03/03 16:40:51 Waltz, Bad Nymph, For Quick Jigs Vex
2023/03/03 16:40:51 Waltz, Bad Nymph, For Quick Jigs Vex
2023/03/03 16:40:51 Waltz, Bad Nymph, For Quick Jigs Vex
2023/03/03 16:40:51 Waltz, Bad Nymph, For Quick Jigs Vex
2023/03/03 16:40:51 Waltz, Bad Nymph, For Quick Jigs Vex
2023/03/03 16:40:51  Nym
2023/03/03 16:40:51 Wph, For icc Jigs Vex
2023/03/03 16:40:51 Waltz, Bad Nymph, For Quick Jigs Vex
2023/03/03 16:40:51 Waltz, Bad Nymph, For Quick Jigs Vex

and with the fix:

2023/03/03 21:33:15 Waltz, Bad Nymph, For Quick Jigs Vex
2023/03/03 21:33:15 Waltz, Bad Nymph, For Quick Jigs Vex
2023/03/03 21:33:15 Waltz, Bad Nymph, For Quick Jigs Vex
2023/03/03 21:33:15 Waltz, Bad Nymph, For Quick Jigs Vex
2023/03/03 21:33:15 Waltz, Bad Nymph, For Quick Jigs Vex
2023/03/03 21:33:15 Waltz, Bad Nymph, For Quick Jigs Vex
2023/03/03 21:33:15 Waltz, Bad Nymph, For Quick Jigs Vex
2023/03/03 21:33:15 Waltz, Bad Nymph, For Quick Jigs Vex
2023/03/03 21:33:15 Waltz, Bad Nymph, For Quick Jigs Vex
2023/03/03 21:33:15 Waltz, Bad Nymph, For Quick Jigs Vex

Fixes #3277

Copy link
Member

@gotjosh gotjosh left a comment

Choose a reason for hiding this comment

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

LGTM

Can you please commit the test case you used? Ideally, we iterate trough the list of functions and execute the test for each.

This commit fixes data corruption in templates that use the title
function as it used a shared cases.Title when casers should not
be shared between goroutines. When templates that used the title
function were executed at the same time, data corruption would occur
in the text that was being cased. This is fixed using a separate
caser for each function call.

Signed-off-by: George Robinson <[email protected]>
@simonpasquier simonpasquier merged commit 012dbc9 into prometheus:main Mar 7, 2023
@taisph
Copy link

taisph commented Apr 13, 2023

Is a fix release coming? I've started seeing alert emails with weird or missing subjects and today even an empty email that seem to have been cut off at the subject header. I'll remove the use of title as a workaround in the meantime.

@gotjosh
Copy link
Member

gotjosh commented Apr 13, 2023

It is; we're planning to regularize the release schedule.

hoperays pushed a commit to hoperays/alertmanager that referenced this pull request Apr 23, 2023
This commit fixes data corruption in templates that use the title
function as it used a shared cases.Title when casers should not
be shared between goroutines. When templates that used the title
function were executed at the same time, data corruption would occur
in the text that was being cased. This is fixed using a separate
caser for each function call.

Add tests to assertDefaultFuncs are thread-safe

Signed-off-by: George Robinson <[email protected]>
radek-ryckowski pushed a commit to goldmansachs/alertmanager that referenced this pull request Nov 6, 2023
This commit fixes data corruption in templates that use the title
function as it used a shared cases.Title when casers should not
be shared between goroutines. When templates that used the title
function were executed at the same time, data corruption would occur
in the text that was being cased. This is fixed using a separate
caser for each function call.

Add tests to assertDefaultFuncs are thread-safe

Signed-off-by: George Robinson <[email protected]>
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.

Race condition in ExecuteTextString
4 participants