Is it concurrency safe? about dijkstra HOT 9 CLOSED

I think it isn't... I am having 3 Shortest requests that succeed if i call them sequentially, but fail if I call them concurrently. @RyanCarrier can you confirm/infirm that it is concurrency-safe? Maybe I am doing something wrong...

from dijkstra.

Having a look :)

from dijkstra.

If you mean call shortest while one is still processing, IE; have one graph, and calling shortest on it 3 times before the first has finished, yeah that will fail. it keeps it's state information within the graph itself, this is to not waste time when updating it, or say if an arc updates and you don't want everything to have to fully reprocess

If you mean have two graphs working at the same time, I have no idea why it wouldn't be working concurrently but I'll check incase I'm crazy...

from dijkstra.

Could pretty easily adjust to make the state held in the shortest method, but yeah then we lose state information for following runs of shortest which I think would be a pretty net negative?

Am I understanding correctly?

Would it be best to return an error if shortest is already running? Or reset it?

from dijkstra.

Oof sorry I mean, keeps state information so you can go through and check what's happening, it doesn't do partial updates.

from dijkstra.

Have set it now that it will return error if you run while it's already calculating. Let me know if I've missinterpreted, also added tests for concurrent testing of different graphs just to make sure that's not what you meant.

from dijkstra.

Update to v1.2.0 for fix

from dijkstra.

it keeps it's state information within the graph itself, this is to not waste time when updating it, or say if an arc updates and you don't want everything to have to fully reprocess

I'm unfamiliar with the innards of the dijkstra repo code, so take my thoughts with a grain of salt, but could we put a sync.RWMutex on the Graph struct so that operations updating state information in the graph acquire a write lock first before modifying. Then readers acquire a read lock to allow for multiple concurrent readers that aren't changing state information?

from dijkstra.

Yeah you could, but if you are at the point and you are doing the requesting and reading in different threads/routines, you would probably be implementing that yourself, so like; calculate, wait for result, pass result to other thread.

I don't think it makes sense to add a lock to the graph itself, more so to your use case of it as I feel like the default and normal way would be to define graph, then get the resulting path.

Could be as simple as like go ()func{resultChannel<-graph.Shortest()}()

I feel like I'm probably missing something here though, so if you have some source code I can look at and see the problem you're dealing with I might agree with you

from dijkstra.