Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

accounts, cmd/geth, core: close opened files #29598

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 1 addition & 3 deletions accounts/keystore/keystore.go
Expand Up @@ -312,11 +312,9 @@ func (ks *KeyStore) Unlock(a accounts.Account, passphrase string) error {
// Lock removes the private key with the given address from memory.
func (ks *KeyStore) Lock(addr common.Address) error {
ks.mu.Lock()
defer ks.mu.Unlock()
if unl, found := ks.unlocked[addr]; found {
ks.mu.Unlock()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes the semantics. Previously, it released the lock before invoking ks.expire, but with your changes it retains the lock during the call.

Ostensibly, this was coded like it is for a reason. Please motivate why you consider this change safe/needed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you just want to remove some code, but retain the semantics, you could do

    ks.mu.Lock() 
    unl, found := ks.unlocked[addr]
    ks.mu.Unlock()
    if found{
        ks.expire(addr, unl, time.Duration(0)*time.Nanosecond)
    }
    return nil

ks.expire(addr, unl, time.Duration(0)*time.Nanosecond)
} else {
ks.mu.Unlock()
}
return nil
}
Expand Down
1 change: 1 addition & 0 deletions accounts/scwallet/hub.go
Expand Up @@ -95,6 +95,7 @@ func (hub *Hub) readPairings() error {
}
return err
}
defer pairingFile.Close()

pairingData, err := io.ReadAll(pairingFile)
if err != nil {
Expand Down
2 changes: 2 additions & 0 deletions cmd/geth/logging_test.go
Expand Up @@ -73,6 +73,7 @@ func testConsoleLogging(t *testing.T, format string, tStart, tEnd int) {
if err != nil {
t.Fatal(err)
}
defer readFile.Close()
wantLines := split(readFile)
haveLines := split(bytes.NewBuffer(haveB))
for i, want := range wantLines {
Expand Down Expand Up @@ -109,6 +110,7 @@ func TestJsonLogging(t *testing.T) {
if err != nil {
t.Fatal(err)
}
defer readFile.Close()
wantLines := split(readFile)
haveLines := split(bytes.NewBuffer(haveB))
for i, wantLine := range wantLines {
Expand Down
1 change: 1 addition & 0 deletions cmd/utils/history_test.go
Expand Up @@ -113,6 +113,7 @@ func TestHistoryImportAndExport(t *testing.T) {
if err != nil {
t.Fatalf("error opening era file: %v", err)
}
defer f.Close()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit non-obvious,but era.From takes an io.ReadSeekCloser, and era.Close closes the f. So this change would make it a double-close

var (
h = sha256.New()
buf = bytes.NewBuffer(nil)
Expand Down
1 change: 1 addition & 0 deletions core/mkalloc.go
Expand Up @@ -101,6 +101,7 @@ func main() {
if err != nil {
panic(err)
}
defer file.Close()
if err := json.NewDecoder(file).Decode(g); err != nil {
panic(err)
}
Expand Down