Repository: go-delve/delve
Pull request #2433 (2021-04-12 17:15:07)

service/dap: rename launch configuration 'wd' to 'cwd' 
Rename the launch configuration for specifying the working directory.


derekparker: What's the purpose of this change? Does it adhere better to the DAP spec?

TeamCity build failure is unrelated.
aarzilli: I think it's because the javascript adapter calls it cwd.
hyangah: This is not part of DAP spec, but for historical reasons, a commonly used attribute among many debug adapters (like `program`, `env`, `port`, ...) https://code.visualstudio.com/docs/editor/debugging#_launchjson-attributes

> `cwd` - current working directory for finding dependencies and other files

Patch:
diff --git a/service/dap/server.go b/service/dap/server.go
index a77f913f..444972a1 100644
--- a/service/dap/server.go
+++ b/service/dap/server.go
@@ -606,13 +606,13 @@ func (s *Server) onLaunchRequest(request *dap.LaunchRequest) {
 	s.config.Debugger.WorkingDir = filepath.Dir(program)
 
 	// Set the WorkingDir for this program to the one specified in the request arguments.
-	wd, ok := request.Arguments["wd"]
+	wd, ok := request.Arguments["cwd"]
 	if ok {
 		wdParsed, ok := wd.(string)
 		if !ok {
 			s.sendErrorResponse(request.Request,
 				FailedToLaunch, "Failed to launch",
-				fmt.Sprintf("'wd' attribute '%v' in debug configuration is not a string.", wd))
+				fmt.Sprintf("'cwd' attribute '%v' in debug configuration is not a string.", wd))
 			return
 		}
 		s.config.Debugger.WorkingDir = wdParsed
@@ -1083,8 +1083,6 @@ func (s *Server) onScopesRequest(request *dap.ScopesRequest) {
 	}
 	locScope := &fullyQualifiedVariable{&proc.Variable{Name: "Locals", Children: slicePtrVarToSliceVar(locals)}, "", true}
 
-	// TODO(polina): Annotate shadowed variables
-
 	scopeArgs := dap.Scope{Name: argScope.Name, VariablesReference: s.variableHandles.create(argScope)}
 	scopeLocals := dap.Scope{Name: locScope.Name, VariablesReference: s.variableHandles.create(locScope)}
 	scopes := []dap.Scope{scopeArgs, scopeLocals}
@@ -1237,8 +1235,18 @@ func (s *Server) onVariablesRequest(request *dap.VariablesRequest) {
 				cfqname = "" // complex children are not struct fields and can't be accessed directly
 			}
 			cvalue, cvarref := s.convertVariable(c, cfqname)
+
+			// Annotate any shadowed variables to "(name)" in order
+			// to distinguish from non-shadowed variables.
+			// TODO(suzmue): should we support a special evaluateName syntax that
+			// can access shadowed variables?
+			name := c.Name
+			if c.Flags&proc.VariableShadowed == proc.VariableShadowed {
+				name = fmt.Sprintf("(%s)", name)
+			}
+
 			children[i] = dap.Variable{
-				Name:               c.Name,
+				Name:               name,
 				EvaluateName:       cfqname,
 				Value:              cvalue,
 				VariablesReference: cvarref,
diff --git a/service/rpc1/server.go b/service/rpc1/server.go
index da3e3c8d..558594dd 100644
--- a/service/rpc1/server.go
+++ b/service/rpc1/server.go
@@ -35,11 +35,7 @@ func (s *RPCServer) ProcessPid(arg1 interface{}, pid *int) error {
 }
 
 func (s *RPCServer) Detach(kill bool, ret *int) error {
-	err := s.debugger.Detach(kill)
-	if s.config.DisconnectChan != nil {
-		close(s.config.DisconnectChan)
-	}
-	return err
+	return s.debugger.Detach(kill)
 }
 
 func (s *RPCServer) Restart(arg1 interface{}, arg2 *int) error {
diff --git a/service/rpc2/server.go b/service/rpc2/server.go
index a6f7bc73..81ee8aac 100644
--- a/service/rpc2/server.go
+++ b/service/rpc2/server.go
@@ -58,12 +58,7 @@ type DetachOut struct {
 
 // Detach detaches the debugger, optionally killing the process.
 func (s *RPCServer) Detach(arg DetachIn, out *DetachOut) error {
-	err := s.debugger.Detach(arg.Kill)
-	if s.config.DisconnectChan != nil {
-		close(s.config.DisconnectChan)
-		s.config.DisconnectChan = nil
-	}
-	return err
+	return s.debugger.Detach(arg.Kill)
 }
 
 type RestartIn struct {
diff --git a/service/rpccommon/server.go b/service/rpccommon/server.go
index d3eb47dc..affdd114 100644
--- a/service/rpccommon/server.go
+++ b/service/rpccommon/server.go
@@ -89,8 +89,8 @@ func NewServer(config *service.Config) *ServerImpl {
 
 // Stop stops the JSON-RPC server.
 func (s *ServerImpl) Stop() error {
+	close(s.stopChan)
 	if s.config.AcceptMulti {
-		close(s.stopChan)
 		s.listener.Close()
 	}
 	kill := s.config.Debugger.AttachPid == 0
@@ -323,6 +323,10 @@ func (s *ServerImpl) serveJSONCodec(conn io.ReadWriteCloser) {
 				s.log.Debugf("-> %T%s error: %q", replyv.Interface(), replyvbytes, errmsg)
 			}
 			s.sendResponse(sending, &req, &resp, replyv.Interface(), codec, errmsg)
+			if req.ServiceMethod == "RPCServer.Detach" && s.config.DisconnectChan != nil {
+				close(s.config.DisconnectChan)
+				s.config.DisconnectChan = nil
+			}
 		} else {
 			if logflags.RPC() {
 				argvbytes, _ := json.Marshal(argv.Interface())


Test patch:
diff --git a/service/dap/server_test.go b/service/dap/server_test.go
index 3faad362..ed36e94f 100644
--- a/service/dap/server_test.go
+++ b/service/dap/server_test.go
@@ -1579,6 +1579,46 @@ func TestGlobalScopeAndVariables(t *testing.T) {
 	})
 }
 
+// TestShadowedVariables executes to a breakpoint and checks the shadowed
+// variable is named correctly.
+func TestShadowedVariables(t *testing.T) {
+	runTest(t, "testshadow", func(client *daptest.Client, fixture protest.Fixture) {
+		runDebugSessionWithBPs(t, client, "launch",
+			// Launch
+			func() {
+				client.LaunchRequestWithArgs(map[string]interface{}{
+					"mode": "exec", "program": fixture.Path, "showGlobalVariables": true,
+				})
+			},
+			// Breakpoints are set within the program
+			fixture.Source, []int{},
+			[]onBreakpoint{{
+				// Stop at line 13
+				execute: func() {
+					client.StackTraceRequest(1, 0, 20)
+					stack := client.ExpectStackTraceResponse(t)
+					expectStackFrames(t, stack, "main.main", 13, 1000, 3, 3)
+
+					client.ScopesRequest(1000)
+					scopes := client.ExpectScopesResponse(t)
+					expectScope(t, scopes, 0, "Arguments", 1000)
+					expectScope(t, scopes, 1, "Locals", 1001)
+					expectScope(t, scopes, 2, "Globals (package main)", 1002)
+
+					client.VariablesRequest(1001)
+					locals := client.ExpectVariablesResponse(t)
+
+					expectVarExact(t, locals, 0, "(a)", "a", "0", !hasChildren)
+					expectVarExact(t, locals, 1, "a", "a", "1", !hasChildren)
+
+					// Check that the non-shadowed of "a" is returned from evaluate request.
+					validateEvaluateName(t, client, locals, 1)
+				},
+				disconnect: false,
+			}})
+	})
+}
+
 // Tests that 'stackTraceDepth' from LaunchRequest is parsed and passed to
 // stacktrace requests handlers.
 func TestLaunchRequestWithStackTraceDepth(t *testing.T) {
@@ -1808,7 +1848,7 @@ func TestWorkingDir(t *testing.T) {
 					"mode":        "exec",
 					"program":     fixture.Path,
 					"stopOnEntry": false,
-					"wd":          wd,
+					"cwd":         wd,
 				})
 			},
 			// Set breakpoints
@@ -3032,9 +3072,9 @@ func TestBadLaunchRequests(t *testing.T) {
 		client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug", "program": fixture.Source, "substitutePath": []interface{}{map[string]interface{}{"from": "path1", "to": 123}}})
 		expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t),
 			"Failed to launch: 'substitutePath' attribute '[map[from:path1 to:123]]' in debug configuration is not a []{'from': string, 'to': string}")
-		client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug", "program": fixture.Source, "wd": 123})
+		client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug", "program": fixture.Source, "cwd": 123})
 		expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t),
-			"Failed to launch: 'wd' attribute '123' in debug configuration is not a string.")
+			"Failed to launch: 'cwd' attribute '123' in debug configuration is not a string.")
 
 		// Skip detailed message checks for potentially different OS-specific errors.
 		client.LaunchRequest("exec", fixture.Path+"_does_not_exist", stopOnEntry)
@@ -3055,9 +3095,9 @@ func TestBadLaunchRequests(t *testing.T) {
 		expectFailedToLaunchWithMessage(client.ExpectErrorResponse(t), "Failed to launch: Build error: exit status 1")
 
 		// Bad "wd".
-		client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug", "program": fixture.Source, "noDebug": false, "wd": "dir/invalid"})
+		client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug", "program": fixture.Source, "noDebug": false, "cwd": "dir/invalid"})
 		expectFailedToLaunch(client.ExpectErrorResponse(t)) // invalid directory, the error message is system-dependent.
-		client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug", "program": fixture.Source, "noDebug": true, "wd": "dir/invalid"})
+		client.LaunchRequestWithArgs(map[string]interface{}{"mode": "debug", "program": fixture.Source, "noDebug": true, "cwd": "dir/invalid"})
 		expectFailedToLaunch(client.ExpectErrorResponse(t)) // invalid directory, the error message is system-dependent.
 
 		// We failed to launch the program. Make sure shutdown still works.
diff --git a/service/test/integration2_test.go b/service/test/integration2_test.go
index e7885b7f..ad36d42e 100644
--- a/service/test/integration2_test.go
+++ b/service/test/integration2_test.go
@@ -2318,3 +2318,34 @@ func TestToggleBreakpointRestart(t *testing.T) {
 		assertNoDuplicateBreakpoints(t, c)
 	})
 }
+
+func TestStopServerWithClosedListener(t *testing.T) {
+	// Checks that the error erturned by listener.Accept() is ignored when we
+	// are trying to shutdown. See issue #1633.
+	if testBackend == "rr" || buildMode == "pie" {
+		t.Skip("N/A")
+	}
+	listener, err := net.Listen("tcp", "localhost:0")
+	assertNoError(err, t, "listener")
+	fixture := protest.BuildFixture("math", 0)
+	server := rpccommon.NewServer(&service.Config{
+		Listener:           listener,
+		AcceptMulti:        false,
+		APIVersion:         2,
+		CheckLocalConnUser: true,
+		DisconnectChan:     make(chan struct{}),
+		ProcessArgs:        []string{fixture.Path},
+		Debugger: debugger.Config{
+			WorkingDir:  ".",
+			Backend:     "default",
+			Foreground:  false,
+			BuildFlags:  "",
+			ExecuteKind: debugger.ExecutingGeneratedFile,
+		},
+	})
+	assertNoError(server.Run(), t, "blah")
+	time.Sleep(1 * time.Second) // let server start
+	server.Stop()
+	listener.Close()
+	time.Sleep(1 * time.Second) // give time to server to panic
+}