arduino / arduino-cli

Arduino command line tool
https://arduino.github.io/arduino-cli/latest/
GNU General Public License v3.0
4.24k stars 371 forks source link

Fixed an extremely rare race-condition during `compile` #2512

Closed cmaglie closed 5 months ago

cmaglie commented 5 months ago

Please check if the PR fulfills these requirements

See how to contribute

What kind of change does this PR introduce?

Should fix a memory access panic during saving of compilation database. This is a tentative fix since the error is so rare that we are unable to reproduce it locally.

What is the current behavior?

This is the reported panic:

  panic: runtime error: invalid memory address or nil pointer dereference [recovered]
    panic: runtime error: invalid memory address or nil pointer dereference
  [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x5c018b]

  goroutine 1 [running]:
  encoding/json.(*encodeState).marshal.func1()
    /usr/local/go/src/encoding/json/encode.go:291 +0x6d
  panic({0xd867c0?, 0x189dbc0?})
    /usr/local/go/src/runtime/panic.go:914 +0x21f
  encoding/json.appendString[...]({0xc0099d391e?, 0x50?, 0x0?}, {0x0?, 0xc0003cfc80}, 0x1)
    /usr/local/go/src/encoding/json/encode.go:967 +0xab
  encoding/json.stringEncoder(0xc005bfc300, {0xd2caa0?, 0xc00027c2a0?, 0xe311c0?}, {0x0?, 0x0?})
    /usr/local/go/src/encoding/json/encode.go:591 +0x3c7
  encoding/json.arrayEncoder.encode({0x18d8d80?}, 0xc005bfc300, {0xd1e8e0?, 0xc005d367b8?, 0x4078dd?}, {0x87?, 0x8?})
    /usr/local/go/src/encoding/json/encode.go:849 +0xd6
  encoding/json.sliceEncoder.encode({0xc0032ac758?}, 0xc005bfc300, {0xd1e8e0?, 0xc005d367b8?, 0xe311c0?}, {0xc?, 0x0?})
    /usr/local/go/src/encoding/json/encode.go:822 +0x30f
  encoding/json.structEncoder.encode({{{0xc0099aa6c0, 0x4, 0x4}, 0xc00885bd70, 0xc00885bda0}}, 0xc005bfc300, {0xe311c0?, 0xc005d36798?, 0x418268?}, {0x0, ...})
    /usr/local/go/src/encoding/json/encode.go:706 +0x21e
  encoding/json.arrayEncoder.encode({0xc0032ac918?}, 0xc005bfc300, {0xd268a0?, 0xc00886e6c0?, 0xc0032ac978?}, {0x25?, 0x2?})
    /usr/local/go/src/encoding/json/encode.go:849 +0xd6
  encoding/json.sliceEncoder.encode({0xc005adbe00?}, 0xc005bfc300, {0xd268a0?, 0xc00886e6c0?, 0xd268a0?}, {0xe0?, 0xdb?})
    /usr/local/go/src/encoding/json/encode.go:822 +0x30f
  encoding/json.(*encodeState).reflectValue(0xc0032acb28?, {0xd268a0?, 0xc00886e6c0?, 0x2?}, {0xe0?, 0x4b?})
    /usr/local/go/src/encoding/json/encode.go:323 +0x73
  encoding/json.(*encodeState).marshal(0x30?, {0xd268a0?, 0xc00886e6c0?}, {0x3?, 0x0?})
    /usr/local/go/src/encoding/json/encode.go:295 +0xb9
  encoding/json.Marshal({0xd268a0, 0xc00886e6c0})
    /usr/local/go/src/encoding/json/encode.go:162 +0xd0
  encoding/json.MarshalIndent({0xd268a0?, 0xc00886e6c0?}, {0x0, 0x0}, {0x11dbdf0, 0x1})
    /usr/local/go/src/encoding/json/encode.go:175 +0x47
  github.com/arduino/arduino-cli/arduino/builder/internal/compilation.(*Database).SaveToFile(0xc005bd8dc0)
    /home/build/arduino/builder/internal/compilation/database.go:65 +0x4d
  github.com/arduino/arduino-cli/arduino/builder.(*Builder).build(0xc005543c20)
    /home/build/arduino/builder/builder.go:462 +0x53b
  github.com/arduino/arduino-cli/arduino/builder.(*Builder).Build(0xc005543c20)
    /home/build/arduino/builder/builder.go:341 +0x10b
  github.com/arduino/arduino-cli/commands/compile.Compile({0x18bb250?, 0x0?}, 0xc0032adbc0, {0x11e02e0, 0xc000010f60}, {0x11e02e0, 0xc000010f78}, 0x0?)
    /home/build/commands/compile/compile.go:291 +0x2665
  github.com/arduino/arduino-cli/internal/cli/compile.runCompileCommand(0xc00036a300?, {0xc0002a8d80, 0x1, 0xeb0cf8?})
    /home/build/internal/cli/compile/compile.go:247 +0xd4c
  github.com/spf13/cobra.(*Command).execute(0xc00036a300, {0xc0002a8d00, 0x8, 0x8})
    /go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:944 +0x863
  github.com/spf13/cobra.(*Command).ExecuteC(0xc000004600)
    /go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:1068 +0x3a5
  github.com/spf13/cobra.(*Command).Execute(0x0?)
    /go/pkg/mod/github.com/spf13/cobra@v1.7.0/command.go:992 +0x13
  main.main()
    /home/build/main.go:31 +0xda

This issue is ephemeral, re-running the compile usually leads to a successful compile.

What is the new behavior?

Hopefully, it will always compile successfully.

Does this PR introduce a breaking change, and is titled accordingly?

No

Other information

codecov[bot] commented 5 months ago

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (47645e9) 68.90% compared to head (3342945) 68.92%.

Files Patch % Lines
...l/arduino/builder/internal/compilation/database.go 80.00% 0 Missing and 2 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2512 +/- ## ========================================== + Coverage 68.90% 68.92% +0.02% ========================================== Files 204 204 Lines 20448 20452 +4 ========================================== + Hits 14089 14096 +7 + Misses 5209 5207 -2 + Partials 1150 1149 -1 ``` | [Flag](https://app.codecov.io/gh/arduino/arduino-cli/pull/2512/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=arduino) | Coverage Δ | | |---|---|---| | [unit](https://app.codecov.io/gh/arduino/arduino-cli/pull/2512/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=arduino) | `68.92% <80.00%> (+0.02%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=arduino#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.