Skip to content

Commit 5e10879

Browse files
authored
bugfix: Kill compile processes that generates too much output (#2883)
* Added integration test * Fixed linter warning * bugfix: Kill compile processes that generates too much output * Apply suggestion from code review
1 parent 28c7985 commit 5e10879

File tree

5 files changed

+101
-11
lines changed

5 files changed

+101
-11
lines changed

internal/arduino/builder/internal/preprocessor/gcc.go

+37-4
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package preprocessor
1717

1818
import (
19+
"bytes"
1920
"context"
2021
"errors"
2122
"fmt"
@@ -77,10 +78,42 @@ func GCC(
7778
if err != nil {
7879
return Result{}, err
7980
}
80-
stdout, stderr, err := proc.RunAndCaptureOutput(ctx)
8181

82-
// Append gcc arguments to stdout
83-
stdout = append([]byte(fmt.Sprintln(strings.Join(args, " "))), stdout...)
82+
stdout := bytes.NewBuffer(nil)
83+
stderr := bytes.NewBuffer(nil)
8484

85-
return Result{args: proc.GetArgs(), stdout: stdout, stderr: stderr}, err
85+
ctx, cancel := context.WithCancel(ctx)
86+
defer cancel()
87+
count := 0
88+
stderrLimited := writerFunc(func(p []byte) (int, error) {
89+
// Limit the size of the stderr buffer to 100KB
90+
n, err := stderr.Write(p)
91+
count += n
92+
if count > 100*1024 {
93+
fmt.Fprintln(stderr, i18n.Tr("Compiler error output has been truncated."))
94+
cancel()
95+
}
96+
return n, err
97+
})
98+
99+
proc.RedirectStdoutTo(stdout)
100+
proc.RedirectStderrTo(stderrLimited)
101+
102+
// Append gcc arguments to stdout before running the command
103+
fmt.Fprintln(stdout, strings.Join(args, " "))
104+
105+
if err := proc.Start(); err != nil {
106+
return Result{}, err
107+
}
108+
109+
// Wait for the process to finish
110+
err = proc.WaitWithinContext(ctx)
111+
112+
return Result{args: proc.GetArgs(), stdout: stdout.Bytes(), stderr: stderr.Bytes()}, err
113+
}
114+
115+
type writerFunc func(p []byte) (n int, err error)
116+
117+
func (f writerFunc) Write(p []byte) (n int, err error) {
118+
return f(p)
86119
}

internal/integrationtest/arduino-cli.go

+17-7
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"errors"
2323
"fmt"
2424
"io"
25+
"maps"
2526
"os"
2627
"runtime"
2728
"strings"
@@ -190,12 +191,16 @@ func (cli *ArduinoCLI) Run(args ...string) ([]byte, []byte, error) {
190191
return cli.RunWithCustomEnv(cli.cliEnvVars, args...)
191192
}
192193

194+
// RunWithContext executes the given arduino-cli command with the given context and returns the output.
195+
// If the context is canceled, the command is killed.
196+
func (cli *ArduinoCLI) RunWithContext(ctx context.Context, args ...string) ([]byte, []byte, error) {
197+
return cli.RunWithCustomEnvContext(ctx, cli.cliEnvVars, args...)
198+
}
199+
193200
// GetDefaultEnv returns a copy of the default execution env used with the Run method.
194201
func (cli *ArduinoCLI) GetDefaultEnv() map[string]string {
195202
res := map[string]string{}
196-
for k, v := range cli.cliEnvVars {
197-
res[k] = v
198-
}
203+
maps.Copy(res, cli.cliEnvVars)
199204
return res
200205
}
201206

@@ -324,8 +329,13 @@ func (cli *ArduinoCLI) InstallMockedAvrdude(t *testing.T) {
324329

325330
// RunWithCustomEnv executes the given arduino-cli command with the given custom env and returns the output.
326331
func (cli *ArduinoCLI) RunWithCustomEnv(env map[string]string, args ...string) ([]byte, []byte, error) {
332+
return cli.RunWithCustomEnvContext(context.Background(), env, args...)
333+
}
334+
335+
// RunWithCustomEnv executes the given arduino-cli command with the given custom env and returns the output.
336+
func (cli *ArduinoCLI) RunWithCustomEnvContext(ctx context.Context, env map[string]string, args ...string) ([]byte, []byte, error) {
327337
var stdoutBuf, stderrBuf bytes.Buffer
328-
err := cli.run(&stdoutBuf, &stderrBuf, nil, env, args...)
338+
err := cli.run(ctx, &stdoutBuf, &stderrBuf, nil, env, args...)
329339

330340
errBuf := stderrBuf.Bytes()
331341
cli.t.NotContains(string(errBuf), "panic: runtime error:", "arduino-cli panicked")
@@ -336,15 +346,15 @@ func (cli *ArduinoCLI) RunWithCustomEnv(env map[string]string, args ...string) (
336346
// RunWithCustomInput executes the given arduino-cli command pushing the given input stream and returns the output.
337347
func (cli *ArduinoCLI) RunWithCustomInput(in io.Reader, args ...string) ([]byte, []byte, error) {
338348
var stdoutBuf, stderrBuf bytes.Buffer
339-
err := cli.run(&stdoutBuf, &stderrBuf, in, cli.cliEnvVars, args...)
349+
err := cli.run(context.Background(), &stdoutBuf, &stderrBuf, in, cli.cliEnvVars, args...)
340350

341351
errBuf := stderrBuf.Bytes()
342352
cli.t.NotContains(string(errBuf), "panic: runtime error:", "arduino-cli panicked")
343353

344354
return stdoutBuf.Bytes(), errBuf, err
345355
}
346356

347-
func (cli *ArduinoCLI) run(stdoutBuff, stderrBuff io.Writer, stdinBuff io.Reader, env map[string]string, args ...string) error {
357+
func (cli *ArduinoCLI) run(ctx context.Context, stdoutBuff, stderrBuff io.Writer, stdinBuff io.Reader, env map[string]string, args ...string) error {
348358
if cli.cliConfigPath != nil {
349359
args = append([]string{"--config-file", cli.cliConfigPath.String()}, args...)
350360
}
@@ -402,8 +412,8 @@ func (cli *ArduinoCLI) run(stdoutBuff, stderrBuff io.Writer, stdinBuff io.Reader
402412
}
403413
}()
404414
}
415+
cliErr := cliProc.WaitWithinContext(ctx)
405416
wg.Wait()
406-
cliErr := cliProc.Wait()
407417
fmt.Fprintln(terminalOut, color.HiBlackString("<<< Run completed (err = %v)", cliErr))
408418

409419
return cliErr
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// This file is part of arduino-cli.
2+
//
3+
// Copyright 2025 ARDUINO SA (http://www.arduino.cc/)
4+
//
5+
// This software is released under the GNU General Public License version 3,
6+
// which covers the main part of arduino-cli.
7+
// The terms of this license can be found at:
8+
// https://www.gnu.org/licenses/gpl-3.0.en.html
9+
//
10+
// You can be released from the requirements of the above licenses by purchasing
11+
// a commercial license. Buying such a license is mandatory if you want to
12+
// modify or otherwise use the software for commercial activities involving the
13+
// Arduino software without disclosing the source code of your own applications.
14+
// To purchase a commercial license, send an email to [email protected].
15+
16+
package compile_test
17+
18+
import (
19+
"context"
20+
"testing"
21+
"time"
22+
23+
"github.com/arduino/arduino-cli/internal/integrationtest"
24+
"github.com/arduino/go-paths-helper"
25+
"github.com/stretchr/testify/require"
26+
)
27+
28+
func TestCompileWithInfiniteMultipleIncludeRecursion(t *testing.T) {
29+
env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
30+
t.Cleanup(env.CleanUp)
31+
32+
// Install Arduino AVR Boards
33+
_, _, err := cli.Run("core", "install", "arduino:[email protected]")
34+
require.NoError(t, err)
35+
36+
sketch, err := paths.New("testdata", "SketchWithRecursiveIncludes").Abs()
37+
require.NoError(t, err)
38+
39+
// Time-limited test to prevent OOM
40+
ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second)
41+
t.Cleanup(cancel)
42+
_, _, _ = cli.RunWithContext(ctx, "compile", "-b", "arduino:avr:uno", sketch.String())
43+
require.NotErrorIs(t, ctx.Err(), context.DeadlineExceeded, "compilation should not hang")
44+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
#include "a.h"
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
#include "a.h"
2+
#include "a.h"

0 commit comments

Comments
 (0)