cmd/dlv: add --continue to continue process on launch/attach #1585

briandealwis · 2019-06-18T20:02:29Z

Add --continue option for attach, debug, exec, and trace, to issue a continue on start.

Fixes #245

derekparker

This works technically, but there is an issue here. The main use case for this feature would be to start a headless delve server (for example within a container) and then attach to it. However when the headless server is running and I try to connect to it the client just hangs. It connects to the socket, but when it tries to make the initial RPC call to set API version it will wait forever for the server, which is waiting for the process, so the RPC call will never be addressed.

This patch needs to address this possibly by having the server detect a new connection and stopping the process if it is running so the user can start issuing commands, etc.

derekparker · 2019-06-18T22:03:05Z

service/debugger/debugger.go

@@ -57,6 +57,9 @@ type Config struct {
 	// attach.
 	AttachPid int

+	// ContinueOnStart determiend whether the new process should be paused on start.


nit: spelling error: s/determiend/determines/

derekparker · 2019-06-18T22:14:55Z

cc @aarzilli for your thoughts as well.

briandealwis · 2019-06-19T12:56:07Z

Sorry for wasting your time @derekparker — I hadn't tested this in an end-to-end situation.

aarzilli

This works technically, but there is an issue here. The main use case for this feature would be to start a headless delve server (for example within a container) and then attach to it. However when the headless server is running and I try to connect to it the client just hangs

The reason this happens is that the Continue call is done in debugger.New: the call never terminates and the server never finishes initializing and never starts actually accepting connections (the socket however is already open, which is why the connect doesn't fail completely).

The call to continue needs to happen at some point after the server initialization is finished.

aarzilli · 2019-06-19T15:04:21Z

service/debugger/debugger.go

@@ -99,6 +102,11 @@ func New(config *Config, processArgs []string) (*Debugger, error) {
 			return nil, attachErrorMessage(d.config.AttachPid, err)
 		}
 		d.target = p
+		if d.config.ContinueOnStart {
+			if _, err := d.target.ContinueOnce(); err != nil {


ContinueOnce shouldn't be called directly, it should be proc.Continue.

aarzilli · 2019-06-19T15:08:57Z

cmd/dlv/cmds/commands.go

@@ -240,6 +246,7 @@ to know what functions your process is executing.`,
 	traceCommand.Flags().BoolVarP(&traceTestBinary, "test", "t", false, "Trace a test binary.")
 	traceCommand.Flags().IntVarP(&traceStackDepth, "stack", "s", 0, "Show stack trace with given depth.")
 	traceCommand.Flags().String("output", "debug", "Output path for the binary.")
+	traceCommand.Flags().BoolVar(&ContinueOnStart, "continue", false, "Continue the traced process on start.")


The trace command always continues automatically, there's no need for this.

briandealwis · 2019-06-19T15:22:12Z

Thanks — I'll fix this up and do more comprehensive testing.

derekparker · 2019-06-24T15:04:04Z

Sorry for wasting your time @derekparker — I hadn't tested this in an end-to-end situation.

Not a waste of time, just doing my due diligence! I appreciate this PR as it's been a much requested one!

derekparker · 2019-07-02T18:00:41Z

@briandealwis ping on this.

briandealwis · 2019-07-02T20:08:19Z

Sorry, I was pulled away with holidays and other work. My attempt to use a client like the traceCmd hasn't worked out: dlv just exits. I'll be digging into this tomorrow.

briandealwis · 2019-07-08T14:13:59Z

So it seemed to make sense to limiting --continue to --headless since otherwise --init can be used. I got it to work with by forcing AcceptMulti and then using a separate Client instance to connect to the server and then issue a Disconnect(true). But the use of AcceptMulti means the delve server process stays around even after the actual process has exited.

I'm currently looking at whether I can do this at a lower level

briandealwis · 2019-07-08T14:19:18Z

Fatfingered the close button.

Would this be better done at lower levels the stack?

Unfortunately I can't seem to debug delve with delve on macOS to get a better understanding of what's going on: I get some ioctl error from the second lldb debugserver.

derekparker · 2019-07-08T17:58:29Z

My attempt to use a client like the traceCmd hasn't worked out: dlv just exits.

The trace command could be used as a starting point, I think. But I'm not sure what you mean by "dlv just exits". What does the process look like that you're testing against? Is it a server or something that is intended to be long running? Is there any output when delve exits? Is Delve exiting before the process does?

aarzilli · 2019-07-15T13:37:56Z

So it seemed to make sense to limiting --continue to --headless since otherwise --init can be used. I got it to work with by forcing AcceptMulti and then using a separate Client instance to connect to the server and then issue a Disconnect(true). But the use of AcceptMulti means the delve server process stays around even after the actual process has exited.

I'm currently looking at whether I can do this at a lower level

No, I think this level is the correct one. AcceptMulti is necessary because otherwise the whole thing would be useless (you'd have a process running under delve but no way to connect to delve itself).

But the use of AcceptMulti means the delve server process stays around even after the actual process has exited.

This is always the case, if you run a program under delve and the program terminates you go back to delve's prompt.

briandealwis · 2019-07-15T20:27:08Z

Sorry for lack of response: I was away at a lake and forbidden from touching a laptop :-)

I realize my comments above were from an odd behaviour I hit where dlv exec --headless --accept-multiclient of an app that exits normally during the session (like the example in _fixtures/buildtest/main.go) renders the headless dlv unkillable. I filed #1617.

This is always the case, if you run a program under delve and the program terminates you go back to delve's prompt.

I should have provided some context. My driving use-case is to leverage Delve to allow debugging containerized Go programs in a Kubernete cluster as part of Skaffold(tracked as GoogleContainerTools/skaffold#2306). skaffold debug rewrites the various Kubernetes resource objects to configure application containers for debugging as required for their language runtimes, and exposes the debug port so that IDEs can connect and debug programs in the cluster. Since the users may not want to connect to each and every container (e.g., in a multi-service setup), we want the containers to be debuggable (able to be connected to) but not paused waiting for a debugger. So for Go-based containers, that would launch the app with dlv exec --continue --headless --listen :xxxx --accept-multiclient .../app (for some port xxxx).

In the Kubernetes case, once the user has finished their debugging session and stops the application, then we want the container to exit so that Kubernetes can do its magic and relaunch the container. As I noticed with #1617, if the application has exited normally then the headless instance becomes unkillable (at least with the dlv terminal client), and the container appears to hang. (I suppose Kubernetes will eventually deem the container as being unhealthy and restart it?)

(I didn't test whether the RPC detach will properly cause the headless dlv process to exit.)

… instance

derekparker · 2019-07-16T20:13:41Z

Sorry for lack of response: I was away at a lake and forbidden from touching a laptop :-)

No worries, it's good to unplug for a bit!

Saw you pushed some updates but the tests are failing in CI. Does everything pass for you locally?

briandealwis · 2019-07-16T20:53:25Z

The tests are timing out on the TestContinue that I introduced earlier which tries to run

dlv debug --continue --headless --continue --accept-multiclient --listen :0

on the test in `_fixtures/buildtest/main.go.

But I just realized that test doesn't make sense in --headless. Oops.

aarzilli

Documentation/faq.md should be changed, to say at the end:

If you want the program to start immediately you can do that by passing the `--continue` option the dlv

aarzilli · 2019-07-18T12:26:15Z

service/config.go

@@ -13,6 +13,8 @@ type Config struct {
 	Listener net.Listener
 	// ProcessArgs are the arguments to launch a new process.
 	ProcessArgs []string
+	// ContinueOnStart determines whether the new process should be continued on start.
+	ContinueOnStart bool


Am I missing something or this isn't used?

aarzilli · 2019-07-18T12:26:25Z

service/debugger/debugger.go

@@ -57,6 +57,9 @@ type Config struct {
 	// attach.
 	AttachPid int

+	// ContinueOnStart determines whether the new process should be paused on start.
+	ContinueOnStart bool


Same for this.

derekparker

LGTM

derekparker · 2019-07-19T17:59:50Z

Thanks for the patch!

…e#1585) * Add --continue to continue process on launch/attach * Add small test of --continue * regenerate usage docs * minor cleanup * Use similar approach to `trace` and connect and detach using a client instance * back out previous attempt * regen usage doc * fix up continue test * fix TestContinue to properly test --continue * back out unnecessary changes * update faq

briandealwis added 3 commits June 18, 2019 15:59

Add --continue to continue process on launch/attach

474ff1f

Add small test of --continue

a5043fe

regenerate usage docs

e780de0

briandealwis changed the title ~~Add --continue to continue process on launch/attach~~ cmd/dlv: add --continue to continue process on launch/attach Jun 18, 2019

derekparker requested changes Jun 18, 2019

View reviewed changes

aarzilli reviewed Jun 19, 2019

View reviewed changes

briandealwis closed this Jul 8, 2019

briandealwis reopened this Jul 8, 2019

minor cleanup

8e2e6cd

briandealwis added 4 commits July 15, 2019 16:42

back out previous attempt

2ac9d38

Use similar approach to trace and connect and detach using a client…

b3f93b4

… instance

regen usage doc

7a89f2d

fix up continue test

951b0fc

briandealwis added 2 commits July 17, 2019 21:47

Merge remote-tracking branch 'origin/master' into HEAD

eaf06ae

fix TestContinue to properly test --continue

4901f27

aarzilli reviewed Jul 18, 2019

View reviewed changes

back out unnecessary changes

da4356a

update faq

27b59e8

derekparker approved these changes Jul 19, 2019

View reviewed changes

derekparker merged commit cb65877 into go-delve:master Jul 19, 2019

quoctruong mentioned this pull request Aug 8, 2019

Remote Attach fails to set a breakpoint if program is started with --continue option microsoft/vscode-go#2690

Closed

adibrastegarnia mentioned this pull request Aug 27, 2019

Add --continue to the debuger to continue the executation of onos-config onosproject/onos-config#655

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/dlv: add --continue to continue process on launch/attach #1585

cmd/dlv: add --continue to continue process on launch/attach #1585

briandealwis commented Jun 18, 2019

derekparker left a comment •

edited

Loading

derekparker Jun 18, 2019

derekparker commented Jun 18, 2019

briandealwis commented Jun 19, 2019

aarzilli left a comment

aarzilli Jun 19, 2019

aarzilli Jun 19, 2019

briandealwis commented Jun 19, 2019

derekparker commented Jun 24, 2019

derekparker commented Jul 2, 2019

briandealwis commented Jul 2, 2019

briandealwis commented Jul 8, 2019

briandealwis commented Jul 8, 2019

derekparker commented Jul 8, 2019

aarzilli commented Jul 15, 2019

briandealwis commented Jul 15, 2019

derekparker commented Jul 16, 2019

briandealwis commented Jul 16, 2019

aarzilli left a comment

aarzilli Jul 18, 2019

aarzilli Jul 18, 2019

derekparker left a comment

derekparker commented Jul 19, 2019

cmd/dlv: add --continue to continue process on launch/attach #1585

cmd/dlv: add --continue to continue process on launch/attach #1585

Conversation

briandealwis commented Jun 18, 2019

derekparker left a comment • edited Loading

Choose a reason for hiding this comment

derekparker Jun 18, 2019

Choose a reason for hiding this comment

derekparker commented Jun 18, 2019

briandealwis commented Jun 19, 2019

aarzilli left a comment

Choose a reason for hiding this comment

aarzilli Jun 19, 2019

Choose a reason for hiding this comment

aarzilli Jun 19, 2019

Choose a reason for hiding this comment

briandealwis commented Jun 19, 2019

derekparker commented Jun 24, 2019

derekparker commented Jul 2, 2019

briandealwis commented Jul 2, 2019

briandealwis commented Jul 8, 2019

briandealwis commented Jul 8, 2019

derekparker commented Jul 8, 2019

aarzilli commented Jul 15, 2019

briandealwis commented Jul 15, 2019

derekparker commented Jul 16, 2019

briandealwis commented Jul 16, 2019

aarzilli left a comment

Choose a reason for hiding this comment

aarzilli Jul 18, 2019

Choose a reason for hiding this comment

aarzilli Jul 18, 2019

Choose a reason for hiding this comment

derekparker left a comment

Choose a reason for hiding this comment

derekparker commented Jul 19, 2019

derekparker left a comment •

edited

Loading