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

nodeDb.DeleteVersionsFrom can leave mutex locked indefinitely hence a deadlock #842

Closed
2 tasks
odeke-em opened this issue Oct 12, 2023 · 0 comments · Fixed by #843
Closed
2 tasks

nodeDb.DeleteVersionsFrom can leave mutex locked indefinitely hence a deadlock #842

odeke-em opened this issue Oct 12, 2023 · 0 comments · Fixed by #843

Comments

@odeke-em
Copy link
Contributor

Noticed in an audit and while preparing a test for P #838 that this code exists

iavl/nodedb.go

Lines 493 to 499 in 3574b99

ndb.mtx.Lock()
for v, r := range ndb.versionReaders {
if v >= fromVersion && r != 0 {
return fmt.Errorf("unable to delete version %v with %v active readers", v, r)
}
}
ndb.mtx.Unlock()

but in case the erroring condition matches, it'll be locked forever. Let's fix that by 2 ways

  • A dedicated closure with an error return
err := func() error {
	ndb.mtx.Lock()
	defer ndb.mtx.Unlock()

	for v, r := range ndb.versionReaders {
		if v >= fromVersion && r != 0 {
			return fmt.Errorf("unable to delete version %v with %v active readers", v, r)
		}
	}
         return nil
}()
  • Manually unlocking in the error path
	ndb.mtx.Lock()
	for v, r := range ndb.versionReaders {
		if v >= fromVersion && r != 0 {
                         ndb.mtx.Unlock()
			return fmt.Errorf("unable to delete version %v with %v active readers", v, r)
		}
	}
	ndb.mtx.Unlock()

/cc @elias-orijtech

odeke-em added a commit to orijtech/iavl that referenced this issue Oct 13, 2023
Noticed during an audit that there can be a deadlock resulting from
an unreleased mutex if there happens to be a newer version with one or
more concurrent readers.

Fixes cosmos#842
odeke-em added a commit to orijtech/iavl that referenced this issue Oct 16, 2023
…n error

Noticed during an audit that there can be a deadlock resulting from
an unreleased mutex if there happens to be a newer version with one or
more concurrent readers.

Fixes cosmos#842
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant