In wasmtime CLI, passing --tcplisten argument breaks --dir

[SteveSandersonMS]

Test Case

testcase.zip

Steps to Reproduce

1. Extract main.wasm from the test case zip file. Or compile it yourself using WASI SDK and the following source code, e.g. via ~/wasi-sdk/bin/clang main.c --sysroot ~/wasi-sdk/share/wasi-sysroot -o main.wasm if you have WASI SDK at ~/wasi-sdk.

#include <stdio.h>
#include <sys/types.h>
#include <dirent.h>

int main (void) {
  DIR *dp = opendir ("./");
  struct dirent *ep;

  if (dp != NULL) {
    while ((ep = readdir (dp)))
      printf("%s\n", ep->d_name);
    closedir(dp);
  }
  else
    printf("Couldn't open the directory\n");

  return 0;
}

2. Run wasmtime main.wasm --dir=. and observe it print a directory listing
3. Run wasmtime main.wasm --dir=. --tcplisten=127.0.0.1:9000 and observe it fail to do so (it will print Couldn't open the directory)

Expected Results

I expect that --tcplisten should not break the directory mapping.

Actual Results

Passing --tcplisten makes it behave as if you didn't pass --dir=..

Versions and Environment

Wasmtime version or commit: 0.35.1

Operating system: linux

Architecture: x86_64

Extra Info

Other than this, --tcplisten works brilliantly and I've been able to implement a nontrivial web server application using this flag and the new sock_accept API.

[sunfishcode]

CC @haraldh. It's possible this is related to how WASI programs discover the preopened sockets. Would it work to do the self.compute_preopen_dirs() before the self.compute_preopen_sockets(), so that the directories have lower file descriptor numbers and are found first?

This is an area where, in the future, Typed Main will help: instead of having WASI programs search through the file descriptor space to find preopens, we ideally want them provided explicitly.

[tschneidereit]

@haraldh is this something you might be able to look into? Addressing this looks like it'd help make --tcplisten quite a bit more useful :)

[haraldh]

yes, will do

[haraldh]

Ok, here is the analysis.

• socket gets fd 3
• directory gets fd 4
• wasi-libc/libc-bottom-half/sources/preopens.c:__wasilibc_populate_preopens() calls for every fd >= 3 fd_prestat_get() and internally registers directories. But if it encounters an error, like with fd 3 straight away, it bails out.

So with a WASI change like this:

diff --git a/phases/snapshot/witx/typenames.witx b/phases/snapshot/witx/typenames.witx
index 893e5b2..6aef907 100644
--- a/phases/snapshot/witx/typenames.witx
+++ b/phases/snapshot/witx/typenames.witx
@@ -730,6 +730,7 @@
   (enum (@witx tag u8)
     ;;; A pre-opened directory.
     $dir
+    $listen_socket
   )
 )
 
@@ -741,10 +742,19 @@
   )
 )
 
+;;; The contents of a $prestat when type is `preopentype::listen_socket`.
+(typename $prestat_listen_socket
+  (record
+    ;;; The length of the address string for use with `fd_prestat_socket_addr`.
+    (field $pr_addr_len $size)
+  )
+)
+
 ;;; Information about a pre-opened capability.
 (typename $prestat
   (union (@witx tag $preopentype)
     $prestat_dir
+    $prestat_listen_socket
   )
 )

we could fix wasmtime like this:

diff --git a/crates/wasi-common/src/snapshots/preview_0.rs b/crates/wasi-common/src/snapshots/preview_0.rs
index 96f86820b..8bdc2039a 100644
--- a/crates/wasi-common/src/snapshots/preview_0.rs
+++ b/crates/wasi-common/src/snapshots/preview_0.rs
@@ -198,6 +198,7 @@ impl From<snapshot1_types::Prestat> for types::Prestat {
     fn from(p: snapshot1_types::Prestat) -> types::Prestat {
         match p {
             snapshot1_types::Prestat::Dir(d) => types::Prestat::Dir(d.into()),
+            snapshot1_types::Prestat::ListenSocket(_) => todo!(),
         }
     }
 }
