Skip to content

Commit 40b6b7f

Browse files
matloobprattmic
authored andcommitted
[internal-branch.go1.23-vendor] telemetry: do not crash parent if child could not be started
Instead of calling log.Fatal if the child could not be started, call log.Print. Various factors in the user's environment could cause the child to not be able to start, but that shouldn't crash the parent process (usually the go command). Change other fatals into prints with early returns when attempting to start the child. Reset the crash output file to clean up if the child process could not be started and crashmonitoring is enabled. Updates golang/go#68976 Updates golang/go#68995 Change-Id: I42f55dc90f68f91b272a7ebf64d2a4a3b00815c7 Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/607595 Commit-Queue: Michael Matloob <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Robert Findley <[email protected]> (cherry picked from commit 61baa7d) Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/609136
1 parent 8ab0cad commit 40b6b7f

File tree

2 files changed

+16
-7
lines changed

2 files changed

+16
-7
lines changed

internal/crashmonitor/monitor.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,12 @@ import (
2121
"golang.org/x/telemetry/internal/counter"
2222
)
2323

24-
// Supported reports whether the runtime supports [runtime.SetCrashOutput].
24+
// Supported reports whether the runtime supports [runtime/debug.SetCrashOutput].
2525
//
2626
// TODO(adonovan): eliminate once go1.23+ is assured.
2727
func Supported() bool { return setCrashOutput != nil }
2828

29-
var setCrashOutput func(*os.File) error // = runtime.SetCrashOutput on go1.23+
29+
var setCrashOutput func(*os.File) error // = runtime/debug.SetCrashOutput on go1.23+
3030

3131
// Parent sets up the parent side of the crashmonitor. It requires
3232
// exclusive use of a writable pipe connected to the child process's stdin.

start.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -206,31 +206,40 @@ func startChild(reportCrashes, upload bool, result *StartResult) {
206206
fd, err := os.Stat(telemetry.Default.DebugDir())
207207
if err != nil {
208208
if !os.IsNotExist(err) {
209-
log.Fatalf("failed to stat debug directory: %v", err)
209+
log.Printf("failed to stat debug directory: %v", err)
210+
return
210211
}
211212
} else if fd.IsDir() {
212213
// local/debug exists and is a directory. Set stderr to a log file path
213214
// in local/debug.
214215
childLogPath := filepath.Join(telemetry.Default.DebugDir(), "sidecar.log")
215216
childLog, err := os.OpenFile(childLogPath, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0600)
216217
if err != nil {
217-
log.Fatalf("opening sidecar log file for child: %v", err)
218+
log.Printf("opening sidecar log file for child: %v", err)
219+
return
218220
}
219221
defer childLog.Close()
220222
cmd.Stderr = childLog
221223
}
222224

225+
var crashOutputFile *os.File
223226
if reportCrashes {
224227
pipe, err := cmd.StdinPipe()
225228
if err != nil {
226-
log.Fatalf("StdinPipe: %v", err)
229+
log.Printf("StdinPipe: %v", err)
230+
return
227231
}
228232

229-
crashmonitor.Parent(pipe.(*os.File)) // (this conversion is safe)
233+
crashOutputFile = pipe.(*os.File) // (this conversion is safe)
230234
}
231235

232236
if err := cmd.Start(); err != nil {
233-
log.Fatalf("can't start telemetry child process: %v", err)
237+
// The child couldn't be started. Log the failure.
238+
log.Printf("can't start telemetry child process: %v", err)
239+
return
240+
}
241+
if reportCrashes {
242+
crashmonitor.Parent(crashOutputFile)
234243
}
235244
result.wg.Add(1)
236245
go func() {

0 commit comments

Comments
 (0)