Skip to content

fix: panic: close of closed channel #5939

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 15, 2025
+20 −15
Merged

fix: panic: close of closed channel #5939

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions pkg/goanalysis/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package goanalysis

import (
"context"
"encoding/gob"
"fmt"
"go/token"
Expand Down Expand Up @@ -259,18 +260,20 @@ func (r *runner) analyze(pkgs []*packages.Package, analyzers []*analysis.Analyze
debugf("Analyzing at most %d packages in parallel", gomaxprocs)

loadSem := make(chan struct{}, gomaxprocs)
stopChan := make(chan struct{}, 1)

debugf("There are %d initial and %d total packages", len(initialPkgs), len(loadingPackages))

ctx, cancel := context.WithCancel(context.Background())
defer cancel()

var wg sync.WaitGroup

for _, lp := range loadingPackages {
if lp.isInitial {
wg.Add(1)

go func(lp *loadingPackage) {
lp.analyzeRecursive(stopChan, r.loadMode, loadSem)
lp.analyzeRecursive(ctx, cancel, r.loadMode, loadSem)

wg.Done()
}(lp)
Expand Down
4 changes: 1 addition & 3 deletions pkg/goanalysis/runner_action.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,10 @@ func (actAlloc *actionAllocator) alloc() *action {
return act
}

func (act *action) waitUntilDependingAnalyzersWorked(ctx context.Context, stopChan chan struct{}) {
func (act *action) waitUntilDependingAnalyzersWorked(ctx context.Context) {
for _, dep := range act.Deps {
if dep.Package == act.Package {
select {
case <-stopChan:
return
case <-ctx.Done():
return
case <-dep.analysisDoneCh:
Expand Down
24 changes: 14 additions & 10 deletions pkg/goanalysis/runner_loadingpackage.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ type loadingPackage struct {
decUseMutex sync.Mutex
}

func (lp *loadingPackage) analyzeRecursive(stopChan chan struct{}, loadMode LoadMode, loadSem chan struct{}) {
func (lp *loadingPackage) analyzeRecursive(ctx context.Context, cancel context.CancelFunc, loadMode LoadMode, loadSem chan struct{}) {
lp.analyzeOnce.Do(func() {
// Load the direct dependencies, in parallel.
var wg sync.WaitGroup
Expand All @@ -50,24 +50,30 @@ func (lp *loadingPackage) analyzeRecursive(stopChan chan struct{}, loadMode Load

for _, imp := range lp.imports {
go func(imp *loadingPackage) {
imp.analyzeRecursive(stopChan, loadMode, loadSem)
imp.analyzeRecursive(ctx, cancel, loadMode, loadSem)

wg.Done()
}(imp)
}

wg.Wait()

lp.analyze(stopChan, loadMode, loadSem)
lp.analyze(ctx, cancel, loadMode, loadSem)
})
}

func (lp *loadingPackage) analyze(stopChan chan struct{}, loadMode LoadMode, loadSem chan struct{}) {
func (lp *loadingPackage) analyze(ctx context.Context, cancel context.CancelFunc, loadMode LoadMode, loadSem chan struct{}) {
loadSem <- struct{}{}
defer func() {
<-loadSem
}()

select {
case <-ctx.Done():
return
default:
}
Comment on lines +71 to +75
Copy link
Member

Choose a reason for hiding this comment

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

It should be logically the same but when ctx.Done() is closed ctx.Err() will return a non nil error. Just a personal pref on what's easier to read I guess.

Suggested change
select {
case <-ctx.Done():
return
default:
}
if ctx.Err() != nil {
return
}

Copy link
Member Author

Choose a reason for hiding this comment

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

In this situation, for me, checking the error is confusing because we don't expect an error but a stop, and it will act like we are ignoring an error.

It's a personal pref too, but in this context, I prefer using Done.

Copy link
Member

Choose a reason for hiding this comment

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

Go for it, either works. ✅


// Save memory on unused more fields.
defer lp.decUse(loadMode < LoadModeWholeProgram)

Expand All @@ -85,16 +91,14 @@ func (lp *loadingPackage) analyze(stopChan chan struct{}, loadMode LoadMode, loa
return
}

actsWg, ctx := errgroup.WithContext(context.Background())
actsWg, ctxGroup := errgroup.WithContext(ctx)

for _, act := range lp.actions {
actsWg.Go(func() error {
act.waitUntilDependingAnalyzersWorked(ctx, stopChan)
act.waitUntilDependingAnalyzersWorked(ctxGroup)

select {
case <-stopChan:
return nil
case <-ctx.Done():
case <-ctxGroup.Done():
return nil
default:
}
Expand All @@ -107,7 +111,7 @@ func (lp *loadingPackage) analyze(stopChan chan struct{}, loadMode LoadMode, loa

err := actsWg.Wait()
if err != nil {
close(stopChan)
cancel()
}
}

Expand Down
Loading