Skip to content

Commit

Permalink
Fix unbuffered error channel usage
Browse files Browse the repository at this point in the history
This fixes a common issue where we have an error channel that is
unbuffered. Later we have a select{}, and if that selects another case
the channel will never be read from, causing a deadlock

See istio#15897 for a real life case where
this caused Pilot to die.

Fixes istio#15902
  • Loading branch information
howardjohn committed Aug 28, 2019
1 parent 04c5b1b commit c2ceb7c
Show file tree
Hide file tree
Showing 5 changed files with 4 additions and 7 deletions.
2 changes: 1 addition & 1 deletion istioctl/pkg/kubernetes/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ func (client *Client) PodsForSelector(namespace, labelSelector string) (*v1.PodL

func RunPortForwarder(fw *PortForward, readyFunc func(fw *PortForward) error) error {

errCh := make(chan error)
errCh := make(chan error, 1)
go func() {
errCh <- fw.Forwarder.ForwardPorts()
}()
Expand Down
3 changes: 0 additions & 3 deletions pkg/adsc/adsc.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ type ADSC struct {
// NodeID is the node identity sent to Pilot.
nodeID string

done chan error

certDir string
url string

Expand Down Expand Up @@ -130,7 +128,6 @@ var (
// Dial connects to a ADS server, with optional MTLS authentication if a cert dir is specified.
func Dial(url string, certDir string, opts *Config) (*ADSC, error) {
adsc := &ADSC{
done: make(chan error),
Updates: make(chan string, 100),
VersionInfo: map[string]string{},
certDir: certDir,
Expand Down
2 changes: 1 addition & 1 deletion pkg/test/docker/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func (c *Container) Exec(ctx context.Context, cmd ...string) (ExecResult, error)

// read the output
var stdout, stderr bytes.Buffer
outputDone := make(chan error)
outputDone := make(chan error, 1)

go func() {
// StdCopy demultiplexes the stream into two buffers
Expand Down
2 changes: 1 addition & 1 deletion pkg/test/kube/port_forwarder.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ type defaultPortForwarder struct {
}

func (f *defaultPortForwarder) Start() error {
errCh := make(chan error)
errCh := make(chan error, 1)
go func() {
errCh <- f.forwarder.ForwardPorts()
}()
Expand Down
2 changes: 1 addition & 1 deletion tests/util/kube_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,7 @@ func CheckDeployment(ctx context.Context, namespace, deployment string, kubeconf
// This can be deployed by previous tests, but doesn't complete currently, blocking the test.
return nil
}
errc := make(chan error)
errc := make(chan error, 1)
go func() {
if _, err := ShellMuteOutput("kubectl -n %s rollout status %s --kubeconfig=%s", namespace, deployment, kubeconfig); err != nil {
errc <- fmt.Errorf("%s in namespace %s failed", deployment, namespace)
Expand Down

0 comments on commit c2ceb7c

Please sign in to comment.