Panic! about libvirt-go

[berrange]

Have you perhaps kept a copy of the "*Connect" after some other bit of your code has called Close() on it ?

[purpleidea]

On Mon, Feb 6, 2017 at 5:20 AM, Daniel Berrange ***@***.***> wrote: Have you perhaps kept a copy of the "*Connect" after some other bit of your code has called Close() on it ?

This is possible. I'm trying to track down a possible bug (on my side) that might have caused this, but I'm hoping it's not me. If I do something to the conn after I've called close will it cause this? I got the feeling that maybe the mainloop impl thing kept running after I was closed, maybe there is a better way I should be shutting it down? To be continued... Comments welcome!

[berrange]

The main event loop is independent of any single libvirt connection. Your stack trace shows its triggering via runtime.goexit() so I'd guess you have a race condition with variable cleanp somewhere.

[purpleidea]

@berrange I've got a few bugs in my code which might have caused this which I am working on fixing. I'm going to assume this is entirely my fault for now, and will re-open if I can prove otherwise.

Thanks again!

[purpleidea]

I'd like to re-open this for now, since I've done a bunch of code cleanup, and I've narrowed something similar down to:

1. A bug in libvirt
2. A bug in mgmt (getting less confident that it's this)
3. A misunderstanding about how something should be working, therefore possibly causing this issue inadvertently.

Firstly, I'm assuming that any callback that we register, eg:

	domCallback := func(c *libvirt.Connect, d *libvirt.Domain, ev *libvirt.DomainEventLifecycle) {
		domName, _ := d.GetName()
		if domName == obj.GetName() {
			select {
			case domChan <- ev.Event: // send
			case <-exitChan:
			}
		}
	}
	// if dom is nil, we get events for *all* domains!
	domCallbackID, err := obj.conn.DomainEventLifecycleRegister(nil, domCallback)

actually gets called in the internal main loop where we call:

	if err := libvirt.EventRunDefaultImpl(); err != nil {

(which I assume internally processes and runs the callbacks...) Is this correct?

Secondly, when I run the unregister stuff, eg:

obj.conn.DomainEventDeregister(domCallbackID)

Is it possible that if I don't then give the EventRunDefaultImpl a chance to pump through this once, that when I run conn.Close() at the end it's upset that something wasn't shutdown cleanly and generates that signal panic?

Lastly, if I have more than one EventRunDefaultImpl running (eg each vm has it's own connection and go routine for calling that method), is there any problem with that that would cause such a panic?

Here is a more current form of the panic:

fatal error: unexpected signal during runtime execution
[signal 0xb code=0x1 addr=0x7f4854000fa8 pc=0x7f4bf3609559]

runtime stack:
runtime.throw(0x169c760, 0x2a)
	/usr/lib/golang/src/runtime/panic.go:547 +0x90
runtime.sigpanic()
	/usr/lib/golang/src/runtime/sigpanic_unix.go:12 +0x5a

goroutine 1259 [syscall, locked to thread]:
runtime.cgocall(0xf73ee0, 0xc42087ce20, 0xc400000000)
	/usr/lib/golang/src/runtime/cgocall.go:123 +0x11b fp=0xc42087cdf8 sp=0xc42087cdc8
github.com/libvirt/libvirt-go._Cfunc_virEventRunDefaultImpl(0xc400000000)
	??:0 +0x45 fp=0xc42087ce20 sp=0xc42087cdf8
github.com/libvirt/libvirt-go.EventRunDefaultImpl(0x0, 0x0)
	/home/james/code/gopath/src/github.com/libvirt/libvirt-go/events.go:54 +0x28 fp=0xc42087ce88 sp=0xc42087ce20
github.com/purpleidea/mgmt/resources.(*VirtRes).Watch.func1(0xc4206b8280, 0xc420634170, 0xc4208bf4a0, 0xc4208bf440)
	/home/james/code/gopath/src/github.com/purpleidea/mgmt/resources/virt.go:288 +0x10c fp=0xc42087cf90 sp=0xc42087ce88
runtime.goexit()
	/usr/lib/golang/src/runtime/asm_amd64.s:1998 +0x1 fp=0xc42087cf98 sp=0xc42087cf90
created by github.com/purpleidea/mgmt/resources.(*VirtRes).Watch
	/home/james/code/gopath/src/github.com/purpleidea/mgmt/resources/virt.go:298 +0x1ca

As an aside, I occasionally get this sort of message in my logs:

2017-02-23 03:55:25.303+0000: 29501: info : libvirt version: 1.3.3.2, package: 1.fc24 (Fedora Project, 2016-07-19-00:36:57, buildvm-25.phx2.fedoraproject.org)
2017-02-23 03:55:25.303+0000: 29501: info : hostname: freed
2017-02-23 03:55:25.303+0000: 29501: warning : virEventPollUpdateHandle:172 : Got update for non-existent handle watch 4
2017-02-23 03:55:25.469+0000: 29501: warning : virEventPollUpdateHandle:172 : Got update for non-existent handle watch 4
2017-02-23 03:55:25.470+0000: 29501: warning : virEventPollUpdateHandle:172 : Got update for non-existent handle watch 4
2017-02-23 03:55:25.470+0000: 29501: warning : virEventPollUpdateHandle:172 : Got update for non-existent handle watch 4

Any I have no idea what's causing or sending that either. Comments welcome!

Thanks

[purpleidea]

As a followup, would calling of

libvirt.EventRegisterDefaultImpl

more than once cause harm, and does this have a matching close?

[purpleidea]

(as an aside, I'm able to repro this 100%)

git checkout feat/cockpit from https://github.com/purpleidea/mgmt/
git rebase master
make clean build
sudo systemctl restart cockpit
go here: http://localhost:9090/mgmt/mgmt
./mgmt run --yaml cockpit/mgmt.yaml --tmp-prefix --no-pgp
(it will spin up between one and 5 vm's - 512mb, no disks)
now click the "live" checkbox in the website. Slide the slider back and forth very quickly until mgmt panics.

[berrange]

Is it possible that if I don't then give the EventRunDefaultImpl a chance to pump
through this once, that when I run conn.Close() at the end it's upset that something
wasn't shutdown cleanly and generates that signal panic?

You should not make any assumption about interaction or synchronization between EventRunDefaultImpl and event de-registration. After registering an event loop, it is required that this be run continuously in the process, essentially forever. If this doesn't happen you'll at least get dangling references leading to memory leaks.

Lastly, if I have more than one EventRunDefaultImpl running (eg each vm has it's own connection
and go routine for calling that method), is there any problem with that that would cause such a
panic?

Running the EventRunDefaultImpl multiple times from different threads is a bug. The event loop has a global state - there's no separation between different connections. That said, there is locking on the default event loop impl, so you shouldn't get corruption, but what it means is that you'll get events randomly serviced by different threads. If your application locking model isn't robust your app could corrupt its state when then handling events.

Essentially your code should just have a package somewhere that just does this:

func eventloop() {
for {
libvirt.EventRunDefaultImpl()
}
}

func init() {
libvirt.EventRegisterDefaultImpl()
go eventloop()
}

[berrange]

Closing because this sounds like an app bug. Please reopen if you can identify an isolated case to reproduce the problem