[maybe][bug]History Crash Without Daemon

[fennewald]

Steps to reproduce

• Start elvish (ssh into Ubuntu server running elvish binary)
• Ignore warning Cannot connect to daemon: unexpected RPC error on socket /run/user/1014/elvish/sock: timeout
• Run any command
• arrow-up on next line to re-run
• elvish crash, stackdump, revert to sh

Error message is:

goroutine 1 [running]:
github.com/elves/elvish/pkg/sys.DumpStack(0xc00013a818, 0x8f7960)
	github.com/elves/elvish@/pkg/sys/dumpstack.go:10 +0x9d
github.com/elves/elvish/pkg/shell.rescue()
	github.com/elves/elvish@/pkg/shell/shell.go:100 +0x7d
panic(0x8f7960, 0xd9ded0)
	runtime/panic.go:679 +0x1b2
sync.(*RWMutex).RLock(...)
	sync/rwmutex.go:48
github.com/elves/elvish/pkg/cli/histutil.(*Fuser).Walker(0x0, 0x0, 0x0, 0x0, 0x0)
	github.com/elves/elvish@/pkg/cli/histutil/fuser.go:96 +0x48
github.com/elves/elvish/pkg/edit.histWalkStart(0xa590a0, 0xc0000b41c0, 0x0, 0xa49860, 0xc000172540)
	github.com/elves/elvish@/pkg/edit/histwalk.go:43 +0xb4
github.com/elves/elvish/pkg/edit.initHistWalk.func1()
	github.com/elves/elvish@/pkg/edit/histwalk.go:23 +0x4e
reflect.Value.call(0x8bfd00, 0xc0001725a0, 0x13, 0x9926cf, 0x4, 0x0, 0x0, 0x0, 0xc00013ad58, 0x436eff, ...)
	reflect/value.go:460 +0x5f6
reflect.Value.Call(0x8bfd00, 0xc0001725a0, 0x13, 0x0, 0x0, 0x0, 0xc00013ade0, 0x478850, 0x475e9c)
	reflect/value.go:321 +0xb4
github.com/elves/elvish/pkg/eval.(*GoFn).Call(0xc0000a5ab0, 0xc00022d650, 0x0, 0x0, 0x0, 0xc0000e2510, 0xc00013afe0, 0x78c946)
	github.com/elves/elvish@/pkg/eval/go_fn.go:235 +0x638
github.com/elves/elvish/pkg/edit.callWithNotifyPorts(0xa4e920, 0xc0000e8f40, 0xc000020ea0, 0xa489c0, 0xc0000a5ab0, 0x0, 0x0, 0x0)
	github.com/elves/elvish@/pkg/edit/key_binding.go:71 +0x23b
github.com/elves/elvish/pkg/edit.mapBinding.Handle(0xa4e920, 0xc0000e8f40, 0xc000020ea0, 0xc00008eae0, 0x1, 0x1, 0xa497c0, 0xc00007dc08, 0x40a26b)
	github.com/elves/elvish@/pkg/edit/key_binding.go:41 +0x1c8
github.com/elves/elvish/pkg/cli.(*codeArea).handleKeyEvent(0xc0000de300, 0xfffffc25, 0xc0000b4200)
	github.com/elves/elvish@/pkg/cli/codearea.go:289 +0x109
github.com/elves/elvish/pkg/cli.(*codeArea).Handle(0xc0000de300, 0xa497c0, 0xc00007dc00, 0x0)
	github.com/elves/elvish@/pkg/cli/codearea.go:164 +0x95
github.com/elves/elvish/pkg/cli.(*app).handle(0xc0000b41c0, 0x9307e0, 0xc00007dc00)
	github.com/elves/elvish@/pkg/cli/app.go:171 +0x26a
github.com/elves/elvish/pkg/cli.(*loop).Run(0xc000067a00, 0xc000068101, 0xc000068780, 0xc0000b41c0, 0xc00007dbb0)
	github.com/elves/elvish@/pkg/cli/loop.go:129 +0x230
github.com/elves/elvish/pkg/cli.(*app).ReadCode(0xc0000b41c0, 0x0, 0x0, 0x0, 0x0)
	github.com/elves/elvish@/pkg/cli/app.go:334 +0x4eb
github.com/elves/elvish/pkg/edit.(*Editor).ReadCode(0xc0000e8f40, 0xc00007e010, 0xa489a0, 0xc0002509c0, 0xc)
	github.com/elves/elvish@/pkg/edit/editor.go:115 +0x33
github.com/elves/elvish/pkg/shell.Interact(0xc00007e000, 0xc00007e008, 0xc00007e010, 0xc00013bc50)
	github.com/elves/elvish@/pkg/shell/interact.go:60 +0x3ba
github.com/elves/elvish/pkg/shell.program.Run(0xc00007e000, 0xc00007e008, 0xc00007e010, 0xc00012a000, 0xc00006a190, 0x0, 0x0, 0xdcbba0, 0xc0000b1f40)
	github.com/elves/elvish@/pkg/shell/shell.go:40 +0x2bc
