Skip to content

v3client: wrap watch ctxs with blank ctx#7816

Merged
heyitsanthony merged 1 commit intoetcd-io:masterfrom
heyitsanthony:v3client-blankctx
Apr 26, 2017
Merged

v3client: wrap watch ctxs with blank ctx#7816
heyitsanthony merged 1 commit intoetcd-io:masterfrom
heyitsanthony:v3client-blankctx

Conversation

@heyitsanthony
Copy link
Copy Markdown

Printing the values in ctx.String() will data race if the value
is mutable and doesn't implement String(), which seems to be common.
Instead, just return a fixed string instead of computing it; v3client
watches don't need as much flexibility for creating separate strings,
so separate ctx strings probably aren't necessary at this point.

Fixes #7811

Printing the values in ctx.String() will data race if the value
is mutable and doesn't implement String(), which seems to be common.
Instead, just return a fixed string instead of computing it; v3client
watches don't need as much flexibility for creating separate strings,
so separate ctx strings probably aren't necessary at this point.

Fixes etcd-io#7811
package v3client

import (
"context"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With std context, we cannot compile because of gRPC?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yeah :(

Copy link
Copy Markdown
Contributor

@gyuho gyuho left a comment

Choose a reason for hiding this comment

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

lgtm. Thanks! /cc @xiang90

@fanminshi
Copy link
Copy Markdown
Contributor

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

TestV3ElectionObserve: race condition

3 participants