Skip to content

Commit

Permalink
Fix goroutine leak, expose global stop ctx
Browse files Browse the repository at this point in the history
It is common to want a global stop context passed to the provider configure
function. Unfortunately the grpc ctx is request scoped and cancels after
the function returns. This makes saving/resusing a global ctx impossible.
This hack will attach a global stop synchronized context... onto the ctx
passed into the provider config function. The reason this is done is to
avoid an API signature breaking change, this feature should also be hidden
as we would prefer providers use the request scoped and deadlines contexts
passed into the CRUD functions over a global one.
  • Loading branch information
appilon committed Jul 14, 2020
1 parent 621b208 commit 39d496d
Showing 1 changed file with 15 additions and 8 deletions.
23 changes: 15 additions & 8 deletions internal/helper/plugin/grpc_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,16 @@ type GRPCProviderServer struct {
stopMu sync.Mutex
}

func mergeStop(stopCh chan struct{}, cancel context.CancelFunc) {
<-stopCh
cancel()
// mergeStop is called in a goroutine and waits for the global stop signal
// and propagates cancellation to the passed in ctx/cancel func. The ctx is
// also passed to this function and waited upon so no goroutine leak is caused.
func mergeStop(ctx context.Context, cancel context.CancelFunc, stopCh chan struct{}) {
select {
case <-ctx.Done():
return
case <-stopCh:
cancel()
}
}

// StopContext derives a new context from the passed in grpc context.
Expand All @@ -50,10 +57,7 @@ func (s *GRPCProviderServer) StopContext(ctx context.Context) context.Context {
defer s.stopMu.Unlock()

stoppable, cancel := context.WithCancel(ctx)
// It's important to pass the reference to the current stopCh.
// Closing over with an anonymous function and referencing s.stopCh
// across a goroutine is unsafe, given a new stopCh is set in Stop()
go mergeStop(s.stopCh, cancel)
go mergeStop(stoppable, cancel, s.stopCh)
return stoppable
}

Expand Down Expand Up @@ -506,7 +510,10 @@ func (s *GRPCProviderServer) Configure(ctx context.Context, req *proto.Configure
}

config := terraform.NewResourceConfigShimmed(configVal, schemaBlock)
diags := s.provider.Configure(ctx, config)
// TODO: hack, pass a context with global cancel hooked up.. on the request
// scoped context...
ctxHack := context.WithValue(ctx, "StopContext", s.StopContext(context.Background()))
diags := s.provider.Configure(ctxHack, config)
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, diags)

return resp, nil
Expand Down

0 comments on commit 39d496d

Please sign in to comment.