Potential race condition with the in memory store.

[elliotcourant]

I hope I am not creating clutter with this.

I think there is the potential for a race condition when a job is started at the same time that Shutdown(...) is called for the in memory store.

When shutdown is called it reads the context array without a mutex, which would be in a different go routine than the workers themselves that own the context. A context could be appended to this array while shutdown is being performed.

https://github.com/acaloiaro/neoq/blob/a17ea73f6e381c462cba4aabf01cbc854cb3b5a3/backends/memory/memory_backend.go#L193-L199

But a mutex is used when the contexts are added.

https://github.com/acaloiaro/neoq/blob/a17ea73f6e381c462cba4aabf01cbc854cb3b5a3/backends/memory/memory_backend.go#L139-L141

1. I think the mutex should be changed to a RWMutex with Shutdown acquiring a read lock on the array, and start acquiring a write lock on the array. This would prevent start from adding a new context and thus starting a new job during a shutdown. But does not prevent a job from starting immediately afterwards.
2. Do the contexts ever get cleaned up from this array? I don't see anything immediately that would remove items from the array after a job has completed. While I doubt these are of significant overhead would it be worth looking into a way to clean them up afterwards as well?

Happy to take on this work myself and contribute, really excited to start using this tool!

[elliotcourant]

I see that the context is only created per handler, not per job. So its very unlikely that a handler will "start" after the shutdown. And the likely-hood of a handler being added during a shutdown is also very low. Happy to tweak this in a PR or to just close it!

[acaloiaro]

I think you're right about this.

If it's not too tedious to trigger it with a test, I think that'd be a good start.

[elliotcourant]

Do you think if start was called on a handler after shutdown that some error should be raised or a panic?