SDK 3.9.4: Goroutine leak in reportToRun

[Segflow]

In helm.sh/helm/v3 v3.9.4 the method reportToRun is implemented as follow:

func (i *Install) reportToRun(c chan<- resultMessage, rel *release.Release, err error) {
	i.Lock.Lock()
	if err != nil {
		rel, err = i.failRelease(rel, err)
	}
	c <- resultMessage{r: rel, e: err}
	i.Lock.Unlock()
}

I have confirmed that after performing an install action (using RunWithContext) leads to goroutine leak.

The above screenshot shows that goroutines are stuck in reportToRun. Here a sample stack trace of one of those routines:

goroutine 15463 [chan send, 32 minutes]:
helm.sh/helm/v3/pkg/action.(*Install).reportToRun(0xc0026086e0, 0xc0026804c0?, 0x3?, {0x22dd9c0?, 0xc006a7bf20?})
	/go/pkg/mod/helm.sh/helm/v3@v3.9.4/pkg/action/install.go:435 +0xd6
helm.sh/helm/v3/pkg/action.(*Install).performInstall(0xc0026086e0, 0xc005269f60?, 0xc005216310, {0x0?, 0x0?, 0x0?}, {0xc0026804c0, 0x3, 0x4})
	/go/pkg/mod/helm.sh/helm/v3@v3.9.4/pkg/action/install.go:389 +0x2ea
created by helm.sh/helm/v3/pkg/action.(*Install).RunWithContext
	/go/pkg/mod/helm.sh/helm/v3@v3.9.4/pkg/action/install.go:349 +0x128a

The block seems to be in the chan send call.

On another note, I believe if we fall into the error path, the i.Lock will stay locked forever.

[Segflow]

According to this code:

// Run executes the installation with Context
func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals map[string]interface{}) (*release.Release, error) {
        ...
        rChan := make(chan resultMessage)
	doneChan := make(chan struct{})
	defer close(doneChan)
	go i.performInstall(rChan, rel, toBeAdopted, resources)
	go i.handleContext(ctx, rChan, doneChan, rel)
	result := <-rChan
	//start preformInstall go routine
	return result.r, result.e
}

We read from rChan only once. But both performInstall and handleContext do call reportToRun (more then once) which tries to send to rChan.

So basically rChan is getting consumed only once why we send to it more then once.

[joejulian]

Have you checked to see if that's still in main?

[Segflow]

Yes, it's the same code in main

[zhanghaohao]

I have seen the same problem in my program.
My program build helm client and call (*Upgrade)RunWithContext to perform upgrade/install.
After the program runs for a period, it ends up with OOM.
so i used pprof to parse the memory allocation and goroutine, i noticed memory leak and goroutine leak.
memory allocation flame graph as follow:

goroutine flame graph as follow :

When we run helm client command to call (*Upgrade)RunWithContext, it is fine, because the main func exits after upgrade/install finished. but if we develop program by call (*Upgrade)RunWithContext, the main func will not exit(until we want it exit), and the goroutine leak happens, will caused memory leak which is a bigger problem.

[Segflow]

@joejulian is there any plan to work on this issue soon?

[zhanghaohao]

this problem had been solved according to the code in tag: v3.9.4

	go u.releasingUpgrade(rChan, upgradedRelease, current, target, originalRelease)  
	go u.handleContext(ctx, doneChan, ctxChan, upgradedRelease)  
	select {  
	case result := <-rChan:
		return result.r, result.e
	case result := <-ctxChan:
		return result.r, result.e
	}

related issue #10439
could close the issue now

[Segflow]

No that in the upgrade release path not install release path

[zhanghaohao]

@Segflow sorry, i didnot notice that you are mentioning install release path rather than upgrade release path.
after reading the install code carefully, i realized that it should not lead to goroutine leak in normal cases.

// Run executes the installation with Context
func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals map[string]interface{}) (*release.Release, error) {
        ...
        rChan := make(chan resultMessage)
	doneChan := make(chan struct{})
	defer close(doneChan)
	go i.performInstall(rChan, rel, toBeAdopted, resources)
	go i.handleContext(ctx, rChan, doneChan, rel)
	result := <-rChan
	//start preformInstall go routine
	return result.r, result.e
}

according to the above code, in normal cases, performInstall should finish with result sent to rChan, and result := <-rChan is unblocked, then close doneChan before return.
the close(doneChan) will lead handleContext goroutine return. this is what we expected.

func (i *Install) handleContext(ctx context.Context, c chan<- resultMessage, done chan struct{}, rel *release.Release) {
	select {
	case <-ctx.Done():
		err := ctx.Err()
		i.reportToRun(c, rel, err)
	case <-done:
		return
	}
}

@Segflow how do you think about this?

[Segflow]

The issue is performInstall also calls reportToRun, this leads to have both

	go i.performInstall(rChan, rel, toBeAdopted, resources)
	go i.handleContext(ctx, rChan, doneChan, rel)

Send to the channel. We have this issue in our production

[zhanghaohao]

i guess the case <-ctx.Done() in handleContext is triggered in your case, this could lead to goroutine leak due to performInstall and handleContext both are sending to rChan.
but goroutine leak donot happen in other cases.

Please correct me if i am wrong.

[Segflow]

indeed

[Segflow]

Any update on this please?

[mattfarina]

There's a PR to address this at #11978