-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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 Issue 20928 #21981
Fix Issue 20928 #21981
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address comments before merging. Thanks!
@@ -1746,13 +1746,24 @@ XdsClient::XdsClient(Combiner* combiner, grpc_pollset_set* interested_parties, | |||
} | |||
} | |||
|
|||
XdsClient::~XdsClient() { GRPC_COMBINER_UNREF(combiner_, "xds_client"); } | |||
XdsClient::~XdsClient() { | |||
gpr_log(GPR_INFO, "[xds_client %p] Destroying xds client", this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please guard this by checking whether the tracer is enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or, alternatively, just remove this line for now, and wait for me to add it as part of #21941. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, aside from the one remaining comment. Thanks!
Thanks for reviewing! |
Fixes #20928