Cron:Entries() may return nil when called concurrently

[mrwonko]

This is a theoretical issue I have not yet encountered in practice (but think could actually happen) that I came across while thinking about the source code, in particular these bits:

// Entries returns a snapshot of the cron entries.
func (c *Cron) Entries() []*Entry {
	if c.running {
		c.snapshot <- nil // A
		x := <-c.snapshot // B
		return x
	}
	return c.entrySnapshot()
}

		case <-c.snapshot: // C
			c.snapshot <- c.entrySnapshot() // D

Let's say Goroutine 1 is executing the run() loop containing C and D. Now Goroutine 2 executes Entries() until A, where it blocks. Goroutine 1 executes C, but before it gets a chance to execute D the scheduler decides to preempt it in favour of Goroutine 3, which executes Entries() until A, where it puts nil into the snapshot channel, leading Goroutine 2 to receive nil in B!

Since entrySnapshot() is a non-trivial function I feel like there's a good chance the Goroutine might get preempted during its execution.

The issue lies in using the same channel for communication in both directions. You could instead have a channel of channels over which you send a result channel created in Entries().

[monotone]

go func() {
	<-c
	c <- 3
}()

go func() {
	c <- 0
	t.Log("2:", <-c)
}()

c <- 0
t.Log("1:", <-c)

Is this issue like this code snap? it could print different value for channel c.

I think the cron object must be used in just one Goroutine. the Start() function is not Thread safety obviously

[robfig]

I agree with you, good find! I think using the same channel for requests and responses is not a good design either. I believe I can fix this easily, as you say by using a channel of channels.

[robfig]

Fixed on v3