Skip to content

Commit

Permalink
fix: all: fix resource leaks found by staticmajor (cosmos#13394)
Browse files Browse the repository at this point in the history 
Fixes resource leaks in which a .Close, .Stop method
wasn't invoked in return routes such as with errors.

Found by cosmos#13392
  • Loading branch information
@odeke-em
odeke-em authored Oct 6, 2022
1 parent fbd81ba commit 481569c
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 12 deletions.
1 change: 1 addition & 0 deletions server/grpc/gogoreflection/fix_registration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ func compress(fd *dpb.FileDescriptorProto) ([]byte, error) {
cw := gzip.NewWriter(buf)
_, err = cw.Write(fdBytes)
if err != nil {
cw.Close()
return nil, err
}
err = cw.Close()
Expand Down
10 changes: 10 additions & 0 deletions server/start.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,16 @@ func startInProcess(ctx *Context, clientCtx client.Context, appCreator types.App
return err
}

// Clean up the traceWriter in the cpuProfileCleanup routine that is invoked
// when the server is shutting down.
fn := cpuProfileCleanup
cpuProfileCleanup = func() {
if fn != nil {
fn()
}
traceWriter.Close()
}

config, err := serverconfig.GetConfig(ctx.Viper)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion server/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ func openDB(rootDir string, backendType dbm.BackendType) (dbm.DB, error) {
return dbm.NewDB("application", backendType, dataDir)
}

func openTraceWriter(traceWriterFile string) (w io.Writer, err error) {
func openTraceWriter(traceWriterFile string) (w io.WriteCloser, err error) {
if traceWriterFile == "" {
return
}
Expand Down
9 changes: 7 additions & 2 deletions telemetry/metrics.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ type GatherResponse struct {
}

// New creates a new instance of Metrics
func New(cfg Config) (*Metrics, error) {
func New(cfg Config) (_ *Metrics, rerr error) {
if !cfg.Enabled {
return nil, nil
}
Expand All @@ -90,7 +90,12 @@ func New(cfg Config) (*Metrics, error) {
metricsConf.EnableHostnameLabel = cfg.EnableHostnameLabel

memSink := metrics.NewInmemSink(10*time.Second, time.Minute)
metrics.DefaultInmemSignal(memSink)
inMemSig := metrics.DefaultInmemSignal(memSink)
defer func() {
if rerr != nil {
inMemSig.Stop()
}
}()

m := &Metrics{memSink: memSink}
fanout := metrics.FanoutSink{memSink}
Expand Down
2 changes: 1 addition & 1 deletion testutil/network/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func startInProcess(cfg Config, val *Validator) error {
app := cfg.AppConstructor(*val)
genDocProvider := node.DefaultGenesisDocProviderFunc(tmCfg)

tmNode, err := node.NewNode(
tmNode, err := node.NewNode( //resleak:notresource
tmCfg,
pvm.LoadOrGenFilePV(tmCfg.PrivValidatorKeyFile(), tmCfg.PrivValidatorStateFile()),
nodeKey,
Expand Down
57 changes: 49 additions & 8 deletions x/auth/client/testutil/suite.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,14 @@ func (s *IntegrationTestSuite) TestCLIValidateSignatures() {

// write unsigned tx to file
unsignedTx := testutil.WriteToNewTempFile(s.T(), res.String())
defer unsignedTx.Close()
res, err = TxSignExec(val.ClientCtx, val.Address, unsignedTx.Name())
s.Require().NoError(err)
signedTx, err := val.ClientCtx.TxConfig.TxJSONDecoder()(res.Bytes())
s.Require().NoError(err)

signedTxFile := testutil.WriteToNewTempFile(s.T(), res.String())
defer signedTxFile.Close()
txBuilder, err := val.ClientCtx.TxConfig.WrapTxBuilder(signedTx)
s.Require().NoError(err)
_, err = TxValidateSignaturesExec(val.ClientCtx, signedTxFile.Name())
Expand All @@ -116,6 +118,7 @@ func (s *IntegrationTestSuite) TestCLIValidateSignatures() {
s.Require().NoError(err)

modifiedTxFile := testutil.WriteToNewTempFile(s.T(), string(bz))
defer modifiedTxFile.Close()

_, err = TxValidateSignaturesExec(val.ClientCtx, modifiedTxFile.Name())
s.Require().EqualError(err, "signatures validation failed")
Expand Down Expand Up @@ -147,6 +150,7 @@ func (s *IntegrationTestSuite) TestCLISignGenOnly() {
generatedStd, err := clitestutil.ExecTestCLICmd(val.ClientCtx, bank.NewSendTxCmd(), args)
s.Require().NoError(err)
opFile := testutil.WriteToNewTempFile(s.T(), generatedStd.String())
defer opFile.Close()

commonArgs := []string{
fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest),
Expand Down Expand Up @@ -227,9 +231,12 @@ func (s *IntegrationTestSuite) TestCLISignGenOnly() {
s.Require().Contains(err.Error(), tc.errMsg)
} else {
s.Require().NoError(err)
signedTx := testutil.WriteToNewTempFile(s.T(), out.String())
_, err := TxBroadcastExec(val.ClientCtx, signedTx.Name())
s.Require().NoError(err)
func() {
signedTx := testutil.WriteToNewTempFile(s.T(), out.String())
defer signedTx.Close()
_, err := TxBroadcastExec(val.ClientCtx, signedTx.Name())
s.Require().NoError(err)
}()
}
}
}
Expand All @@ -246,6 +253,7 @@ func (s *IntegrationTestSuite) TestCLISignBatch() {
s.Require().NoError(err)

outputFile := testutil.WriteToNewTempFile(s.T(), strings.Repeat(generatedStd.String(), 3))
defer outputFile.Close()
val.ClientCtx.HomeDir = strings.Replace(val.ClientCtx.HomeDir, "simd", "simcli", 1)

// sign-batch file - offline is set but account-number and sequence are not
Expand Down Expand Up @@ -277,6 +285,7 @@ func (s *IntegrationTestSuite) TestCLISignBatch() {

// Sign batch malformed tx file.
malformedFile := testutil.WriteToNewTempFile(s.T(), fmt.Sprintf("%smalformed", generatedStd))
defer malformedFile.Close()
_, err = TxSignBatchExec(val.ClientCtx, val.Address, malformedFile.Name(), fmt.Sprintf("--%s=%s", flags.FlagChainID, val.ClientCtx.ChainID))
s.Require().Error(err)

Expand Down Expand Up @@ -375,6 +384,7 @@ func (s *IntegrationTestSuite) TestCLISignAminoJSON() {
sendTokens, fmt.Sprintf("--%s=true", flags.FlagGenerateOnly))
require.NoError(err)
fileUnsigned := testutil.WriteToNewTempFile(s.T(), txBz.String())
defer fileUnsigned.Close()
chainFlag := fmt.Sprintf("--%s=%s", flags.FlagChainID, val1.ClientCtx.ChainID)
sigOnlyFlag := "--signature-only"
signModeAminoFlag := "--sign-mode=amino-json"
Expand Down Expand Up @@ -792,6 +802,7 @@ func (s *IntegrationTestSuite) TestCLISendGenerateSignAndBroadcast() {

// Write the output to disk
unsignedTxFile := testutil.WriteToNewTempFile(s.T(), finalGeneratedTx.String())
defer unsignedTxFile.Close()

// Test validate-signatures
res, err := TxValidateSignaturesExec(val1.ClientCtx, unsignedTxFile.Name())
Expand Down Expand Up @@ -824,6 +835,7 @@ func (s *IntegrationTestSuite) TestCLISendGenerateSignAndBroadcast() {

// Write the output to disk
signedTxFile := testutil.WriteToNewTempFile(s.T(), signedTx.String())
defer signedTxFile.Close()

// validate Signature
res, err = TxValidateSignaturesExec(val1.ClientCtx, signedTxFile.Name())
Expand Down Expand Up @@ -908,6 +920,7 @@ func (s *IntegrationTestSuite) TestCLIMultisignInsufficientCosigners() {

// Save tx to file
multiGeneratedTxFile := testutil.WriteToNewTempFile(s.T(), multiGeneratedTx.String())
defer multiGeneratedTxFile.Close()

// Multisign, sign with one signature
val1.ClientCtx.HomeDir = strings.Replace(val1.ClientCtx.HomeDir, "simd", "simcli", 1)
Expand All @@ -917,12 +930,14 @@ func (s *IntegrationTestSuite) TestCLIMultisignInsufficientCosigners() {
s.Require().NoError(err)

sign1File := testutil.WriteToNewTempFile(s.T(), account1Signature.String())
defer sign1File.Close()

multiSigWith1Signature, err := TxMultiSignExec(val1.ClientCtx, multisigRecord.Name, multiGeneratedTxFile.Name(), sign1File.Name())
s.Require().NoError(err)

// Save tx to file
multiSigWith1SignatureFile := testutil.WriteToNewTempFile(s.T(), multiSigWith1Signature.String())
defer multiSigWith1SignatureFile.Close()

_, err = TxValidateSignaturesExec(val1.ClientCtx, multiSigWith1SignatureFile.Name())
s.Require().Error(err)
Expand All @@ -941,6 +956,7 @@ func (s *IntegrationTestSuite) TestCLIEncode() {
)
s.Require().NoError(err)
savedTxFile := testutil.WriteToNewTempFile(s.T(), normalGeneratedTx.String())
defer savedTxFile.Close()

// Encode
encodeExec, err := TxEncodeExec(val1.ClientCtx, savedTxFile.Name())
Expand Down Expand Up @@ -1022,6 +1038,7 @@ func (s *IntegrationTestSuite) TestCLIMultisignSortSignatures() {

// Save tx to file
multiGeneratedTxFile := testutil.WriteToNewTempFile(s.T(), multiGeneratedTx.String())
defer multiGeneratedTxFile.Close()

// Sign with account1
addr1, err := account1.GetAddress()
Expand All @@ -1031,6 +1048,7 @@ func (s *IntegrationTestSuite) TestCLIMultisignSortSignatures() {
s.Require().NoError(err)

sign1File := testutil.WriteToNewTempFile(s.T(), account1Signature.String())
defer sign1File.Close()

// Sign with account2
addr2, err := account2.GetAddress()
Expand All @@ -1039,6 +1057,7 @@ func (s *IntegrationTestSuite) TestCLIMultisignSortSignatures() {
s.Require().NoError(err)

sign2File := testutil.WriteToNewTempFile(s.T(), account2Signature.String())
defer sign2File.Close()

// Sign with dummy account
dummyAddr, err := dummyAcc.GetAddress()
Expand All @@ -1052,6 +1071,7 @@ func (s *IntegrationTestSuite) TestCLIMultisignSortSignatures() {

// Write the output to disk
signedTxFile := testutil.WriteToNewTempFile(s.T(), multiSigWith2Signatures.String())
defer signedTxFile.Close()

_, err = TxValidateSignaturesExec(val1.ClientCtx, signedTxFile.Name())
s.Require().NoError(err)
Expand Down Expand Up @@ -1095,6 +1115,7 @@ func (s *IntegrationTestSuite) TestSignWithMultisig() {

// Save multi tx to file
multiGeneratedTx2File := testutil.WriteToNewTempFile(s.T(), multisigTx.String())
defer multiGeneratedTx2File.Close()

// Sign using multisig. We're signing a tx on behalf of the multisig address,
// even though the tx signer is NOT the multisig address. This is fine though,
Expand Down Expand Up @@ -1155,6 +1176,7 @@ func (s *IntegrationTestSuite) TestCLIMultisign() {

// Save tx to file
multiGeneratedTxFile := testutil.WriteToNewTempFile(s.T(), multiGeneratedTx.String())
defer multiGeneratedTxFile.Close()

addr1, err := account1.GetAddress()
s.Require().NoError(err)
Expand All @@ -1164,6 +1186,7 @@ func (s *IntegrationTestSuite) TestCLIMultisign() {
s.Require().NoError(err)

sign1File := testutil.WriteToNewTempFile(s.T(), account1Signature.String())
defer sign1File.Close()

addr2, err := account2.GetAddress()
s.Require().NoError(err)
Expand All @@ -1172,6 +1195,7 @@ func (s *IntegrationTestSuite) TestCLIMultisign() {
s.Require().NoError(err)

sign2File := testutil.WriteToNewTempFile(s.T(), account2Signature.String())
defer sign2File.Close()

// Does not work in offline mode.
_, err = TxMultiSignExec(val1.ClientCtx, multisigRecord.Name, multiGeneratedTxFile.Name(), "--offline", sign1File.Name(), sign2File.Name())
Expand All @@ -1183,6 +1207,7 @@ func (s *IntegrationTestSuite) TestCLIMultisign() {

// Write the output to disk
signedTxFile := testutil.WriteToNewTempFile(s.T(), multiSigWith2Signatures.String())
defer signedTxFile.Close()

_, err = TxValidateSignaturesExec(val1.ClientCtx, signedTxFile.Name())
s.Require().NoError(err)
Expand Down Expand Up @@ -1233,6 +1258,7 @@ func (s *IntegrationTestSuite) TestSignBatchMultisig() {

// Write the output to disk
filename := testutil.WriteToNewTempFile(s.T(), strings.Repeat(generatedStd.String(), 1))
defer filename.Close()
val.ClientCtx.HomeDir = strings.Replace(val.ClientCtx.HomeDir, "simd", "simcli", 1)

addr1, err := account1.GetAddress()
Expand All @@ -1243,6 +1269,7 @@ func (s *IntegrationTestSuite) TestSignBatchMultisig() {
s.Require().Equal(1, len(strings.Split(strings.Trim(res.String(), "\n"), "\n")))
// write sigs to file
file1 := testutil.WriteToNewTempFile(s.T(), res.String())
defer file1.Close()

addr2, err := account2.GetAddress()
s.Require().NoError(err)
Expand All @@ -1252,6 +1279,7 @@ func (s *IntegrationTestSuite) TestSignBatchMultisig() {
s.Require().Equal(1, len(strings.Split(strings.Trim(res.String(), "\n"), "\n")))
// write sigs to file2
file2 := testutil.WriteToNewTempFile(s.T(), res.String())
defer file2.Close()
_, err = TxMultiSignExec(val.ClientCtx, multisigRecord.Name, filename.Name(), file1.Name(), file2.Name())
s.Require().NoError(err)
}
Expand Down Expand Up @@ -1295,6 +1323,7 @@ func (s *IntegrationTestSuite) TestMultisignBatch() {

// Write the output to disk
filename := testutil.WriteToNewTempFile(s.T(), strings.Repeat(generatedStd.String(), 3))
defer filename.Close()
val.ClientCtx.HomeDir = strings.Replace(val.ClientCtx.HomeDir, "simd", "simcli", 1)

queryResJSON, err := QueryAccountExec(val.ClientCtx, addr)
Expand All @@ -1310,6 +1339,7 @@ func (s *IntegrationTestSuite) TestMultisignBatch() {
s.Require().Equal(3, len(strings.Split(strings.Trim(res.String(), "\n"), "\n")))
// write sigs to file
file1 := testutil.WriteToNewTempFile(s.T(), res.String())
defer file1.Close()

// sign-batch file with account2
addr2, err := account2.GetAddress()
Expand All @@ -1320,17 +1350,21 @@ func (s *IntegrationTestSuite) TestMultisignBatch() {

// multisign the file
file2 := testutil.WriteToNewTempFile(s.T(), res.String())
defer file2.Close()
res, err = TxMultiSignBatchExec(val.ClientCtx, filename.Name(), multisigRecord.Name, file1.Name(), file2.Name())
s.Require().NoError(err)
signedTxs := strings.Split(strings.Trim(res.String(), "\n"), "\n")

// Broadcast transactions.
for _, signedTx := range signedTxs {
signedTxFile := testutil.WriteToNewTempFile(s.T(), signedTx)
val.ClientCtx.BroadcastMode = flags.BroadcastSync
_, err = TxBroadcastExec(val.ClientCtx, signedTxFile.Name())
s.Require().NoError(err)
s.Require().NoError(s.network.WaitForNextBlock())
func() {
signedTxFile := testutil.WriteToNewTempFile(s.T(), signedTx)
defer signedTxFile.Close()
val.ClientCtx.BroadcastMode = flags.BroadcastSync
_, err = TxBroadcastExec(val.ClientCtx, signedTxFile.Name())
s.Require().NoError(err)
s.Require().NoError(s.network.WaitForNextBlock())
}()
}
}

Expand Down Expand Up @@ -1420,6 +1454,7 @@ func TestGetBroadcastCommandWithoutOfflineFlag(t *testing.T) {
txContents, err := txCfg.TxJSONEncoder()(builder.GetTx())
require.NoError(t, err)
txFile := testutil.WriteToNewTempFile(t, string(txContents))
defer txFile.Close()

cmd.SetArgs([]string{txFile.Name()})
err = cmd.ExecuteContext(ctx)
Expand Down Expand Up @@ -1498,6 +1533,7 @@ func (s *IntegrationTestSuite) TestTxWithoutPublicKey() {
txJSON, err := txCfg.TxJSONEncoder()(txBuilder.GetTx())
s.Require().NoError(err)
unsignedTxFile := testutil.WriteToNewTempFile(s.T(), string(txJSON))
defer unsignedTxFile.Close()

// Sign the file with the unsignedTx.
signedTx, err := TxSignExec(val1.ClientCtx, val1.Address, unsignedTxFile.Name(), fmt.Sprintf("--%s=true", cli.FlagOverwrite))
Expand All @@ -1514,6 +1550,7 @@ func (s *IntegrationTestSuite) TestTxWithoutPublicKey() {
txJSON, err = val1.ClientCtx.Codec.MarshalJSON(&tx)
s.Require().NoError(err)
signedTxFile := testutil.WriteToNewTempFile(s.T(), string(txJSON))
defer signedTxFile.Close()
s.Require().True(strings.Contains(string(txJSON), "\"public_key\":null"))

// Broadcast tx, test that it shouldn't panic.
Expand Down Expand Up @@ -1553,11 +1590,13 @@ func (s *IntegrationTestSuite) TestSignWithMultiSignersAminoJSON() {
txJSON, err := val0.ClientCtx.TxConfig.TxJSONEncoder()(txBuilder.GetTx())
require.NoError(err)
unsignedTxFile := testutil.WriteToNewTempFile(s.T(), string(txJSON))
defer unsignedTxFile.Close()

// Let val0 sign first the file with the unsignedTx.
signedByVal0, err := TxSignExec(val0.ClientCtx, val0.Address, unsignedTxFile.Name(), "--overwrite", "--sign-mode=amino-json")
require.NoError(err)
signedByVal0File := testutil.WriteToNewTempFile(s.T(), signedByVal0.String())
defer signedByVal0File.Close()

// Then let val1 sign the file with signedByVal0.
val1AccNum, val1Seq, err := val0.ClientCtx.AccountRetriever.GetAccountNumberSequence(val0.ClientCtx, val1.Address)
Expand All @@ -1574,6 +1613,7 @@ func (s *IntegrationTestSuite) TestSignWithMultiSignersAminoJSON() {
)
require.NoError(err)
signedTxFile := testutil.WriteToNewTempFile(s.T(), signedTx.String())
defer signedTxFile.Close()

res, err := TxBroadcastExec(
val0.ClientCtx,
Expand Down Expand Up @@ -1872,6 +1912,7 @@ func (s *IntegrationTestSuite) TestAuxToFeeWithTips() {
} else {
require.NoError(err)
genTxFile := testutil.WriteToNewTempFile(s.T(), string(res.Bytes()))
defer genTxFile.Close()

// broadcast the tx
res, err = TxAuxToFeeExec(
Expand Down
Loading

0 comments on commit 481569c

Please sign in to comment.