Skip to content

Commit

Permalink
Remove extra lock-taking in preopen setup (#491)
Browse files Browse the repository at this point in the history
Issue [#8392] in Wasmtime highlights how preopen registration can result
in a hang when compiled for a threaded environment. The problem is that
`internal_register_preopened_fd_unlocked` assumes it will not touch the
global lock because its caller, `__wasilibc_populate_preopens`, already
has taken the lock. Unfortunately, a refactoring in #408 (which
introduces `internal_register_preopened_fd_unlocked`) did not catch that
the `resize` function called internally also takes the global lock. This
change removes that locking in `resize` under the assumption that it
will only be called when the lock is already taken.

[#8392]: bytecodealliance/wasmtime#8392
  • Loading branch information
abrown committed May 2, 2024
1 parent 129ee9b commit 8876138
Showing 1 changed file with 1 addition and 5 deletions.
6 changes: 1 addition & 5 deletions libc-bottom-half/sources/preopens.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,13 @@ static void assert_invariants(void) {

/// Allocate space for more preopens. Returns 0 on success and -1 on failure.
static int resize(void) {
LOCK(lock);
size_t start_capacity = 4;
size_t old_capacity = preopen_capacity;
size_t new_capacity = old_capacity == 0 ? start_capacity : old_capacity * 2;

preopen *old_preopens = preopens;
preopen *new_preopens = calloc(sizeof(preopen), new_capacity);
if (new_preopens == NULL) {
UNLOCK(lock);
return -1;
}

Expand All @@ -77,7 +75,6 @@ static int resize(void) {
free(old_preopens);

assert_invariants();
UNLOCK(lock);
return 0;
}

Expand All @@ -101,8 +98,7 @@ static const char *strip_prefixes(const char *path) {
return path;
}

/// Similar to `internal_register_preopened_fd_unlocked` but does not
/// take a lock.
/// Similar to `internal_register_preopened_fd` but does not take a lock.
static int internal_register_preopened_fd_unlocked(__wasi_fd_t fd, const char *relprefix) {
// Check preconditions.
assert_invariants();
Expand Down

0 comments on commit 8876138

Please sign in to comment.