fix: potential deadlock in Subscriber

[dunglas]

Fixes https://github.com/dunglas/mercure/pull/578/files#r748726493.

[divine]

Hello,

Does it affect performance in any case (again select case uh oh)? Benchmark results are the same, right?

I don't remember when it was added but I think subscribers were not closing correctly and memory was leaking or whatever...

Thanks!

[dunglas]

I didn't check performance yet because currently... It just doesn't work. The tests are still failing. I think that this case has been forgotten in the new implementation, it was one of the reasons my previous implementation was so complex. I hope that we'll manage to find a solution without partially reverting #578.

[dunglas]

The fix looks ok now. Here are the benchmarks:

Before:

Running tool: /opt/homebrew/bin/go test -benchmem -run=^$ -bench ^BenchmarkSubscriber$ github.com/dunglas/mercure

goos: darwin
goarch: arm64
pkg: github.com/dunglas/mercure
BenchmarkSubscriber/topics_1_concurrency_100_matchpct_1-8         	 8197478	       142.9 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_100_matchpct_10-8        	 9163732	       136.4 ns/op	     117 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_100_matchpct_100-8       	 8619999	       138.5 ns/op	     118 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_1000_matchpct_1-8        	 8454166	       150.1 ns/op	     118 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_1000_matchpct_10-8       	 8950066	       133.5 ns/op	     117 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_1000_matchpct_100-8      	 8266060	       139.1 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_5000_matchpct_1-8        	 8431623	       139.8 ns/op	     118 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_5000_matchpct_10-8       	 8772720	       131.4 ns/op	     118 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_5000_matchpct_100-8      	 9008971	       136.9 ns/op	     117 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_20000_matchpct_1-8       	 8496710	       147.0 ns/op	     118 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_20000_matchpct_10-8      	 8008602	       148.8 ns/op	     120 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_20000_matchpct_100-8     	 8044272	       151.4 ns/op	     120 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_100_matchpct_1-8         	 7840760	       155.4 ns/op	     120 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_100_matchpct_10-8        	 8054492	       147.2 ns/op	     120 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_100_matchpct_100-8       	 8382222	       143.1 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_1000_matchpct_1-8        	 7056787	       144.0 ns/op	     117 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_1000_matchpct_10-8       	 8493970	       138.6 ns/op	     118 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_1000_matchpct_100-8      	 8966272	       132.8 ns/op	     117 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_5000_matchpct_1-8        	 9069616	       131.0 ns/op	     117 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_5000_matchpct_10-8       	 8847879	       133.2 ns/op	     117 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_5000_matchpct_100-8      	 7727300	       154.4 ns/op	     121 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_20000_matchpct_1-8       	 8244864	       128.7 ns/op	     116 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_20000_matchpct_10-8      	 8470156	       134.4 ns/op	     118 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_20000_matchpct_100-8     	 7781270	       159.5 ns/op	     120 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_100_matchpct_1-8        	 7640623	       139.1 ns/op	     115 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_100_matchpct_10-8       	 8363376	       143.7 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_100_matchpct_100-8      	 8616998	       144.3 ns/op	     118 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_1000_matchpct_1-8       	 7698120	       154.3 ns/op	     121 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_1000_matchpct_10-8      	 8209983	       138.7 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_1000_matchpct_100-8     	 8305965	       136.1 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_5000_matchpct_1-8       	 8414283	       142.1 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_5000_matchpct_10-8      	 8085674	       160.1 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_5000_matchpct_100-8     	 6867969	       156.6 ns/op	     117 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_20000_matchpct_1-8      	 8193468	       140.4 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_20000_matchpct_10-8     	 8650684	       142.0 ns/op	     118 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_20000_matchpct_100-8    	 7934019	       148.8 ns/op	     120 B/op	       1 allocs/op
PASS
ok  	github.com/dunglas/mercure	50.142s

After:

Running tool: /opt/homebrew/bin/go test -benchmem -run=^$ -bench ^BenchmarkSubscriber$ github.com/dunglas/mercure

goos: darwin
goarch: arm64
pkg: github.com/dunglas/mercure
BenchmarkSubscriber/topics_1_concurrency_100_matchpct_1-8         	 7090341	       151.8 ns/op	     117 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_100_matchpct_10-8        	 6897596	       155.0 ns/op	     117 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_100_matchpct_100-8       	 8114916	       148.1 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_1000_matchpct_1-8        	 7870522	       146.8 ns/op	     120 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_1000_matchpct_10-8       	 7926254	       149.3 ns/op	     120 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_1000_matchpct_100-8      	 8005965	       149.2 ns/op	     120 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_5000_matchpct_1-8        	 8371846	       155.7 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_5000_matchpct_10-8       	 8039814	       150.6 ns/op	     120 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_5000_matchpct_100-8      	 8160457	       146.5 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_20000_matchpct_1-8       	 8123503	       150.7 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_20000_matchpct_10-8      	 7804729	       144.8 ns/op	     120 B/op	       1 allocs/op
BenchmarkSubscriber/topics_1_concurrency_20000_matchpct_100-8     	 8215717	       146.8 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_100_matchpct_1-8         	 7974360	       158.4 ns/op	     120 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_100_matchpct_10-8        	 8101228	       144.8 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_100_matchpct_100-8       	 8190596	       150.0 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_1000_matchpct_1-8        	 8067156	       150.2 ns/op	     120 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_1000_matchpct_10-8       	 8223716	       146.4 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_1000_matchpct_100-8      	 8056371	       144.5 ns/op	     120 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_5000_matchpct_1-8        	 7559812	       150.7 ns/op	     115 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_5000_matchpct_10-8       	 8075546	       144.0 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_5000_matchpct_100-8      	 8036002	       145.0 ns/op	     120 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_20000_matchpct_1-8       	 8098959	       147.8 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_20000_matchpct_10-8      	 8079504	       152.6 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_5_concurrency_20000_matchpct_100-8     	 8022060	       149.5 ns/op	     120 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_100_matchpct_1-8        	 6780748	       155.6 ns/op	     118 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_100_matchpct_10-8       	 8010022	       150.1 ns/op	     120 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_100_matchpct_100-8      	 8153761	       144.6 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_1000_matchpct_1-8       	 8218114	       148.2 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_1000_matchpct_10-8      	 8069245	       147.2 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_1000_matchpct_100-8     	 7629559	       154.4 ns/op	     115 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_5000_matchpct_1-8       	 7375036	       150.4 ns/op	     116 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_5000_matchpct_10-8      	 8095328	       149.0 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_5000_matchpct_100-8     	 8057392	       147.3 ns/op	     120 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_20000_matchpct_1-8      	 8248732	       143.8 ns/op	     119 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_20000_matchpct_10-8     	 7946229	       151.6 ns/op	     120 B/op	       1 allocs/op
BenchmarkSubscriber/topics_10_concurrency_20000_matchpct_100-8    	 7848465	       155.6 ns/op	     120 B/op	       1 allocs/op
PASS
ok  	github.com/dunglas/mercure	49.465s

@divine would you mind to test this patch in prod to see if doesn't introduce a regression?

[divine]

Hello @dunglas,

Sure, I'll let you know the results in a few days.

Thanks!