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 fake_upstream for tsan #34652

Merged
merged 1 commit into from
Jun 11, 2024
Merged

Conversation

ravenblackx
Copy link
Contributor

Commit Message: Fix fake_upstream for tsan
Additional Description: Based on the traces in #34644 my understanding is that the lambda given to postToConnectionThread reads from the header object sometimes before and sometimes after the main thread 'deletes' the shared_ptr. I think this is actually tsan being incorrect, because either way the race goes, the shared_ptr is always deleted after the read, but it also is unnecessary to have the race because the main thread doesn't use the object after posting it, so posting it by move should resolve the problem.
I wasn't able to test this fix myself because some part of the test command in standard vscode dev container was unhappy and just failed the test at startup every time with or without this change, saying FATAL: ThreadSanitizer: unexpected memory mapping 0x74ee2ec72000-0x74ee2f100000. Even if this doesn't fix it it's still an improvement.
Risk Level: None
Testing: Yes it is
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a
Fixes #34644

Signed-off-by: Raven Black <ravenblack@dropbox.com>
Copy link
Contributor

@alyssawilk alyssawilk left a comment

Choose a reason for hiding this comment

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

Wow, nice find. I wonder how long that's been the case?

@alyssawilk alyssawilk enabled auto-merge (squash) June 10, 2024 19:47
@alyssawilk alyssawilk disabled auto-merge June 11, 2024 12:44
@alyssawilk alyssawilk merged commit 59f4370 into envoyproxy:main Jun 11, 2024
20 checks passed
@ravenblackx ravenblackx deleted the tcp_tunneling_test branch June 11, 2024 14:35
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.

//test/integration:tcp_tunneling_integration_test flakes after custom inline header changes
2 participants