influxdata / influxdb-client-go

InfluxDB 2 Go Client
MIT License
609 stars 116 forks source link

Panic: close on closed channel when multiple goroutines call `influxdb2.Client.Close()` #317

Closed dleonard203 closed 2 years ago

dleonard203 commented 2 years ago

It is possible to get the influxdb2.Client to panic if Client.Close() is called at the same time from multiple goroutines:

panic: close of closed channel
goroutine 341 [running]:
github.com/influxdata/influxdb-client-go/v2/api.(*WriteAPIImpl).Close(0xc0001b15e0)
        /go/pkg/mod/github.com/influxdata/influxdb-client-go/v2@v2.2.0/api/write.go:176 +0x6f
github.com/influxdata/influxdb-client-go/v2.(*clientImpl).Close(0xc00064e0c0)
        /go/pkg/mod/github.com/influxdata/influxdb-client-go/v2@v2.2.0/client.go:218 +0x8e

It appears the cause is that on calling Client.Close, it is looping through all non-blocking WriteAPI's, and closing a channel on the struct. Before doing this channel closing, the Close method mkes sure the channel is not nil. The channel is set to nil after some processing is done, so two goroutines can pass this nil check, causing them both to attempt to close the channel.

I think a fix to this would be to place a shutdown mutex on the non-blocking WriteAPI object, lock it when WriteAPI.Close() is called, set the channel to nil with the lock, and check for the channel being nil after the lock is acquired. That way, if two goroutines call client.Close() at the same time, the second goroutine will see that the first has set the channel to nil, and will not attempt the shutdown processing.

Steps to reproduce:

  1. Create one influxdb2.Client object to be accessed from multiple goroutines
  2. Start a goroutine reading from influxdb every second, calling a defer client.Close()
  3. Start a goroutine writing 300 points once per second using non-blocking WriteAPI, also calling defer client.Close() at the end of its call
  4. There should be a panic within a few seconds

Expected behavior: There should be no panic if two goroutines try to close the client.

Actual behavior: The client panics

Specifications:

Edit: I haven't used the latest client, but checked the 2.8.1 tag and see the same issue should still exist in latest.

vlastahajek commented 2 years ago

This was fixed by #319