samber / lo

💥 A Lodash-style Go library based on Go 1.18+ Generics (map, filter, contains, find...)
https://pkg.go.dev/github.com/samber/lo
MIT License
17.47k stars 802 forks source link

lop.ForEach #326

Open JC1738 opened 1 year ago

JC1738 commented 1 year ago

99% of the time all results are created, but 1% of the time 1 or 2 routines get dropped.

I have a slice of 100 that combines a structure from various variables and appends it to a new slice. There is no error, but running this method a lot I have consistently noticed that the end of the run the new slice has either 98, 99, or 100 elements. I would understand if there is an error, but it seems like the blocking occasionally doesn't wait long enough? Is there a way to force to wait till all routines finish? When I switch to lo.ForEach it always works and end result is a slice of 100.

Does the append to slice need some type of mutex? Didn't think it wasn't thread safe, but maybe not?

Thanks for suggestions.

GRbit commented 1 year ago

Can you please post an example of a code that can demonstrate the problem? Looking at ForEach in the lo package it doesn't look like it can cause a problem like this. Still, I think it'll be easier for somebody to help if you post your code.

JC1738 commented 1 year ago

@GRbit //This always works, total will be the full count of orders (if orders == 100, total == 100) lo.ForEach(orders, func(o order, index int) { pack := OrderStruct{ Order: o, OrderItems: getOrderItems(o.ID), } total = append(total, pack) })

//This sometimes returns a total with 99, or 98, or sometimes 100 lop.ForEach(orders, func(o order, index int) { pack := OrderStruct{ Order: o, OrderItems: getOrderItems(o.ID), } total = append(total, pack) })

The Only difference in the code is lop vs lo (|| or not), running the above both ways for 1000 times you can see the lo works all 1000 times, the lop will be accurate ~90% of the time.

GRbit commented 1 year ago

@JC1738 thank you for the code snippet.

Looking at these couple of lines, it looks like you don't really handle parallel appends to the total slice. Since you append in parallel, you can lost some values. If you want it to work with "lop" you need a mutex to not work with total from two different goroutines.

Also, it would be just great if you could provide a link to go playground with a fully working example. It really help anyone who would like to answer your questions.

It looks for me like "lop" package is totally fine.

JC1738 commented 1 year ago

Thanks, I was wondering if append was thread safe, sounds like that is what I need to do. Thanks for the tip.