[BUG]: Crash when stopping gossip

Is there an existing issue for this?

[X] I have searched the existing issues

Current behavior

The p2p node crashed with SIGSEGV and stack trace, here's the end of the logs:

2022-09-21 15:36:08.781367 (p2p.Koinos) [p2p/gossip.go:404] <info>: Gossiped transaction applied - ID: 0x1220cdb0d0c54be0fa9ca548e9d7794f21c559b2110fc7a05d634e4d49d14a9e26d8 from peer Qmb...
2022-09-21 15:36:08.781501 (p2p.Koinos) [p2p/gossip.go:404] <info>: Gossiped transaction applied - ID: 0x1220246530b47ddf885ae8cd66142402940d8553378d52ad49b74d9230b7ff2cf2a4 from peer Qmb...
2022-09-21 15:36:08.792164 (p2p.Koinos) [p2p/gossip.go:404] <info>: Gossiped transaction applied - ID: 0x1220a9f28b6a02237ad47bf83926edd1e35db97ea6bf644b6e7e24e675273df03101 from peer Qmb...
2022-09-21 15:36:08.794310 (p2p.Koinos) [p2p/gossip.go:404] <info>: Gossiped transaction applied - ID: 0x122069ca9766d89fb204648fac3cdf0c1e14228b4f60f9e6d321195629dbc5890a35 from peer Qmb...
2022-09-21 15:36:08.794423 (p2p.Koinos) [p2p/gossip.go:208] <info>: Stopping gossip mode
2022-09-21 15:36:08.794746 (p2p.Koinos) [p2p/gossip.go:132] <warn>: Error getting message for topic koinos.blocks: context canceled
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0xa88b4d]

goroutine 1068775 [running]:
github.com/libp2p/go-libp2p-pubsub.(*Subscription).Next(0x0, {0x1543b28, 0xc00306adc0})
        /go/pkg/mod/github.com/libp2p/go-libp2p-pubsub@v0.7.1/subscription.go:27 +0x2d
github.com/koinos/koinos-p2p/internal/p2p.(*GossipManager).readMessages(0xc000330f50, {0x1543b28, 0xc00306adc0}, 0xc004edea80)
        /koinos-p2p/internal/p2p/gossip.go:130 +0x6c
created by github.com/koinos/koinos-p2p/internal/p2p.(*GossipManager).Start
        /koinos-p2p/internal/p2p/gossip.go:88 +0x1fb

Expected behavior

Not crash

Steps to reproduce

This is a race on gm.sub() so it may be difficult to reproduce.

Environment

- OS: Ubuntu 22.04

Anything else?

Looking at the code and backtrace, it is clear this is a race on gm.sub.

Basically Stop() assumes the thread will immediately exit when cancel() is called.

However it's possible for the following sequence of events to occur:

The context is cancelled and gm.sub is set to nil.
The channel in readMessages() receives data at about the same time.
The select in readMessages() could go into either branch.
According to Golang docs, select picks a branch at random if multiple branches are ready.
Golang happens to pick the ch <- msg.Data branch
The select runs to completion as it's a no-op
The select is the last thing in the readMessages() for loop, so we go back to the top of the loop
The first thing that happens at the top of the loop is calling gm.sub.Next()
This causes SIGSEGV because gm.sub is nil

The proper thing to do is have Stop() wait for the readMessages() goroutine to exit after calling cancel(), then only assign gm.sub = nil after cancel() is finished.