diff --git a/crates/wasi-common/src/snapshots/preview_1.rs b/crates/wasi-common/src/snapshots/preview_1.rs
index 9c6f372d3..758b9da61 100644
--- a/crates/wasi-common/src/snapshots/preview_1.rs
+++ b/crates/wasi-common/src/snapshots/preview_1.rs
@@ -554,13 +554,26 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
 
     async fn fd_prestat_get(&mut self, fd: types::Fd) -> Result<types::Prestat, Error> {
         let table = self.table();
-        let dir_entry: &DirEntry = table.get(u32::from(fd)).map_err(|_| Error::badf())?;
-        if let Some(ref preopen) = dir_entry.preopen_path() {
-            let path_str = preopen.to_str().ok_or_else(|| Error::not_supported())?;
-            let pr_name_len = u32::try_from(path_str.as_bytes().len())?;
-            Ok(types::Prestat::Dir(types::PrestatDir { pr_name_len }))
+        if let Ok(dir_entry) = table.get::<DirEntry>(u32::from(fd)) {
+            if let Some(ref preopen) = dir_entry.preopen_path() {
+                let path_str = preopen.to_str().ok_or_else(|| Error::not_supported())?;
+                let pr_name_len = u32::try_from(path_str.as_bytes().len())?;
+                Ok(types::Prestat::Dir(types::PrestatDir { pr_name_len }))
+            } else {
+                Err(Error::not_supported().context("file is not a preopen"))
+            }
+        } else if let Ok(file_entry) = table.get_mut::<FileEntry>(u32::from(fd)) {
+            let fdstat = file_entry.get_fdstat().await?;
+            if matches!(fdstat.filetype, FileType::SocketStream) {
+                let pr_addr_len = 0; // TODO
+                Ok(types::Prestat::ListenSocket(types::PrestatListenSocket {
+                    pr_addr_len,
+                }))
+            } else {
+                Err(Error::not_supported().context("file is not a preopen"))
+            }
         } else {
-            Err(Error::not_supported().context("file is not a preopen"))
+            Err(Error::badf())
         }
     }

which would fix the code of this issue.

[haraldh]

The quick "band-aid" fix is #3995

[SteveSandersonMS]

Thanks for looking into this, @haraldh!

Just to clarify, how is application code meant to know which file descriptor numbers to pass to sock_accept? Is this a gap in the WASI spec, or does it say how they should be ordered or discovered? I could imagine two different WASI hosts might take different approaches (with the PR, wasmtime might put directory preopens first, but another runtime might put TCP listeners first).

Is there a recommended pattern of calls for application code to discover the available TCP listener file descriptors?

cc @sunfishcode

[bjorn3]

Is there a recommended pattern of calls for application code to discover the available TCP listener file descriptors?

Using the LISTEN_FDS env var. This env var is also used by systemd socket activation. According to man sd_listen_fds the first socket must be at fd 3 (so preopened directories must be after the preopened sockets, not before) and there must be $LISTEN_FDS sockets passed in.

[haraldh]

A combination of fd_prestat_get() for the directories and fd_fdstat_get() checking fs_filetype == SocketStream could be used without extending the WASI spec.

[SteveSandersonMS]

Thanks for the info! That's really useful.

preopened directories must be after the preopened sockets, not before

Does the "band-aid" fix (#3995) satisfy this condition? I did try out the patch given above and observed that it did still supply listener FDs starting from 3, even when there's also a directory preopen.

[haraldh]

Is there a recommended pattern of calls for application code to discover the available TCP listener file descriptors?

Using the LISTEN_FDS env var. This env var is also used by systemd socket activation. According to man sd_listen_fds the first socket must be at fd 3 (so preopened directories must be after the preopened sockets, not before) and there must be $LISTEN_FDS sockets passed in.

Or we have to increment the LISTEN_FDS env var by the number of the preopened directories (and also LISTEN_FDNAMES).

[haraldh]

Thanks for the info! That's really useful.

preopened directories must be after the preopened sockets, not before

Does the "band-aid" fix (#3995) satisfy this condition? I did try out the patch given above and observed that it did still supply listener FDs starting from 3, even when there's also a directory preopen.

The "band-aid" fix violates this condition... The fix with the WASI spec change satisfies it.

[npmccallum]

@bjorn3 One problem with LISTEN_FDS is that it is only used for fd >= 3. What if an application has fewer file descriptors?

[bjorn3]

Fds 0, 1 and 2 are reserved for stdin, stdout and stderr respectively. Wasi doesn't allow them to be absent AFAIK.

[haraldh]

Anyway, to get into this situation, you have to deliberately use the new command line features or the new WasiCtx methods and as I said,

A combination of fd_prestat_get() for the directories and fd_fdstat_get() checking fs_filetype == SocketStream could be used without extending the WASI spec.

So, instead of DIR *dp = opendir ("./"); one would use DIR *dp = fdopendir (fd);

[npmccallum]

@bjorn3 I don't see stdio in the snapshot at all.

[bjorn3]

What snapshot?

[npmccallum]

@bjorn3 https://github.com/WebAssembly/WASI/blob/main/phases/snapshot/docs.md

[bjorn3]

Both the wasi impl in wasmtime and wasi-libc assume the first three fds are stdio:

wasmtime/crates/wasi-common/src/ctx.rs

Lines 35 to 37 in 76f7cde

	s.set_stdin(Box::new(crate::pipe::ReadPipe::new(std::io::empty())));
	s.set_stdout(Box::new(crate::pipe::WritePipe::new(std::io::sink())));
	s.set_stderr(Box::new(crate::pipe::WritePipe::new(std::io::sink())));

https://github.com/WebAssembly/wasi-libc/blob/079adff840032c3455eb1cb34dc9ceaa0b2bfc0c/libc-bottom-half/sources/preopens.c#L213-L215

[sdeleuze]

Hi, thanks for raising this since I hit the same problem when implementing a web server that serve static files. I am wondering if there is a consensus on how to fix it? Could be the case based on this discussion above but I am not 100% sure.

@bjorn3 said:

According to man sd_listen_fds the first socket must be at fd 3 (so preopened directories must be after the preopened sockets, not before)

Then @haraldh said:

A combination of fd_prestat_get() for the directories and fd_fdstat_get() checking fs_filetype == SocketStream could be used without extending the WASI spec.

So is it correct to assume that it is possible to fix this issue without extending the WASI spec, in a compliant way with man sd_listen_fds (preopened directories must be after the preopened sockets) just by refining how Wasmtime handles this?

Happy to try crafting a PR with proper guidance.

[npmccallum]

Both the wasi impl in wasmtime and wasi-libc assume the first three fds are stdio:

wasmtime/crates/wasi-common/src/ctx.rs

Lines 35 to 37 in 76f7cde

	s.set_stdin(Box::new(crate::pipe::ReadPipe::new(std::io::empty())));
	s.set_stdout(Box::new(crate::pipe::WritePipe::new(std::io::sink())));
	s.set_stderr(Box::new(crate::pipe::WritePipe::new(std::io::sink())));

https://github.com/WebAssembly/wasi-libc/blob/079adff840032c3455eb1cb34dc9ceaa0b2bfc0c/libc-bottom-half/sources/preopens.c#L213-L215

But it isn't in the standard. Implementations can assume anything they want. It doesn't make it standard behavior.

[bjorn3]

Anything that wants to use wasi-libc implicitly assumes this. Assemblyscript also assumes it (https://github.com/AssemblyScript/assemblyscript/blob/d884ac8032b2bfa0caf26d4dc11d99c5a9543c13/std/assembly/wasi/index.ts#L57) Nodejs also uses 0, 1 and 2 for stdin, stdout and stderr by default. Because of this IMO it should just be added to the actual wasi standard as it is already de-facto standard, just not de-jure.

[npmccallum]

@bjorn3 But this is an example of a host feature "bleeding" into the spec. In Enarx, the host stdio is untrusted and shouldn't be connected to the encrypted guest wasm instance. I do not thing we should presume that just because everyone assumes it that it should be added to WASI uncritically.

[bjorn3]

It isn't required that the host stdio is connected to the guest stdio. The guest can have /dev/null or a file on which every operations fails as stdio if there is no stdio to pass through.

[sunfishcode]

WASI is in the process of moving to interface types and Typed Main, at which point I expect we'll completely overhaul the way file descriptors are passed into programs. The accept feature here is about enabling certain functionality with minimal changes to the current infrastructure.

Within the current infrastructure, fds 0,1,2 are effectively reserved for stdin, stdout, stderr. wasi-libc assumes this. This isn't how POSIX would do things, or how Linux would do them, but it's how the current infrastructure works, so the way to make things work with minimal changes is just to teach everything that 0,1,2 are reserved for stdin, stdout, and stderr.

[tiran]

Hi, is there any progress on this issue? I would like to use --tcplisten to experiment with sockets from CPython WASI port. For now Python also requires --mapdir or --dir to mount Python's stdlib.

[squillace]

@sunfishcode is there any resolution we might work toward here? This continues to break a few things.

[squillace]

it appears that #3995 should fix it, but...

[alexcrichton]

@squillace is it possible to use components instead and use the wasi:sockets proposal? Not much thought has gone into trying to backfill the socket APIs of preview1 with the implementations of preview2 because it was assumed that there weren't all that many users of preview1 sockets. This may be possible to get working somewhat, but in general it'd probably be best to start riding the preview2/component train if possible.

[squillace]

we'll have a look to see whether the wasi-sockets proposal is ready for freddy. In general, there'll be a choice that language runtimes make here, so for them this is a fork in the road, so to speak....

[thaystg]

Hi, @alexcrichton , currently we use the sock_accept importing the function like this in our C code:

__attribute__((import_module("wasi_snapshot_preview1"))) __attribute__((import_name("sock_accept"))) int sock_accept(int fd, int fdflags, int* result_ptr);

How should I use components to replace it? Is there any documentation that I can follow?

[alexcrichton]

@thaystg I think that, unfortunately, the answer won't be so simple. If using the component model and/or preview 2 you wouldn't have access to that API exactly but you would have access to other wasi:sockets APIs. (which also support the functionality of sock_accept for example). To use that exact import, however, it won't work with preview2.

If you're interested though it's theoretically possible to get it working. Wasmtime has an in-guest adapter which translates preview1 to preview2, and this is how binaries don't have to explicitly update to preview2 yet to make use of it. The way this works is that the adapter exports functions like fd_read which implement preview1 in terms of preview2. This has not yet been done for sock_accept and other socket-related functions. The reason for this is primarily time and/or motivation, not technical. So if you're feeling intrepid I think it would be reasonable to fill out the implementation there.

To set expectations, though, I don't expect that filling out the adapter for socket-related functionality is going to be an easy task. It's god a nontrivial design component to it along with implementation concerns too. That's all surmountable in the technical sense which is why I'm mentioning this. I'm not aware of anyone else who's lined up to implement this, however, and so far I don't believe this is considered a blocker or todo item for the preview2 timeline, only a nice-to-have if someone is interested in implementing it.

[squillace]

Hey @alexcrichton thanks; so to sum up: Re: #3936 (comment) we should jump on the components train but in the components train we have to implement something that wasmtime experts should probably design. Do I have this correct?

If so, I'd be a bit more interested hearing about This may be possible to get working somewhat for preview 1. Of course, it's always a question of time and resources. .NET needs the socket work to do the debugging jump into wasmtime from another location; others do, too. We can try to find resources to apply here, but I'm not clear we'd apply the same resources to each path.

Your take?

[alexcrichton]

Almost yeah, although the component part doesn't require any work on Wasmtime's side, but it may require more work on y'all's side if you're not already using components.

I can try to summarize as well a bit if that helps. There's more-or-less three different ways to go about solving this:

Fix this single issue in isolation

There's quite a bit of discussion on this thread, and for example I haven't reviewed the above possible fix mentioned. Fixing this issue in isolation will require confirming that it actually works with wasi-libc which seems like a hazard given the above discussion. I haven't dug into this myself because sockets in preview1 are not something we'd like to breathe more life into at this time. Instead we're looking ideally to encourage users to use the preview2 implementation where possible. That being said from a perspective of "I just want something working" this is probably the easiest route since it has nothing to do with components and it's probably just taking existing stuff and wiring it together a bit differently.

Implement sock_accept in the preview1 adapter.

This effectively boils down to my above thoughts in filling out this function. This requires writing code in the adapter which is not a trivial task. This additionally is something I'd ideally prefer to avoid if we can to again avoid breathing more life into preview1 sockets. Additionally a major snag here is that preview1 has no way to open a socket, meaning that any sockets in use today must have been supplied as a preopen. WASI preview2 has no mechanism, at this time at least, for preopened sockets. It has preopened directories but not preopened sockets. This means that even if the preview1 adapter were filled out it likely wouldn't work for your use case as-is because you'd still have no means of configuring a preopened socket on the host.

Use components

Using components in their entirety involves directly using the wasi:sockets APIs which gives you access to opening sockets, accepting sockets, etc. This all works as-is today and requires no changes to Wasmtime. Depending on how you're running components though this may require changes on your end which may be expansive.

---

If you'd like I can also talk through some of these routes in more detail over video. Otherwise also happy to follow-up here too.

[squillace]

Thanks, @alexcrichton, I appreciate this. We are intending to target components directly, so skating to where the puck WILL be is better, I think, though it will require us to do more work in the shorter term that we eventually intended to do. @SteveSandersonMS and @thaystg let's understand how to tackle this and come back to the conversation.

[Archie3d]

Is there a workaround to use mapdir and tcplisten simultaneously? wasmtime seems to not recognize --mapdir argument when any other argument is used.