Skip to content

Commit 48cc729

Browse files
Arshia001Copilot
andcommitted
Add test, address review comments, fix implementation
Co-authored-by: Copilot <copilot@github.com>
1 parent 98db504 commit 48cc729

7 files changed

Lines changed: 238 additions & 14 deletions

File tree

lib/wasix/src/syscalls/mod.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -340,11 +340,18 @@ where
340340
if let Poll::Ready(res) = Pin::new(&mut self.pinned_work).poll(cx) {
341341
return Poll::Ready(Ok(res));
342342
}
343-
WasiEnv::do_pending_link_operations(self.ctx, false);
343+
if let Err(err) = WasiEnv::do_pending_link_operations(self.ctx, false) {
344+
return Poll::Ready(Err(err));
345+
}
344346
if let Some(signals) = self.ctx.data().thread.pop_signals_or_subscribe(cx.waker()) {
347+
let only_sigwakeup = signals.iter().all(|sig| matches!(sig, Signal::Sigwakeup));
345348
if let Err(err) = WasiEnv::process_signals_internal(self.ctx, signals) {
346349
return Poll::Ready(Err(err));
347350
}
351+
if only_sigwakeup {
352+
self.ctx.data().thread.signals_subscribe(cx.waker());
353+
return Poll::Pending;
354+
}
348355
return Poll::Ready(Ok(Err(Errno::Intr)));
349356
}
350357
Poll::Pending
@@ -382,7 +389,9 @@ where
382389
return Poll::Ready(Ok(res));
383390
}
384391

385-
WasiEnv::do_pending_link_operations(self.ctx, false);
392+
if let Err(err) = WasiEnv::do_pending_link_operations(self.ctx, false) {
393+
return Poll::Ready(Err(err));
394+
}
386395

