cmd/pprof: Control-C does not exit interactive mode

[mvdan]

What version of Go are you using (go version)?

go version devel +f4e4ec2cd0 Wed Aug 22 06:54:03 2018 +0000 linux/amd64

Does this issue reproduce with the latest release?

No - works fine on go version go1.10.3 linux/amd64.

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/mvdan/go/cache"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/mvdan/go/land:/home/mvdan/go"
GOPROXY=""
GORACE=""
GOROOT="/home/mvdan/tip"
GOTMPDIR=""
GOTOOLDIR="/home/mvdan/tip/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build845884737=/tmp/go-build -gno-record-gcc-switches"

What did you do?

$ go tool pprof cpu.out
File: json.test
Type: cpu
Time: Aug 22, 2018 at 1:52pm (BST)
Duration: 7.05s, Total samples = 20.59s (292.15%)
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) # hit control-C

What did you expect to see?

The same that I see via Go 1.10:

$ go1 tool pprof cpu.out
File: json.test
Type: cpu
Time: Aug 22, 2018 at 1:52pm (BST)
Duration: 7.05s, Total samples = 20.59s (292.15%)
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) ^C
$

What did you see instead?

^C having no effect whatsoever. The only way to exit is now to type exit, or ^D (EOF).

I presume this is a regression in 1.11, likely because of all the readline changes that were retrofitted into go tool pprof. cc @hyangah

Milestoning for 1.12 for now, but this is a regression in a way.

[mvdan]

Another weird behavior is that pressing ^D only has an effect if there's an empty command. That is, writing top^D doesn't actually run the top command either. Although this doesn't work on 1.10 either.

[hyangah]

Yes, that's the regression due to the readline changes.
I tried to replace the terminal package with github.com/chzyer/readline during 1.11
cycle in order to address this regression, but it was late in the dev cycle and chzyer/readline
had a few other issues to be addressed so I just gave up.

I think it's desirable for the upstream pprof and the go tool pprof to to go with the
same readline package. But I don't know if that will be the current terminal package or
other 3rd party readline packages. Anyway, it should be decided before Go1.12.

[mvdan]

We obviously missed this for 1.12 :) These little readline issues bite me at least once a week - I'd almost give up the possibility to walk up the history to just fix these bugs. So I do think we should fix it for 1.13.

Remind me - what's the issue with golang.org/x/crypto/ssh/terminal? I find it a bit weird that the package is under crypto/ssh, but otherwise it's a terminal package, so shouldn't we just fix these bugs there?

Also, I assume that the reason we don't want a third-party terminal package inside Go is bloat. Why not go the other way, and have the third-party pprof tool use the same readline library as the one in cmd/pprof?

[hyangah]

Update: I gave up my original plan of using the third-party terminal package. Sent a PR to upstream to be compatible with our builders and didn't get responses. I now think depending on third-party packages that we can't fix quickly doesn't seem like a good idea.

Let's fix golang.org/x/crypto/ssh/terminal. My concern on this package was, it's minimal and has been a less popular choice among go users - meaning less chances of being tested. But at least, it allows us to maintain the source and fix bugs promptly.

[hyangah]

Sorry for the delay.
I believe we have now a better story about importing 3rd party dependencies, so I am proposing to retry to switch to github.com/chzyer/readline like the upstream.

[gopherbot]

Change https://golang.org/cl/222342 mentions this issue: cmd/pprof: use github.com/chzyer/readline for readline support