diff --git a/v2/tools/identify_license/backend/backend.go b/v2/tools/identify_license/backend/backend.go index 1a98207..7b3983f 100644 --- a/v2/tools/identify_license/backend/backend.go +++ b/v2/tools/identify_license/backend/backend.go @@ -69,7 +69,19 @@ func (b *ClassifierBackend) SetTraceConfiguration(tc *classifier.TraceConfigurat } // ClassifyLicenses runs the license classifier over the given file. -func (b *ClassifierBackend) ClassifyLicenses(numTasks int, filenames []string, headers bool) (errors []error) { +func (b *ClassifierBackend) ClassifyLicenses(numTasks int, filenames []string, headers bool) []error { + return b.classifyLicenses(context.Background(), numTasks, filenames, headers) +} + +// ClassifyLicensesWithContext runs the license classifier over the given file. +// Once ctx is done no further files are started, and the call returns ctx.Err() +// after the files already in flight finish, so it can return slightly after the +// deadline. No classification work outlives the call. +func (b *ClassifierBackend) ClassifyLicensesWithContext(ctx context.Context, numTasks int, filenames []string, headers bool) []error { + return b.classifyLicenses(ctx, numTasks, filenames, headers) +} + +func (b *ClassifierBackend) classifyLicenses(ctx context.Context, numTasks int, filenames []string, headers bool) (errors []error) { // Create a pool from which tasks can later be started. We use a pool because the OS limits // the number of files that can be open at any one time. task := make(chan bool, numTasks) @@ -83,6 +95,8 @@ func (b *ClassifierBackend) ClassifyLicenses(numTasks int, filenames []string, h analyze := func(filename string) { defer func() { wg.Done() + // task is never closed, nothing ranges over it and this send runs + // after wg.Done(), so closing it would panic with "send on closed channel". task <- true }() if err := b.classifyLicense(filename, headers); err != nil { @@ -90,40 +104,32 @@ func (b *ClassifierBackend) ClassifyLicenses(numTasks int, filenames []string, h } } +dispatch: for _, filename := range filenames { + // checked first so a done ctx always wins over a free task slot. + if ctx.Err() != nil { + break + } + select { + case <-ctx.Done(): + break dispatch + case <-task: + } wg.Add(1) - <-task go analyze(filename) } - go func() { - wg.Wait() - close(task) - close(errs) - }() + wg.Wait() + close(errs) + if err := ctx.Err(); err != nil { + return []error{err} + } for err := range errs { errors = append(errors, err) } return errors } -// ClassifyLicensesWithContext runs the license classifier over the given file; ensure that it will respect the timeout in the provided context. -func (b *ClassifierBackend) ClassifyLicensesWithContext(ctx context.Context, numTasks int, filenames []string, headers bool) (errors []error) { - done := make(chan bool) - go func() { - errors = b.ClassifyLicenses(numTasks, filenames, headers) - done <- true - }() - select { - case <-ctx.Done(): - err := ctx.Err() - errors = append(errors, err) - return errors - case <-done: - return errors - } -} - // classifyLicense is called by a Go-function to perform the actual // classification of a license. func (b *ClassifierBackend) classifyLicense(filename string, headers bool) error { @@ -162,5 +168,9 @@ func (b *ClassifierBackend) classifyLicense(filename string, headers bool) error // GetResults returns the results of the classifications. func (b *ClassifierBackend) GetResults() results.LicenseTypes { - return b.results + // read under the lock and return a copy, so callers can sort or hold the + // results while another classification on this backend is appending. + b.mu.Lock() + defer b.mu.Unlock() + return append(results.LicenseTypes(nil), b.results...) } diff --git a/v2/tools/identify_license/backend/backend_test.go b/v2/tools/identify_license/backend/backend_test.go new file mode 100644 index 0000000..2cb4777 --- /dev/null +++ b/v2/tools/identify_license/backend/backend_test.go @@ -0,0 +1,97 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package backend + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/google/licenseclassifier/v2/assets" +) + +func writeLicense(t *testing.T) string { + t.Helper() + b, err := assets.ReadLicenseFile("License/MIT/pristine.txt") + if err != nil { + t.Fatal(err) + } + path := filepath.Join(t.TempDir(), "LICENSE") + if err := os.WriteFile(path, b, 0o600); err != nil { + t.Fatal(err) + } + return path +} + +// TestClassifyLicensesRepeated calls ClassifyLicenses many times on one file. +// Closing the task channel used to race with the last worker returning its +// slot, which panicked with "send on closed channel". Run with -race for the +// most reliable reproduction. +func TestClassifyLicensesRepeated(t *testing.T) { + be, err := New() + if err != nil { + t.Fatal(err) + } + path := writeLicense(t) + + for i := 0; i < 200; i++ { + if errs := be.ClassifyLicenses(10, []string{path}, false); errs != nil { + t.Fatalf("ClassifyLicenses() = %v", errs) + } + } + + found := false + for _, r := range be.GetResults() { + if r.Name == "MIT" { + found = true + } + } + if !found { + t.Errorf("GetResults() did not contain MIT") + } +} + +// TestClassifyLicensesWithContextDone runs a classification with a done +// context, then a normal one on the same backend. The done run must not start +// any work or leave any running, so only the second run's single result shows +// up. This used to fail because the done run kept classifying in the +// background and its results leaked into the second run. +func TestClassifyLicensesWithContextDone(t *testing.T) { + be, err := New() + if err != nil { + t.Fatal(err) + } + path := writeLicense(t) + paths := make([]string, 50) + for i := range paths { + paths[i] = path + } + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + errs := be.ClassifyLicensesWithContext(ctx, 4, paths, false) + if len(errs) != 1 || errs[0] != context.Canceled { + t.Fatalf("ClassifyLicensesWithContext() = %v, want [%v]", errs, context.Canceled) + } + + if errs := be.ClassifyLicenses(4, []string{path}, false); errs != nil { + t.Fatalf("ClassifyLicenses() = %v", errs) + } + if got := be.GetResults(); len(got) != 1 { + t.Errorf("GetResults() has %d results, want 1", len(got)) + } +}