dominikbraun / graph

A library for creating generic graph data structures and modifying, analyzing, and visualizing them.
https://graph.dominikbraun.io
Apache License 2.0
1.82k stars 96 forks source link

AdjacencyMap may cause panic #102

Open liuxiaomeiG opened 1 year ago

liuxiaomeiG commented 1 year ago

Concurrent calls to AddVertex, AddEdge, and AdjacencyMap redundantly panic.

panic: assignment to entry in nil map

goroutine 1996 [running]:
github.com/dominikbraun/graph.(*undirected[...]).AdjacencyMap(0xc0009542c0?)
    github.com/dominikbraun/graph@v0.16.2/undirected.go:167 +0x225

image

ListVertices and ListEdges directly occur AddVertex and AddEdge will cause this problem

davidovich commented 1 year ago

We have experienced this also in a non concurrent environment. PredecessorMap() is guarded against nil access, I wonder if it was intended to leave it out in AdjacencyMap(). If not, would you accept a PR to add an initialization if the edge.Target is not found ?

dominikbraun commented 1 year ago

@davidovich Yes. AdjacencyMap should behave exactly as PredecessorMap since these functions do the same thing but complement each other.

davidovich commented 1 year ago

While investigating this issue, it appeared to me that the code is correct on assuming the presence of a vertex, hence it would be, IMHO, a bad fix to just initialize the map on an un-existing source vertex because that would create a degenerate edge.

In our implementation of the store interface, we hit a panic because the store was returning edges with vertices that did not exist. So I went and opened a PR for reporting that error instead of panicking.