p2p_1                  | 2022-10-21 07:40:01.515568 (p2p.Koinos) [p2p/gossip.go:227] <info>: Publishing transaction - ID: 0x12202a86023b1f926c70c78448bbf5d312e8a0ff97e63a5a99f0f1a9b8934594b1a7
p2p_1                  | 2022-10-21 07:40:01.518263 (p2p.Koinos) [p2p/gossip.go:399] <info>: Gossiped transaction applied - ID: 0x12202a86023b1f926c70c78448bbf5d312e8a0ff97e63a5a99f0f1a9b8934594b1a7 from peer QmcGiTpSm6YrmYo3rWoqrCPez2aJY4VdraBQsGsZKwFRuG
p2p_1                  | 2022-10-21 07:40:01.530219 (p2p.Koinos) [p2p/gossip.go:227] <info>: Publishing transaction - ID: 0x1220722045ca0f1f1b417c9e42643ed5f2c56b695b969aff6b03685ecffdbe05579c
p2p_1                  | 2022-10-21 07:40:01.533016 (p2p.Koinos) [p2p/gossip.go:399] <info>: Gossiped transaction applied - ID: 0x1220722045ca0f1f1b417c9e42643ed5f2c56b695b969aff6b03685ecffdbe05579c from peer QmcGiTpSm6YrmYo3rWoqrCPez2aJY4VdraBQsGsZKwFRuG
p2p_1                  | 2022-10-21 07:40:01.553265 (p2p.Koinos) [p2p/gossip.go:399] <info>: Gossiped transaction applied - ID: 0x1220feff091390151c7e5043523a7a18b84d9611fd4da0bc23d9fa1fbe91a746b2cf from peer QmcGiTpSm6YrmYo3rWoqrCPez2aJY4VdraBQsGsZKwFRuG
p2p_1                  | 2022-10-21 07:40:01.553705 (p2p.Koinos) [p2p/gossip.go:227] <info>: Publishing transaction - ID: 0x1220472225625ed09c8d5b0496e456db113d0a68ae3e10fb961286a499f21c95e05a
p2p_1                  | 2022-10-21 07:40:01.554257 (p2p.Koinos) [p2p/gossip.go:227] <info>: Publishing transaction - ID: 0x1220feff091390151c7e5043523a7a18b84d9611fd4da0bc23d9fa1fbe91a746b2cf
p2p_1                  | 2022-10-21 07:40:02.397483 (p2p.Koinos) [p2p/gossip.go:211] <info>: Stopping gossip mode
p2p_1                  | 2022-10-21 07:40:02.397971 (p2p.Koinos) [p2p/gossip.go:132] <warn>: Error getting message for topic koinos.blocks: context canceled
p2p_1                  | 2022-10-21 07:40:02.401580 (p2p.Koinos) [p2p/gossip.go:227] <info>: Publishing transaction - ID: 0x12203a2fd03e69f8784f21612a093058914c0c34d845cf7b255b3167cf2157b996a0
p2p_1                  | 2022-10-21 07:40:02.407037 (p2p.Koinos) [p2p/gossip.go:399] <info>: Gossiped transaction applied - ID: 0x12203a2fd03e69f8784f21612a093058914c0c34d845cf7b255b3167cf2157b996a0 from peer QmcGiTpSm6YrmYo3rWoqrCPez2aJY4VdraBQsGsZKwFRuG
p2p_1                  | 2022-10-21 07:40:02.407145 (p2p.Koinos) [p2p/gossip.go:399] <info>: Gossiped transaction applied - ID: 0x1220472225625ed09c8d5b0496e456db113d0a68ae3e10fb961286a499f21c95e05a from peer QmcGiTpSm6YrmYo3rWoqrCPez2aJY4VdraBQsGsZKwFRuG
p2p_1                  | 2022-10-21 07:40:02.412013 (p2p.Koinos) [p2p/gossip.go:332] <info>: Gossiped block applied - Height: 1644430 ID: 0x12202947d13c8eca9540354aa042c494ce03c10bc0d27d81c90f5bfcc2f3fbe52d09 Prev: 0x12207f666de93a246a1cab30ef5d061564d0d521f95969ee2363e0e79152d4046d51 from peer QmcGiTpSm6YrmYo3rWoqrCPez2aJY4VdraBQsGsZKwFRuG
p2p_1                  | 2022-10-21 07:40:02.426252 (p2p.Koinos) [p2p/gossip.go:227] <info>: Publishing transaction - ID: 0x12208fe233ab666d27779d855e918662684206d11b1999d64d1c95e5bb36a87225ac
p2p_1                  | 2022-10-21 07:40:02.428477 (p2p.Koinos) [p2p/gossip.go:399] <info>: Gossiped transaction applied - ID: 0x12208fe233ab666d27779d855e918662684206d11b1999d64d1c95e5bb36a87225ac from peer QmcGiTpSm6YrmYo3rWoqrCPez2aJY4VdraBQsGsZKwFRuG
p2p_1                  | 2022-10-21 07:40:02.435780 (p2p.Koinos) [p2p/gossip.go:227] <info>: Publishing transaction - ID: 0x1220d68f6f2b3cbef1768371a23afcb6a86e0b227ac3e49774a50f61de2d602642b9
p2p_1                  | 2022-10-21 07:40:02.436720 (p2p.Koinos) [p2p/gossip.go:399] <info>: Gossiped transaction applied - ID: 0x1220d68f6f2b3cbef1768371a23afcb6a86e0b227ac3e49774a50f61de2d602642b9 from peer QmcGiTpSm6YrmYo3rWoqrCPez2aJY4VdraBQsGsZKwFRuG
p2p_1                  | 2022-10-21 07:40:02.462970 (p2p.Koinos) [p2p/gossip.go:227] <info>: Publishing transaction - ID: 0x1220d9da1f844cf714b1e975ffd1dc0241e51fd16ec32da366a27f6059d9bc3840ad
p2p_1                  | 2022-10-21 07:40:02.463489 (p2p.Koinos) [p2p/gossip.go:227] <info>: Publishing transaction - ID: 0x122048f8de214b4209930b0d0c21cfdb09987088aeb478dfd155245eaf5c08512e6c
p2p_1                  | 2022-10-21 07:40:02.463836 (p2p.Koinos) [p2p/gossip.go:399] <info>: Gossiped transaction applied - ID: 0x122048f8de214b4209930b0d0c21cfdb09987088aeb478dfd155245eaf5c08512e6c from peer QmcGiTpSm6YrmYo3rWoqrCPez2aJY4VdraBQsGsZKwFRuG
p2p_1                  | 2022-10-21 07:40:02.463950 (p2p.Koinos) [p2p/gossip.go:399] <info>: Gossiped transaction applied - ID: 0x1220d9da1f844cf714b1e975ffd1dc0241e51fd16ec32da366a27f6059d9bc3840ad from peer QmcGiTpSm6YrmYo3rWoqrCPez2aJY4VdraBQsGsZKwFRuG
p2p_1                  | 2022-10-21 07:40:02.498804 (p2p.Koinos) [p2p/gossip.go:399] <info>: Gossiped transaction applied - ID: 0x1220b06fb6d94797815a324b44f78f25b0f9ad1bb4851a2b154cfebec322d29cb051 from peer QmcGiTpSm6YrmYo3rWoqrCPez2aJY4VdraBQsGsZKwFRuG
p2p_1                  | 2022-10-21 07:40:02.502910 (p2p.Koinos) [p2p/gossip.go:399] <info>: Gossiped transaction applied - ID: 0x1220d334e994f5e3d1e736c36d4ac1c2e312d2c8110772c225c911081f529bbbca77 from peer QmcGiTpSm6YrmYo3rWoqrCPez2aJY4VdraBQsGsZKwFRuG
p2p_1                  | 2022-10-21 07:40:02.527325 (p2p.Koinos) [p2p/gossip.go:399] <info>: Gossiped transaction applied - ID: 0x12207726022b4065503b0d601374992cf4df7522de53348d973ec887edb4652d0155 from peer QmcGiTpSm6YrmYo3rWoqrCPez2aJY4VdraBQsGsZKwFRuG
p2p_1                  | 2022-10-21 07:40:02.541019 (p2p.Koinos) [p2p/gossip.go:399] <info>: Gossiped transaction applied - ID: 0x1220b2a347295f3afd0a55945682f868d42b22748b3d4942f91fa9b0af898e3c038b from peer QmcGiTpSm6YrmYo3rWoqrCPez2aJY4VdraBQsGsZKwFRuG
p2p_1                  | 2022-10-21 07:40:02.553708 (p2p.Koinos) [p2p/gossip.go:399] <info>: Gossiped transaction applied - ID: 0x1220ddbbc369d66561a10b3cd26f5a915cdb01e88dc96ec63762fef90116d20fec2a from peer QmcGiTpSm6YrmYo3rWoqrCPez2aJY4VdraBQsGsZKwFRuG
p2p_1                  | 2022-10-21 07:40:02.563522 (p2p.Koinos) [p2p/gossip.go:204] <info>: Starting gossip mode
p2p_1                  | 2022-10-21 07:40:02.564074 (p2p.Koinos) [p2p/gossip.go:258] <info>: Started block gossip listener
p2p_1                  | 2022-10-21 07:40:02.564274 (p2p.Koinos) [p2p/gossip.go:342] <info>: Started transaction gossip listener
p2p_1                  | 2022-10-21 07:40:02.572221 (p2p.Koinos) [p2p/gossip.go:399] <info>: Gossiped transaction applied - ID: 0x1220284aa47295ceff1a4ec4fe12d8293bc24006c4ae68f24b64057fa5deef48de77 from peer QmcGiTpSm6YrmYo3rWoqrCPez2aJY4VdraBQsGsZKwFRuG
p2p_1                  | 2022-10-21 07:40:02.576852 (p2p.Koinos) [p2p/gossip.go:227] <info>: Publishing transaction - ID: 0x1220284aa47295ceff1a4ec4fe12d8293bc24006c4ae68f24b64057fa5deef48de77
p2p_1                  | 2022-10-21 07:40:02.593079 (p2p.Koinos) [p2p/gossip.go:399] <info>: Gossiped transaction applied - ID: 0x1220284f9671f565556a0b579c7e3c2fe94435753ac6b190f84947bad93a5cff56b2 from peer QmcGiTpSm6YrmYo3rWoqrCPez2aJY4VdraBQsGsZKwFRuG
p2p_1                  | 2022-10-21 07:40:02.593494 (p2p.Koinos) [p2p/gossip.go:227] <info>: Publishing transaction - ID: 0x1220284f9671f565556a0b579c7e3c2fe94435753ac6b190f84947bad93a5cff56b2
p2p_1                  | 2022-10-21 07:40:02.610151 (p2p.Koinos) [p2p/gossip.go:227] <info>: Publishing transaction - ID: 0x122061ab9d845c599b99961308b1493b495c45019de1179341e7766f300a8dcf7208
p2p_1                  | 2022-10-21 07:40:02.617167 (p2p.Koinos) [p2p/gossip.go:399] <info>: Gossiped transaction applied - ID: 0x122061ab9d845c599b99961308b1493b495c45019de1179341e7766f300a8dcf7208 from peer QmcGiTpSm6YrmYo3rWoqrCPez2aJY4VdraBQsGsZKwFRuG
p2p_1                  | panic: runtime error: invalid memory address or nil pointer dereference
p2p_1                  | [signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0xa8cb4d]
p2p_1                  |
p2p_1                  | goroutine 2582706 [running]:
p2p_1                  | github.com/libp2p/go-libp2p-pubsub.(*Subscription).Next(0x0, {0x154b368, 0xc01d76b640})
p2p_1                  |    /go/pkg/mod/github.com/libp2p/go-libp2p-pubsub@v0.7.1/subscription.go:27 +0x2d
p2p_1                  | github.com/koinos/koinos-p2p/internal/p2p.(*GossipManager).readMessages(0xc0006620f0, {0x154b368, 0xc01d76b640}, 0xc008708060)
p2p_1                  |    /koinos-p2p/internal/p2p/gossip.go:130 +0x6c
p2p_1                  | created by github.com/koinos/koinos-p2p/internal/p2p.(*GossipManager).Start
p2p_1                  |    /koinos-p2p/internal/p2p/gossip.go:88 +0x1fb
koinos_p2p_1 exited with code 2

koinos / koinos-p2p