Skip to content

bug: EtcdV3Storage.Watch doesn't close the watchResponse chan #779

Description

@starsz

Please answer these questions before submitting your issue.

  • Why do you submit this issue?
  • Question or discussion
  • Bug
  • Requirements
  • Feature or performance improvement
  • Other

Bug

  • Which version of Apache APISIX Dashboard, OS, and Browser?
    APISIX Dashboard branch v2.0

  • What happened?
    If possible, provide a way to reproduce the error.

Hi, I think we need to close ch in EtcdV3Storage.Watch function.
If not, the groutine in GenericStore.Init() can't break the for. This called goroutine leak

Now:

func (s *EtcdV3Storage) Watch(ctx context.Context, key string) <-chan WatchResponse {
eventChan := Client.Watch(ctx, key, clientv3.WithPrefix())
ch := make(chan WatchResponse, 1)
go func() {
for event := range eventChan {
output := WatchResponse{
Canceled: event.Canceled,
}
for i := range event.Events {
e := Event{
Key: string(event.Events[i].Kv.Key),
Value: string(event.Events[i].Kv.Value),
}
switch event.Events[i].Type {
case clientv3.EventTypePut:
e.Type = EventTypePut
case clientv3.EventTypeDelete:
e.Type = EventTypeDelete
}
output.Events = append(output.Events, e)
}
if output.Canceled {
output.Error = fmt.Errorf("channel canceled")
}
ch <- output
}
}()
return ch
}

Advice:

func (s *EtcdV3Storage) Watch(ctx context.Context, key string) <-chan WatchResponse { 
 	eventChan := Client.Watch(ctx, key, clientv3.WithPrefix()) 
 	ch := make(chan WatchResponse, 1) 
 	go func() { 
 		for event := range eventChan { 
 			output := WatchResponse{ 
 				Canceled: event.Canceled, 
 			} 
  
 			for i := range event.Events { 
 				e := Event{ 
 					Key:   string(event.Events[i].Kv.Key), 
 					Value: string(event.Events[i].Kv.Value), 
 				} 
 				switch event.Events[i].Type { 
 				case clientv3.EventTypePut: 
 					e.Type = EventTypePut 
 				case clientv3.EventTypeDelete: 
 					e.Type = EventTypeDelete 
 				} 
 				output.Events = append(output.Events, e) 
 			} 
 			if output.Canceled { 
 				output.Error = fmt.Errorf("channel canceled") 
 			} 
 			ch <- output 
 		} 
              
                close(ch)
 	}() 
  
 	return ch 
 } 

So that, the goroutine created in GenericStore.Init can exit when utils.CloseAll() is called.
Although this problem is not very serious, we still need to fix it. Right?

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions