PenaDevops/delve
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

*: Never use pointer to proc.ErrProcessExited (#2431)

Browse Source 
We have some places where we use proc.ErrProcessExited and some places
that use &proc.ErrProcessExited, resulting in checks for process exited
errors occasionally failing on some architectures.
Uniform use of ErrProcessExited to the non-pointer version.

Fixes intermittent failure of TestStepOutPreservesGoroutine.
This commit is contained in:
Alessandro Arzilli 2021-04-13 08:52:29 +02:00 committed by GitHub
parent fe616c27ff
commit 3c69f7435e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 48 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
pkg/proc/gdbserial/gdbserver.go
View File

@ -730,7 +730,7 @@ func (p *gdbProcess) Valid() (bool, error) {
return false, proc.ErrProcessDetached return false, proc.ErrProcessDetached
} }
if p.exited { if p.exited {
return false, &proc.ErrProcessExited{Pid: p.Pid()} return false, proc.ErrProcessExited{Pid: p.Pid()}
} }
return true, nil return true, nil
} }
@ -780,7 +780,7 @@ const (
// a breakpoint is hit or signal is received. // a breakpoint is hit or signal is received.
func (p *gdbProcess) ContinueOnce() (proc.Thread, proc.StopReason, error) { func (p *gdbProcess) ContinueOnce() (proc.Thread, proc.StopReason, error) {
if p.exited { if p.exited {
return nil, proc.StopExited, &proc.ErrProcessExited{Pid: p.conn.pid} return nil, proc.StopExited, proc.ErrProcessExited{Pid: p.conn.pid}
} }
if p.conn.direction == proc.Forward { if p.conn.direction == proc.Forward {

6
pkg/proc/native/proc.go
View File

@ -140,7 +140,7 @@ func (dbp *nativeProcess) Valid() (bool, error) {
return false, proc.ErrProcessDetached return false, proc.ErrProcessDetached
} }
if dbp.exited { if dbp.exited {
return false, &proc.ErrProcessExited{Pid: dbp.Pid()} return false, proc.ErrProcessExited{Pid: dbp.Pid()}
} }
return true, nil return true, nil
} }
@ -185,7 +185,7 @@ func (dbp *nativeProcess) Breakpoints() *proc.BreakpointMap {
// sends SIGSTOP to all threads. // sends SIGSTOP to all threads.
func (dbp *nativeProcess) RequestManualStop() error { func (dbp *nativeProcess) RequestManualStop() error {
if dbp.exited { if dbp.exited {
return &proc.ErrProcessExited{Pid: dbp.Pid()} return proc.ErrProcessExited{Pid: dbp.Pid()}
} }
dbp.stopMu.Lock() dbp.stopMu.Lock()
defer dbp.stopMu.Unlock() defer dbp.stopMu.Unlock()
@ -222,7 +222,7 @@ func (dbp *nativeProcess) EraseBreakpoint(bp *proc.Breakpoint) error {
// This could be the result of a breakpoint or signal. // This could be the result of a breakpoint or signal.
func (dbp *nativeProcess) ContinueOnce() (proc.Thread, proc.StopReason, error) { func (dbp *nativeProcess) ContinueOnce() (proc.Thread, proc.StopReason, error) {
if dbp.exited { if dbp.exited {
return nil, proc.StopExited, &proc.ErrProcessExited{Pid: dbp.Pid()} return nil, proc.StopExited, proc.ErrProcessExited{Pid: dbp.Pid()}
} }
for { for {

2
pkg/proc/native/proc_darwin.go
View File

@ -424,7 +424,7 @@ func (dbp *nativeProcess) resume() error {
// stop stops all running threads and sets breakpoints // stop stops all running threads and sets breakpoints
func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error) { func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error) {
if dbp.exited { if dbp.exited {
return nil, &proc.ErrProcessExited{Pid: dbp.Pid()} return nil, proc.ErrProcessExited{Pid: dbp.Pid()}
} }
for _, th := range dbp.threads { for _, th := range dbp.threads {
if !th.Stopped() { if !th.Stopped() {

2
pkg/proc/native/proc_freebsd.go
View File

@ -349,7 +349,7 @@ func (dbp *nativeProcess) resume() error {
// stop stops all running threads and sets breakpoints // stop stops all running threads and sets breakpoints
func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error) { func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error) {
if dbp.exited { if dbp.exited {
return nil, &proc.ErrProcessExited{Pid: dbp.Pid()} return nil, proc.ErrProcessExited{Pid: dbp.Pid()}
} }
// set breakpoints on all threads // set breakpoints on all threads
for _, th := range dbp.threads { for _, th := range dbp.threads {

2
pkg/proc/native/proc_linux.go
View File

@ -505,7 +505,7 @@ func (dbp *nativeProcess) resume() error {
// stop stops all running threads and sets breakpoints // stop stops all running threads and sets breakpoints
func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error) { func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error) {
if dbp.exited { if dbp.exited {
return nil, &proc.ErrProcessExited{Pid: dbp.Pid()} return nil, proc.ErrProcessExited{Pid: dbp.Pid()}
} }
for _, th := range dbp.threads { for _, th := range dbp.threads {

2
pkg/proc/native/proc_windows.go
View File

@ -414,7 +414,7 @@ func (dbp *nativeProcess) resume() error {
// stop stops all running threads threads and sets breakpoints // stop stops all running threads threads and sets breakpoints
func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error) { func (dbp *nativeProcess) stop(trapthread *nativeThread) (*nativeThread, error) {
if dbp.exited { if dbp.exited {
return nil, &proc.ErrProcessExited{Pid: dbp.Pid()} return nil, proc.ErrProcessExited{Pid: dbp.Pid()}
} }
dbp.os.running = false dbp.os.running = false

8
service/dap/server.go
View File

@ -161,7 +161,7 @@ func (s *Server) Stop() {
kill := s.config.Debugger.AttachPid == 0 kill := s.config.Debugger.AttachPid == 0
if err := s.debugger.Detach(kill); err != nil { if err := s.debugger.Detach(kill); err != nil {
switch err.(type) { switch err.(type) {
case *proc.ErrProcessExited: case proc.ErrProcessExited:
s.log.Debug(err) s.log.Debug(err)
default: default:
s.log.Error(err) s.log.Error(err)
@ -658,7 +658,7 @@ func (s *Server) onDisconnectRequest(request *dap.DisconnectRequest) {
state, err := s.debugger.Command(&api.DebuggerCommand{Name: api.Halt}, nil) state, err := s.debugger.Command(&api.DebuggerCommand{Name: api.Halt}, nil)
if err != nil { if err != nil {
switch err.(type) { switch err.(type) {
case *proc.ErrProcessExited: case proc.ErrProcessExited:
exited = err exited = err
default: default:
s.log.Error(err) s.log.Error(err)
@ -685,7 +685,7 @@ func (s *Server) onDisconnectRequest(request *dap.DisconnectRequest) {
err = s.debugger.Detach(kill) err = s.debugger.Detach(kill)
if err != nil { if err != nil {
switch err.(type) { switch err.(type) {
case *proc.ErrProcessExited: case proc.ErrProcessExited:
exited = err exited = err
s.logToConsole(exited.Error()) s.logToConsole(exited.Error())
default: default:
@ -808,7 +808,7 @@ func (s *Server) onThreadsRequest(request *dap.ThreadsRequest) {
gs, _, err := s.debugger.Goroutines(0, 0) gs, _, err := s.debugger.Goroutines(0, 0)
if err != nil { if err != nil {
switch err.(type) { switch err.(type) {
case *proc.ErrProcessExited: case proc.ErrProcessExited:
// If the program exits very quickly, the initial threads request will complete after it has exited. // If the program exits very quickly, the initial threads request will complete after it has exited.
// A TerminatedEvent has already been sent. Ignore the err returned in this case. // A TerminatedEvent has already been sent. Ignore the err returned in this case.
s.send(&dap.ThreadsResponse{Response: *newResponse(request.Request)}) s.send(&dap.ThreadsResponse{Response: *newResponse(request.Request)})

2
service/debugger/debugger.go
View File

@ -584,7 +584,7 @@ func (d *Debugger) state(retLoadCfg *proc.LoadConfig) (*api.DebuggerState, error
exited := false exited := false
if _, err := d.target.Valid(); err != nil { if _, err := d.target.Valid(); err != nil {
_, exited = err.(*proc.ErrProcessExited) _, exited = err.(proc.ErrProcessExited)
} }
state = &api.DebuggerState{ state = &api.DebuggerState{

24
service/test/integration1_test.go
View File

@ -813,18 +813,18 @@ func Test1Issue355(t *testing.T) {
state = <-ch state = <-ch
assertError(state.Err, t, "Continue()") assertError(state.Err, t, "Continue()")
_, err = c.Next() s, err := c.Next()
assertError(err, t, "Next()") assertErrorOrExited(s, err, t, "Next()")
_, err = c.Step() s, err = c.Step()
assertError(err, t, "Step()") assertErrorOrExited(s, err, t, "Step()")
_, err = c.StepInstruction() s, err = c.StepInstruction()
assertError(err, t, "StepInstruction()") assertErrorOrExited(s, err, t, "StepInstruction()")
_, err = c.SwitchThread(tid) s, err = c.SwitchThread(tid)
assertError(err, t, "SwitchThread()") assertErrorOrExited(s, err, t, "SwitchThread()")
_, err = c.SwitchGoroutine(gid) s, err = c.SwitchGoroutine(gid)
assertError(err, t, "SwitchGoroutine()") assertErrorOrExited(s, err, t, "SwitchGoroutine()")
_, err = c.Halt() s, err = c.Halt()
assertError(err, t, "Halt()") assertErrorOrExited(s, err, t, "Halt()")
_, err = c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.main", Line: -1}) _, err = c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.main", Line: -1})
assertError(err, t, "CreateBreakpoint()") assertError(err, t, "CreateBreakpoint()")
_, err = c.ClearBreakpoint(bp.ID) _, err = c.ClearBreakpoint(bp.ID)

34
service/test/integration2_test.go
View File

@ -1095,6 +1095,16 @@ func TestClientServer_FullStacktrace(t *testing.T) {
}) })
} }
func assertErrorOrExited(s *api.DebuggerState, err error, t *testing.T, reason string) {
if err != nil {
return
}
if s != nil && s.Exited {
return
}
t.Fatalf("%s (no error and no exited status)", reason)
}
func TestIssue355(t *testing.T) { func TestIssue355(t *testing.T) {
// After the target process has terminated should return an error but not crash // After the target process has terminated should return an error but not crash
protest.AllowRecording(t) protest.AllowRecording(t)
@ -1116,18 +1126,18 @@ func TestIssue355(t *testing.T) {
state = <-ch state = <-ch
assertError(state.Err, t, "Continue()") assertError(state.Err, t, "Continue()")
_, err = c.Next() s, err := c.Next()
assertError(err, t, "Next()") assertErrorOrExited(s, err, t, "Next()")
_, err = c.Step() s, err = c.Step()
assertError(err, t, "Step()") assertErrorOrExited(s, err, t, "Step()")
_, err = c.StepInstruction() s, err = c.StepInstruction()
assertError(err, t, "StepInstruction()") assertErrorOrExited(s, err, t, "StepInstruction()")
_, err = c.SwitchThread(tid) s, err = c.SwitchThread(tid)
assertError(err, t, "SwitchThread()") assertErrorOrExited(s, err, t, "SwitchThread()")
_, err = c.SwitchGoroutine(gid) s, err = c.SwitchGoroutine(gid)
assertError(err, t, "SwitchGoroutine()") assertErrorOrExited(s, err, t, "SwitchGoroutine()")
_, err = c.Halt() s, err = c.Halt()
assertError(err, t, "Halt()") assertErrorOrExited(s, err, t, "Halt()")
_, err = c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.main", Line: -1}) _, err = c.CreateBreakpoint(&api.Breakpoint{FunctionName: "main.main", Line: -1})
if testBackend != "rr" { if testBackend != "rr" {
assertError(err, t, "CreateBreakpoint()") assertError(err, t, "CreateBreakpoint()")