Should a defensive copy of the tasks list be made?

[js-ojus]

Tasqueue/server.go

Line 150 in 42e9432

s.p.RUnlock()

The Go Language Reference document Section "Assignment statements" says:

"When a value is assigned to a variable, only the data that is stored in the variable is replaced. If the value contains a reference, the assignment copies the reference but does not make a copy of the referenced data (such as the underlying array of a slice)."

Would moving s.p.RUnlock() to after the loop be safer?

// GetTasks() returns a list of all tasks registered with the server.
func (s *Server) GetTasks() []string {
	s.p.RLock()
	tasks := s.tasks

	var (
		t = make([]string, len(tasks))
		i = 0
	)
	for k := range tasks {
		t[i] = k
		i++
	}
	s.p.RUnlock()

	return t
}

[kalbhor]

Thanks for pointing this out, you're right, the current implementation can panic if any new tasks are registered concurrently while GetTasks is called.

Below is a slightly modified version of your patch. We could just directly reference s.tasks and use append after defining the length and capacity of the slice.

Would you like to open a PR for this?

// GetTasks() returns a list of all tasks registered with the server.
func (s *Server) GetTasks() []string {
	s.p.RLock()
        defer s.p.RUnlock()

	t := make([]string, 0, len(s.tasks))
	for k := range tasks {
		t = append(t, k)
	}

	return t
}

[js-ojus]

Thanks for your quick response. Done. Tests pass.

#55