[WIP] Fix data race caused by updating map references

[typeless]

For go-gitea/gitea#5189

[mschoch]

Thanks for starting this. I don't think we were aware of any concurrency issue. However, before we go further with a sync.Map solution, I think we need to better understand the problem. When sync.Map was introduced the Go team was very clear that it is not a general purpose solution, so we would have to better understand the nature of the concurrent access before choosing it as the solution.

I think we should first check the usage and ensure it is correct (objects like batches can sometimes be misused in a way that causes concurrency problems. I have no reason to think this is the case, just that we should review it first. Second, we'll need to analyze which goroutines are involved in the concurrent access and which data structures are involved, to see if sync.Map is a good fit in this case.

If you've already done this analysis, please share it here.

[typeless]

@mschoch I agreed. The use of sync.Map could be an overkill.

About the analysis of the problem, please see go-gitea/gitea#5217 (comment) and the backtrace at go-gitea/gitea#5189

[typeless]

A test has been added.

I haven't tried it, but I wonder if the Batch.Reset method can just clear the map using delete with a for-loop rather than replacing with a new map object. It would be much simpler than the whole wrapper thing and lock.

Also sync.Map has been dropped in favor of vanilla map.

[typeless]

The endeavor exceeds what I've expected. There is a working (which passes CI) version using sync.Map but I'm afraid there would be some amount of performance impact and the changes are too invasive to my taste. So, I'm going to give up this in favor of #1038 .