Skip to content

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