kv: fix some goroutine leak when fail halfway in NewKVStore (#1765)

Signed-off-by: lance6716 <lance6716@gmail.com>
This commit is contained in:
lance6716 2025-09-28 18:50:07 +08:00 committed by GitHub
parent 0fdd25559b
commit 0172e26932
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 32 additions and 3 deletions

View File

@ -18,7 +18,7 @@ jobs:
go-version: 1.23.2 go-version: 1.23.2
- name: Test - name: Test
run: go test ./... run: go test --tags=intest ./...
race-test: race-test:
runs-on: ubuntu-latest runs-on: ubuntu-latest
@ -31,7 +31,7 @@ jobs:
go-version: 1.23.2 go-version: 1.23.2
- name: Test with race - name: Test with race
run: go test -race ./... run: go test -race --tags=intest ./...
golangci: golangci:
runs-on: ubuntu-latest runs-on: ubuntu-latest

View File

@ -66,6 +66,7 @@ import (
"github.com/tikv/client-go/v2/txnkv/txnlock" "github.com/tikv/client-go/v2/txnkv/txnlock"
"github.com/tikv/client-go/v2/txnkv/txnsnapshot" "github.com/tikv/client-go/v2/txnkv/txnsnapshot"
"github.com/tikv/client-go/v2/util" "github.com/tikv/client-go/v2/util"
"github.com/tikv/client-go/v2/util/intest"
pd "github.com/tikv/pd/client" pd "github.com/tikv/pd/client"
"github.com/tikv/pd/client/clients/gc" "github.com/tikv/pd/client/clients/gc"
pdhttp "github.com/tikv/pd/client/http" pdhttp "github.com/tikv/pd/client/http"
@ -316,13 +317,24 @@ func requestHealthFeedbackFromKVClient(ctx context.Context, addr string, tikvCli
} }
// NewKVStore creates a new TiKV store instance. // NewKVStore creates a new TiKV store instance.
func NewKVStore(uuid string, pdClient pd.Client, spkv SafePointKV, tikvclient Client, opt ...Option) (*KVStore, error) { func NewKVStore(
uuid string,
pdClient pd.Client,
spkv SafePointKV,
tikvclient Client,
opt ...Option,
) (_ *KVStore, retErr error) {
o, err := oracles.NewPdOracle(pdClient, &oracles.PDOracleOptions{ o, err := oracles.NewPdOracle(pdClient, &oracles.PDOracleOptions{
UpdateInterval: defaultOracleUpdateInterval, UpdateInterval: defaultOracleUpdateInterval,
}) })
if err != nil { if err != nil {
return nil, err return nil, err
} }
defer func() {
if retErr != nil {
o.Close()
}
}()
ctx, cancel := context.WithCancel(context.Background()) ctx, cancel := context.WithCancel(context.Background())
var opts []locate.RegionCacheOpt var opts []locate.RegionCacheOpt
if config.NextGen { if config.NextGen {
@ -349,8 +361,18 @@ func NewKVStore(uuid string, pdClient pd.Client, spkv SafePointKV, tikvclient Cl
cancel: cancel, cancel: cancel,
gP: NewSpool(128, 10*time.Second), gP: NewSpool(128, 10*time.Second),
} }
defer func() {
if retErr != nil {
regionCache.Close()
}
}()
txnSafePoint, err := store.loadTxnSafePoint(context.Background()) txnSafePoint, err := store.loadTxnSafePoint(context.Background())
if intest.InTest {
if uuid == "TestErrorHalfwayInNewKVStore" {
err = errors.New("mock error for TestErrorHalfwayInNewKVStore")
}
}
if err != nil { if err != nil {
return nil, errors.WithStack(err) return nil, errors.WithStack(err)
} }

View File

@ -24,6 +24,7 @@ import (
"github.com/pingcap/kvproto/pkg/kvrpcpb" "github.com/pingcap/kvproto/pkg/kvrpcpb"
"github.com/pingcap/kvproto/pkg/metapb" "github.com/pingcap/kvproto/pkg/metapb"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite" "github.com/stretchr/testify/suite"
"github.com/tikv/client-go/v2/internal/mockstore/mocktikv" "github.com/tikv/client-go/v2/internal/mockstore/mocktikv"
"github.com/tikv/client-go/v2/oracle" "github.com/tikv/client-go/v2/oracle"
@ -273,3 +274,9 @@ func (s *testKVSuite) TestMinSafeTsFromMixed2() {
s.Require().Equal(mockClient.tikvSafeTs, s.store.GetMinSafeTS("z1")) s.Require().Equal(mockClient.tikvSafeTs, s.store.GetMinSafeTS("z1"))
s.Require().Equal(uint64(10), s.store.GetMinSafeTS("z2")) s.Require().Equal(uint64(10), s.store.GetMinSafeTS("z2"))
} }
func (s *testKVSuite) TestErrorHalfwayInNewKVStore() {
// this is a leak test, TestMain will check goroutine leak
_, err := NewKVStore("TestErrorHalfwayInNewKVStore", s.store.pdClient, NewMockSafePointKV(), &mocktikv.RPCClient{})
require.Error(s.T(), err)
}