github.com/elves/elvish/pkg/prog.Run(0xc00007e000, 0xc00007e008, 0xc00007e010, 0xc00006a190, 0x1, 0x1, 0xc0000b1f20, 0x3, 0x3, 0x0)
	github.com/elves/elvish@/pkg/prog/prog.go:134 +0x295
main.main()
	github.com/elves/elvish@/main.go:17 +0x11f

goroutine 5 [syscall]:
os/signal.signal_recv(0xa4fd20)
	runtime/sigqueue.go:147 +0x9c
os/signal.loop()
	os/signal/signal_unix.go:23 +0x22
created by os/signal.init.0
	os/signal/signal_unix.go:29 +0x41

goroutine 6 [chan receive]:
github.com/elves/elvish/pkg/eval.getBlackholeChan.func1(0xc0000260c0)
	github.com/elves/elvish@/pkg/eval/port.go:59 +0x47
created by github.com/elves/elvish/pkg/eval.getBlackholeChan
	github.com/elves/elvish@/pkg/eval/port.go:58 +0x58

goroutine 9 [chan receive]:
github.com/elves/elvish/pkg/shell.setupShell.func1(0xc000026120, 0xc00007e000, 0xc00007e008, 0xc00007e010)
	github.com/elves/elvish@/pkg/shell/shell.go:52 +0xc8
created by github.com/elves/elvish/pkg/shell.setupShell
	github.com/elves/elvish@/pkg/shell/shell.go:51 +0x12d

goroutine 19 [chan receive]:
github.com/elves/elvish/pkg/cli/prompt.(*Prompt).loop(0xc0000a4380)
	github.com/elves/elvish@/pkg/cli/prompt/prompt.go:77 +0xc8
created by github.com/elves/elvish/pkg/cli/prompt.New
	github.com/elves/elvish@/pkg/cli/prompt/prompt.go:70 +0x15f

goroutine 20 [chan receive]:
github.com/elves/elvish/pkg/cli/prompt.(*Prompt).loop(0xc0000a4460)
	github.com/elves/elvish@/pkg/cli/prompt/prompt.go:77 +0xc8
created by github.com/elves/elvish/pkg/cli/prompt.New
	github.com/elves/elvish@/pkg/cli/prompt/prompt.go:70 +0x15f

runtime error: invalid memory address or nil pointer dereference
k
Execing recovery shell /bin/sh

I know this would not occur if the daemon was working. I'm trying to fix that right now, nut I opened this issue because a total crash seems a bit extreme for "daemon related functions may not work"

[krader1961]

I wonder if this failure is related to the fix for issue #909. Regardless, this is yet another example why the Elvish daemon should be eliminated. The daemon increases project complexity and adds failure modes. I have no idea why BoltDB doesn't support concurrent access by different processes. I'd be willing to bet it is a consequence maximizing performance being a primary goal.

Other shells, which includes bash, ksh, zsh and fish, all manage to have an interactive history file that does not require the use of a daemon process to read or write the interactive history. Granted, those shells only store information about interactive commands; whereas elvish includes additional information such as the "location" mode data. Nonetheless, I think elvish would be better off using a history mechanism that allows eliminating the elvish daemon.

One option would be to switch to text based history files. This is what other shells use for interactive commands. By creating a separate file for the location data elvish could use one of the algorithms used by those shells for its interactive history; both command and location. Another option is to switch to a pure Go database implementation such as SQLite. See issue #126.

Having said all that, obviously this panic is unacceptable and should be fixed.

P.S., See also issue #435 from 3.5 years ago that switched from SQLite3 to BoltDB to eliminate the dependency on cgo. It looks like the daemon was already present at that time. Why the daemon was present 3.5 years ago is not clear since I believe SQLite3 at that time supported reading/writing the database by multiple processes.

[krader1961]

Note that the module that causes the panic was removed six months ago by commit 3501b94. So unless there is some reason to think this failure can still occur this should probably be closed as no longer relevant.

[fennewald]

@krader1961 this occurred on a binary I grabbed from the most recent github release. I downloaded it <24 hours ago. Maybe still an issue?

[krader1961]

@fennewald, What does elvish -buildinfo output? I cannot reproduce your problem. The official download version of the program will almost always be older than the current code base. Which is why it is important to note which version of the elvish program you are running.

[krader1961]

Clarification: The file that appears in the panic stack that was removed six months ago is github.com/elves/elvish@/pkg/cli/histutil/fuser.go. Since that file has not been part of the elvish code for a half year it seems likely, @fennewald, that you're running elvish 0.14.0 or older -- not an elvish binary built from the current development branch; i.e., the master branch.

[fennewald]

Yeah, buildinfo is:

Version: v0.14.1
Go version: go1.13.5
Reproducible build: true

I pulled it from the elvish website (Linux x86_64) on January 12th.

[zzamboni]

@fennewald Version 0.14.1 has been released on 2020-08-16, you need to test against the HEAD binary downloadable from the same page.

[xiaq]

Hi, @fennewald. Only HEAD has first-class support at the moment, and since this bug is not present on HEAD I'm closing this.

@krader1961 I grant that the daemon makes things more complex, but if you want to have it eliminated, there needs to be a concrete proposal for replacing it. Feel free to open another issue to discuss it.

[xiaq]

@krader1961 I looked into the sqlite package you pointed to and filed #1222.