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

test: fix a test crash by destroying test time after consumers #5145

Merged
merged 1 commit into from
Nov 28, 2018

Conversation

alyssawilk
Copy link
Contributor

also pretty printing grpc test params while I'm in there because I despise figuring out what test is failing via staring at Test/0 Test/1 Test/2 Test/3

Risk Level: Low (test only)
Testing: goes from 80% failure with flake-options on to 0% failure
Docs Changes: n/a
Release Notes: n/a
Fixes #5071

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

nice!

@alyssawilk alyssawilk merged commit fa65648 into envoyproxy:master Nov 28, 2018
@alyssawilk alyssawilk deleted the use-after-free branch November 28, 2018 16:02
fredlas pushed a commit to fredlas/envoy that referenced this pull request Mar 5, 2019
also pretty printing grpc test params while I'm in there because I dislike figuring out what test is failing via staring at Test/0 Test/1 Test/2 Test/3

Risk Level: Low (test only)
Testing: goes from 80% failure with flake-options on to 0% failure
Docs Changes: n/a
Release Notes: n/a
Fixes envoyproxy#5071

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Fred Douglas <fredlas@google.com>
@adityagujral
Copy link

Hi,
could you please explain how this commit fixes the issue. I am facing a similar issue and I am curious to understand the fix.

@alyssawilk
Copy link
Contributor Author

I'm pulling from memory but I think basically some other objects (fake upstream?) would occasionally and non-deterministically use fake time in its destructor (time based because it happened if the stream wasn't fully cleaned up). Because the time object was deleted before the object referencing it, it caused problems.

@adityagujral
Copy link

Thanks. But how does your change in grpc_client_integration_test_harness.h address this exactly? Sorry if I am missing something very obvious as I'm new to cpp. I am seeing a similar issue in a new integration test only when using GCC and not in Clang.

@alyssawilk
Copy link
Contributor Author

in C++ class variables are destroyed bottom up, so making test_time the first in the test made sure it'd be destroyed last (once all other members were done using it)

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.

"pure virtual method called" in grpc:grpc_client_integration_test
4 participants