387396
let env = self.ctx.data();
388397
if let Some(forced_exit) = env.thread.try_join() {

lib/wasix/src/syscalls/wasi/fd_read.rs

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -307,12 +307,14 @@ pub(crate) fn fd_read_internal<M: MemorySize>(
307307
for buf in unsafe { iov_bufs.iter_uninit_slices_mut() } {
308308
let local_read = socket
309309
.recv(tasks.deref(), buf, Some(timeout), nonblocking, false)
310-
.await?;
310+
.await;
311+
let local_read = match local_read {
312+
Ok(n) => n,
313+
Err(Errno::Again) | Err(Errno::Timedout) if total_read > 0 => break,
314+
Err(err) => return Err(err),
315+
};
311316
total_read += local_read;
312-
// A zero-byte return signals connection closed (EOF);
313-
// a short read is normal for stream sockets and does NOT
314-
// indicate end-of-stream.
315-
if local_read == 0 {
317+
if local_read < buf.len() {
316318
break;
317319
}
318320
}
@@ -338,14 +340,17 @@ pub(crate) fn fd_read_internal<M: MemorySize>(
338340
let res = __asyncify(ctx, asyncify_timeout, async move {
339341
let mut total_read = 0usize;
340342
for buf in unsafe { iov_bufs.iter_slices_mut() } {
341-
let buf_len = buf.len();
342343
let local_read = if nonblocking {
343-
rx.try_read(buf).ok_or(Errno::Again)?
344+
match rx.try_read(buf) {
345+
Some(n) => n,
346+
None if total_read > 0 => break,
347+
None => return Err(Errno::Again),
348+
}
344349
} else {
345350
virtual_fs::AsyncReadExt::read(&mut rx, buf).await?
346351
};
347352
total_read += local_read;
348-
if local_read < buf_len {
353+
if local_read < buf.len() {
349354
break;
350355
}
351356
}
@@ -364,14 +369,17 @@ pub(crate) fn fd_read_internal<M: MemorySize>(
364369
let res = __asyncify(ctx, asyncify_timeout, async move {
365370
let mut total_read = 0usize;
366371
for buf in unsafe { iov_bufs.iter_slices_mut() } {
367-
let buf_len = buf.len();
368372
let local_read = if nonblocking {
369-
pipe.try_read(buf).ok_or(Errno::Again)?
373+
match pipe.try_read(buf) {
374+
Some(n) => n,
375+
None if total_read > 0 => break,
376+
None => return Err(Errno::Again),
377+
}
370378
} else {
371379
virtual_fs::AsyncReadExt::read(&mut pipe, buf).await?
372380
};
373381
total_read += local_read;
374-
if local_read < buf_len {
382+
if local_read < buf.len() {
375383
break;
376384
}
377385
}
Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,35 @@
1-
use super::{run_build_script, run_wasm};
1+
use super::{run_build_script, run_wasm, run_wasm_with_stdin};
2+
use wasmer_wasix::Pipe;
23

34
#[test]
45
fn test_fd_allocate() {
56
let wasm = run_build_script(file!(), "fd-allocate").unwrap();
67
run_wasm(&wasm, wasm.parent().unwrap()).unwrap();
78
}
9+
10+
/// Regression test for fd_read blocking DL operations.
11+
///
12+
/// One WASM thread blocks inside `fd_read` on stdin (which never produces
13+
/// data). While that thread is parked, the main WASM thread must be able to
14+
/// call `dlopen` and load a shared library without deadlocking. Before the
15+
/// fix, `fd_read` held a lock that prevented DL operations from proceeding.
16+
#[test]
17+
fn test_stdin_read_does_not_block_dlopen() {
18+
let wasm = run_build_script(file!(), "stdin-dlopen-race").unwrap();
19+
20+
// Keep the write end alive so stdin remains blocked for the whole guest
21+
// run; the guest exits explicitly after proving dlopen works.
22+
let (_pipe_tx, pipe_rx) = Pipe::channel();
23+
24+
let result =
25+
run_wasm_with_stdin(&wasm, wasm.parent().unwrap(), Box::new(pipe_rx)).unwrap();
26+
27+
let stdout = String::from_utf8_lossy(&result.stdout);
28+
assert_eq!(
29+
stdout.trim(),
30+
"reader_ready\ndlopen_succeeded_after_reader_ready\nside_value=42\nsequence_ok",
31+
"stderr: {}",
32+
String::from_utf8_lossy(&result.stderr)
33+
);
34+
assert_eq!(result.exit_code, Some(0));
35+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
#!/bin/bash
2+
set -ex
3+
4+
export WASIXCC_PIC=yes
5+
6+
# Compile the shared library
7+
$CC -shared side.c -o libside.so
8+
9+
# Compile the main executable
10+
$CC main.c -o main
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
#include <dlfcn.h>
2+
#include <pthread.h>
3+
#include <stdio.h>
4+
#include <stdlib.h>
5+
#include <unistd.h>
6+
7+
static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
8+
static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
9+
static int reader_ready = 0;
10+
11+
typedef int (*side_value_t)(void);
12+
13+
static void *reader_thread(void *arg) {
14+
(void)arg;
15+
16+
/* Signal to the main thread that we are about to enter fd_read. */
17+
pthread_mutex_lock(&mutex);
18+
reader_ready = 1;
19+
pthread_cond_signal(&cond);
20+
pthread_mutex_unlock(&mutex);
21+
22+
/* Block in fd_read on stdin. The host provides a pipe whose write end is
23+
* never written to, so this call parks indefinitely. The process will be
24+
* terminated by main() returning before this ever unblocks. */
25+
char buf[64];
26+
read(STDIN_FILENO, buf, sizeof(buf));
27+
28+
return NULL;
29+
}
30+
31+
int main(void) {
32+
pthread_t t;
33+
pthread_create(&t, NULL, reader_thread, NULL);
34+
35+
/* Wait until the reader thread has set reader_ready (it is about to call
36+
* read(), if it hasn't already). */
37+
pthread_mutex_lock(&mutex);
38+
while (!reader_ready)
39+
pthread_cond_wait(&cond, &mutex);
40+
pthread_mutex_unlock(&mutex);
41+
42+
printf("reader_ready\n");
43+
44+
/* Give the reader thread time to actually enter the blocking read before
45+
* attempting dlopen, so this test exercises the intended race reliably. */
46+
sleep(1);
47+
48+
/* Load a shared library while the other thread is blocked in fd_read.
49+
* Before the fix this would deadlock because fd_read held a lock that
50+
* the DL subsystem also needed. */
51+
void *handle = dlopen("libside.so", RTLD_NOW | RTLD_LOCAL);
52+
if (!handle) {
53+
fprintf(stderr, "dlopen failed: %s\n", dlerror());
54+
return 1;
55+
}
56+
57+
printf("dlopen_succeeded_after_reader_ready\n");
58+
59+
side_value_t side_value = (side_value_t)dlsym(handle, "side_value");
60+
if (!side_value) {
61+
fprintf(stderr, "dlsym failed: %s\n", dlerror());
62+
dlclose(handle);
63+
return 1;
64+
}
65+
66+
printf("side_value=%d\n", side_value());
67+
printf("sequence_ok\n");
68+
dlclose(handle);
69+
70+
fflush(stdout);
71+
_Exit(0);
72+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
int side_value(void) { return 42; }

lib/wasix/tests/wasm_tests/mod.rs

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,102 @@ pub fn run_wasm_with_result(
422422
})
423423
}
424424

425+
/// Run a compiled WASM file using WasiRunner with a custom stdin.
426+
///
427+
/// Useful for testing syscalls that block on stdin (e.g. fd_read): pass the
428+
/// receiving end of a `Pipe::channel()` and keep the sending end alive in the
429+
/// caller so that stdin never returns EOF while the guest is running.
430+
#[allow(unused)]
431+
pub fn run_wasm_with_stdin(
432+
wasm_path: &PathBuf,
433+
dir: &Path,
434+
stdin: Box<dyn wasmer_wasix::VirtualFile + Send + Sync>,
435+
) -> Result<WasmRunResult, anyhow::Error> {
436+
let wasm_bytes = std::fs::read(wasm_path)?;
437+
let engine = create_engine_for_wasm(&wasm_bytes);
438+
let module_data = HashedModuleData::new(wasm_bytes);
439+
let hash = *module_data.hash();
440+
441+
let stdout_buffer = Arc::new(Mutex::new(Vec::new()));
442+
let stderr_buffer = Arc::new(Mutex::new(Vec::new()));
443+
let stdout_capture = Box::new(CaptureFile::new(stdout_buffer.clone()));
444+
let stderr_capture = Box::new(CaptureFile::new(stderr_buffer.clone()));
445+
446+
let rt = create_runtime();
447+
let result = rt.block_on(async {
448+
let cache_dir = get_cache_dir();
449+
std::fs::create_dir_all(&cache_dir).ok();
450+
451+
let rt_handle = wasmer_wasix::runtime::task_manager::tokio::RuntimeOrHandle::Handle(
452+
tokio::runtime::Handle::current(),
453+
);
454+
let tokio_task_manager =
455+
Arc::new(wasmer_wasix::runtime::task_manager::tokio::TokioTaskManager::new(rt_handle));
456+
let module_cache = wasmer_wasix::runtime::module_cache::SharedCache::default()
457+
.with_fallback(wasmer_wasix::runtime::module_cache::FileSystemCache::new(
458+
cache_dir,
459+
tokio_task_manager,
460+
));
461+
let arc_cache = Arc::new(module_cache);
462+
463+
let module = wasmer_wasix::runtime::load_module(
464+
&engine,
465+
&arc_cache,
466+
wasmer_wasix::runtime::ModuleInput::Hashed(Cow::Borrowed(&module_data)),
467+
None,
468+
)
469+
.await
470+
.map_err(|e| anyhow::anyhow!("Failed to load module: {}", e))?;
471+
472+
tokio::task::block_in_place(move || {
473+
let mut runner = WasiRunner::new();
474+
runner
475+
.with_mapped_directories([MappedDirectory {
476+
guest: dir.to_string_lossy().to_string(),
477+
host: dir.to_path_buf(),
478+
}])
479+
.with_mapped_directories([MappedDirectory {
480+
guest: "/lib".to_string(),
481+
host: dir.to_path_buf(),
482+
}])
483+
.with_current_dir(dir.to_string_lossy().to_string())
484+
.with_stdin(stdin)
485+
.with_stdout(stdout_capture)
486+
.with_stderr(stderr_capture);
487+
runner.run_wasm(
488+
RuntimeOrEngine::Engine(engine),
489+
wasm_path.to_string_lossy().as_ref(),
490+
module,
491+
hash,
492+
)
493+
})
494+
});
495+
496+
let stdout = stdout_buffer.lock().unwrap().clone();
497+
let stderr = stderr_buffer.lock().unwrap().clone();
498+
let exit_code = match &result {
499+
Ok(_) => Some(0),
500+
Err(e) => {
501+
let error_msg = e.to_string();
502+
if let Some(code_str) = error_msg.split("ExitCode::").nth(1) {
503+
if let Some(code) = code_str.split_whitespace().next() {
504+
code.parse::<i32>().ok()
505+
} else {
506+
None
507+
}
508+
} else {
509+
None
510+
}
511+
}
512+
};
513+
514+
Ok(WasmRunResult {
515+
stdout,
516+
stderr,
517+
exit_code,
518+
})
519+
}
520+
425521
/// Run a compiled WASM file using WasiRunner
426522
#[allow(unused)]
427523
pub fn run_wasm(wasm_path: &PathBuf, dir: &Path) -> Result<(), anyhow::Error> {

0 commit comments

Comments
 (0)