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

prevent locking fd table while holding a mutex #90

Merged
merged 1 commit into from
Jan 22, 2020
Merged

prevent locking fd table while holding a mutex #90

merged 1 commit into from
Jan 22, 2020

Conversation

cjihrig
Copy link
Collaborator

@cjihrig cjihrig commented Jan 22, 2020

uvwasi_path_rename(), uvwasi_path_link(), uvwasi_path_open(), and uvwasi_fd_renumber() operate on multiple file descriptors. uvwasi_fd_renumber() has been updated prior to this commit, and is not relevant here. The other three functions would perform
the following locking operations:

  • lock the file table
  • acquire a file descriptor mutex
  • unlock the file table
  • lock the file table again
  • acquire another file descriptor mutex
  • unlock the file table
  • unlock the two mutexes

Attempting to acquire the second mutex introduced the possibility of deadlock because another thread could attempt to acquire the first mutex while holding the file table lock.

This commit ensures that multiple mutexes are either:

  • acquired in a single lock of the file table
  • or, only acquired after releasing previously held mutexes

Fixes: #89
Refs: nodejs/node#31432 (comment)

uvwasi_path_rename(), uvwasi_path_link(), uvwasi_path_open(), and
uvwasi_fd_renumber() operate on multiple file descriptors.
uvwasi_fd_renumber() has been updated prior to this commit, and
is not relevant here. The other three functions would perform
the following locking operations:

- lock the file table
- acquire a file descriptor mutex
- unlock the file table
- unlock the file table again
- acquire another file descriptor mutex
- unlock the file table
- unlock the two mutexes

Attempting to acquire the second mutex introduced the possibility
of deadlock because another thread could attempt to acquire the first
mutex while holding the file table lock.

This commit ensures that multiple mutexes are either:
- acquired in a single lock of the file table
- or, only acquired after releasing previously held mutexes

Fixes: #89
@cjihrig cjihrig merged commit c3bef8e into master Jan 22, 2020
@cjihrig
Copy link
Collaborator Author

cjihrig commented Jan 22, 2020

Thanks for pointing these issues out Ben.

@cjihrig cjihrig deleted the deadlock branch January 22, 2020 15:00
This was referenced Mar 14, 2020
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 this pull request may close these issues.

Potential uvwasi_fd_table_renumber() deadlock
2 participants