Panic in drone-runner-exec v1.0.0-beta.4

Drone Support
1

Builds appeared to be running successfully and then randomly hung. I discovered the exec running had paniced.

Running v1.0.0-beta.4 on Ubuntu 18.04.

drone-runner-exec daemon ~/Development/drone/runner-exec.config

➜  Development cat ~/Development/drone/runner-exec.config
DRONE_RPC_PROTO=http
DRONE_RPC_HOST=foobar
DRONE_RPC_SECRET=f09adds3c5c36cf4e6246b8b3
DRONE_LOG_FILE=/home/foobar/Development/drone/drone-runner-exec.log
DRONE_HTTP_PROTO=http
DRONE_HTTP_HOST=foobar:3000
DRONE_TRACE=true
DRONE_DEBUG=true
DRONE_RPC_DUMP_HTTP=true
DRONE_RPC_DUMP_HTTP_BODY=true

panic: close of closed channel

goroutine 706 [running]:
github.com/drone/runner-go/livelog.(*Writer).stop(0xc0000b9c20, 0xc002cfa201)
	/root/go/pkg/mod/github.com/drone/runner-go@v1.1.1-0.20190716195006-0bfd1dd0dea7/livelog/livelog.go:147 +0x70
github.com/drone/runner-go/livelog.(*Writer).Write(0xc0000b9c20, 0xc0001a6000, 0xfa7, 0x8000, 0xfa7, 0x0, 0x0)
	/root/go/pkg/mod/github.com/drone/runner-go@v1.1.1-0.20190716195006-0bfd1dd0dea7/livelog/livelog.go:78 +0xb1
io.copyBuffer(0x7f164db59880, 0xc0000b9c20, 0xa76480, 0xc00029a168, 0xc0001a6000, 0x8000, 0x8000, 0xc000278210, 0xc00022d1a0, 0x0)
	/usr/local/go/src/io/io.go:404 +0x1fb
io.Copy(...)
	/usr/local/go/src/io/io.go:364
os/exec.(*Cmd).writerDescriptor.func1(0x0, 0x0)
	/usr/local/go/src/os/exec/exec.go:288 +0x63
os/exec.(*Cmd).Start.func1(0xc0000bda20, 0xc00000d160)
	/usr/local/go/src/os/exec/exec.go:409 +0x27
created by os/exec.(*Cmd).Start
	/usr/local/go/src/os/exec/exec.go:408 +0x58f
2

I think I found it: https://github.com/drone/runner-go/blob/v1.2.0/livelog/livelog.go

func (b *Writer) stop() bool {
	b.Lock()
	var closed bool
	if b.closed == false {
		close(b.close)
		closed = true
	}
	b.Unlock()
	return closed
}

Note b.closed does not get set to true after a successful call to stop(), so a subsequent call will encounter this panic.

3

hey there, thanks for the report.

Note b.closed does not get set to true after a successful call to stop() , so a subsequent call will encounter this panic.

hmm … the stop method is wrapped in a mutex so it should not be possible to close the same channel twice.

4

It’s not a concurrency issue. Stop is just not idempotent - it will panic on second and subsequent calls

5

ah I see the issue … it is subtle

func (b *Writer) stop() bool {
	b.Lock()
	var closed bool
	if b.closed == false {
		close(b.close)
		closed = true
+		b.closed = true
	}
	b.Unlock()
	return closed
}
6

Yup exactly! Great product btw.

7

And for some reason this post has been flagged as spam and hidden. Thanks discourse.

8

Yup exactly! Great product btw.

thanks! and thank you for reporting and taking time to point me to the problematic section of code. I pushed a patch to runner-go and will post an updated release to the exec runner within the next 24 hours.

9

Thanks a lot - what a great response and turn around :+1: