This is an automated email from the ASF dual-hosted git repository. sjanc pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mynewt-newt.git
commit a47b0b2a43c8b9d71ce834fb0cbb26edb79cecca Author: Szymon Janc <[email protected]> AuthorDate: Mon Oct 19 17:52:35 2026 +0200 newt/toolchain: Fix data races during parallel compilation With -j > 1 the files of one package are compiled concurrently, but the list of compile commands and the dependency tracker's most recent object name/time were updated without locking. This could lose entries in compile_commands.json and corrupt the timestamp used to decide whether to rearchive and relink. Protect these with the compiler mutex, which already guards the object path list, and return a copy of the compile commands. Assisted-by: Claude:claude-opus-5-5 --- newt/toolchain/compiler.go | 25 ++++++++++---- newt/toolchain/deps.go | 13 +++++++- newt/toolchain/deps_test.go | 79 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 8 deletions(-) diff --git a/newt/toolchain/compiler.go b/newt/toolchain/compiler.go index 651dfba0..e8bc2df5 100644 --- a/newt/toolchain/compiler.go +++ b/newt/toolchain/compiler.go @@ -77,7 +77,8 @@ type Compiler struct { AutogeneratedLinkerIncludeDir string // Needs to be locked whenever a mutable field in this struct is accessed - // during a build. Currently, objPathList is the only such member. + // during a build: objPathList, compileCommands and the dependency + // tracker's most recent file data. mutex *sync.Mutex depTracker DepTracker @@ -119,7 +120,21 @@ type Compiler struct { } func (c *Compiler) GetCompileCommands() []CompileCommand { - return c.compileCommands + c.mutex.Lock() + defer c.mutex.Unlock() + + return append([]CompileCommand{}, c.compileCommands...) +} + +func (c *Compiler) addCompileCommand(cmd []string, file string) { + c.mutex.Lock() + defer c.mutex.Unlock() + + c.compileCommands = append(c.compileCommands, + CompileCommand{ + Command: strings.Join(cmd, " "), + File: file, + }) } func (c *Compiler) GetCcPath() string { @@ -761,11 +776,7 @@ func (c *Compiler) CompileFile(file string, compilerType int) error { } util.StatusMessage(util.VERBOSITY_DEFAULT, "%s", string(o)) - c.compileCommands = append(c.compileCommands, - CompileCommand{ - Command: strings.Join(cmd, " "), - File: file, - }) + c.addCompileCommand(cmd, file) err = writeCommandFile(objPath, cmd) if err != nil { diff --git a/newt/toolchain/deps.go b/newt/toolchain/deps.go index fa526f7a..1da52934 100644 --- a/newt/toolchain/deps.go +++ b/newt/toolchain/deps.go @@ -50,6 +50,14 @@ func NewDepTracker(c *Compiler) DepTracker { } func (d *DepTracker) SetMostRecent(name string, t time.Time) { + d.compiler.mutex.Lock() + defer d.compiler.mutex.Unlock() + + d.setMostRecent(name, t) +} + +// setMostRecent must be called with the compiler mutex locked. +func (d *DepTracker) setMostRecent(name string, t time.Time) { d.MostRecentName = name // Truncate sub-second part of timestamp. Timestamps of generated files @@ -128,8 +136,11 @@ func (tracker *DepTracker) ProcessFileTime(file string) error { return err } + tracker.compiler.mutex.Lock() + defer tracker.compiler.mutex.Unlock() + if modTime.After(tracker.MostRecentTime) { - tracker.SetMostRecent(file, modTime) + tracker.setMostRecent(file, modTime) } return nil diff --git a/newt/toolchain/deps_test.go b/newt/toolchain/deps_test.go new file mode 100644 index 00000000..585fcda5 --- /dev/null +++ b/newt/toolchain/deps_test.go @@ -0,0 +1,79 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 toolchain + +import ( + "fmt" + "os" + "path/filepath" + "sync" + "testing" + "time" +) + +// newTestCompiler creates a compiler which is usable for tests that do not +// need a loaded compiler package. +func newTestCompiler() *Compiler { + c := &Compiler{ + mutex: &sync.Mutex{}, + objPathList: map[string]bool{}, + extraDeps: []string{}, + compileCommands: []CompileCommand{}, + } + c.depTracker = NewDepTracker(c) + + return c +} + +// Run with -race to detect unsynchronized access. +func TestCompilerConcurrentUpdates(t *testing.T) { + c := newTestCompiler() + dir := t.TempDir() + + const n = 50 + var wg sync.WaitGroup + for i := 0; i < n; i++ { + obj := filepath.Join(dir, fmt.Sprintf("f%d.o", i)) + if err := os.WriteFile(obj, nil, 0644); err != nil { + t.Fatal(err) + } + + wg.Add(3) + go func() { + defer wg.Done() + c.addCompileCommand([]string{"cc", "-c", obj}, obj) + }() + go func() { + defer wg.Done() + c.depTracker.SetMostRecent(obj, time.Now()) + }() + go func() { + defer wg.Done() + if err := c.depTracker.ProcessFileTime(obj); err != nil { + t.Error(err) + } + }() + } + wg.Wait() + + if got := len(c.GetCompileCommands()); got != n { + t.Errorf("expected %d compile commands, got %d", n, got) + } +}
