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

Questions regarding MemoryFileSystem and Stdio #143

Open
adrienaury opened this issue Nov 4, 2023 · 8 comments
Open

Questions regarding MemoryFileSystem and Stdio #143

adrienaury opened this issue Nov 4, 2023 · 8 comments
Labels
bug Issue identified by VS Code Team member as probable bug
Milestone

Comments

@adrienaury
Copy link

Hi,

First of all thank you for this extension, this is a great job!

I have few questions regarding MemoryFileSystem and Stdio.

First here is my current code i'm trying to get worked.

const pty = wasm.createPseudoterminal();
const terminal = vscode.window.createTerminal({ name: 'PIMO output', pty, isTransient: true });
terminal.show(true);
try {
            const wasmPath = path.resolve(__dirname, "wasm/pimo-wasi.wasm");
            const bits = await vscode.workspace.fs.readFile(vscode.Uri.file(wasmPath));
            const module = await WebAssembly.compile(bits);

            const fs = await wasm.createMemoryFileSystem();
            fs.createFile("masking.yml", new TextEncoder().encode(masking));

            const output = wasm.createReadable();
            const input = wasm.createWritable();
            await input.write(data);
            await input.write(Uint8Array.from([0x0A,0x04])); // end of input (EOF)

            const mountPoints: MountPointDescriptor[] = [
                { kind: 'workspaceFolder'},
                { kind: 'memoryFileSystem', fileSystem: fs, mountPoint: '/memory' }
            ];
            
            const rootfs = await wasm.createRootFileSystem(mountPoints);

            const stdio: Stdio = {
                in: {kind: "pipeIn", pipe: input},
                out: {kind: "pipeOut", pipe: output},
                err: {kind: "terminal", terminal: pty}
            };
            
            // Create a WASM process.
           const process = await wasm.createProcess('pimo', module, { stdio: stdio, args: ["-c", "/memory/masking.yml", "-v", "5"], rootFileSystem: rootfs, trace: true });
            process.stdout?.onData(data => {
                console.log(new TextDecoder().decode(data));
            });

            // Run the process and wait for its result.
            const result = await process.run();
            console.log("success " + result);
        } catch (error) {
            console.error(error);
        }

My questions are :

1 - the process reads an input of type pipeIn created with a wasm.createWritable() call, but it does not seem to catch the EOF and hang indefinitly waiting for more bytes. How can I indicates to the process that there nothing more and it should end ?
2 - at some point, the process reads a file /memory/masking.yml that is created by a call to fs.createFile("masking.yml", ...), the file exist I can get a stat (it is a regular file) but the process get an error Errno.perm ("Operation is not permitted") when trying to read it (call to path_open). Is it something I'm doing wrong ? or maybe the wasm file is not correct (i use the go compiler 1.21)

Thank you in advance ! Any help appreciated

@adrienaury
Copy link
Author

Also I noticed that the behavior of OpenFlags.create is incorrect if a file already exists.

  • when not specified: the content of the file is truncated (recreated)
  • when specified: the content of the file is ok

@glebbash
Copy link

1 - the process reads an input of type pipeIn created with a wasm.createWritable() call, but it does not seem to catch the EOF and hang indefinitly waiting for more bytes. How can I indicates to the process that there nothing more and it should end ?

Same issue here. Can't find examples or tests that shows how to use that.

ping @dbaeumer

@dbaeumer
Copy link
Member

@adrienaury sorry for the late reply. But I totally overlooked the issue.

Regarding EOF.

This is currently not handled correct in the streams implementation (see https://github.com/microsoft/vscode-wasi/blob/main/wasm-wasi-core/src/common/streams.ts#L68)

What I can do to mimic an OS implementation is to return a Uint8Array of length zero if the implementation detects [0x0A,0x04] in the stream. That should return a read of zero bytes in the calling side.

Has any of you tested how this behaves on another WASI compliant runtime like wasmtime?

@dbaeumer dbaeumer added the info-needed Issue requires more information from poster label Sep 16, 2024
@glebbash
Copy link

glebbash commented Sep 16, 2024

I think checking if any chunk ends with 0x04 (End of Transmission) is a good way to indicate the stdin stream end for vscode-wasm.

About wasmtime:

Running with wasmtime by piping the file to stdin works and fd_read's nread is 0 on stream end.

wasmtime --dir=. lo.wasm -i --inspect < examples/test/demos/hello-world-raw.lo

where lo.wasm -i --inspect is the compiled program and it's args

Here is how the file/stdin is read:

fn fd_read_all(fd: u32) -> Result<Vec<u8>, String> {
    let mut output = Vec::<u8>::new();
    let mut chunk = [0; 256];

    let in_vec = [wasi::Iovec {
        buf: chunk.as_mut_ptr(),
        buf_len: chunk.len(),
    }];

    loop {
        let nread = match unsafe { wasi::fd_read(fd, &in_vec) } {
            Ok(nread) => nread,
            Err(err) => {
                // stdin is empty
                if fd == 0 && err == wasi::ERRNO_AGAIN {
                    break;
                }

                return Err(alloc::format!("Error reading file: fd={fd}, err={err}\n"));
            }
        };

        if nread == 0 {
            break;
        }

        output.extend(&chunk[0..nread]);
    }

    Ok(output)
}

@dbaeumer
Copy link
Member

dbaeumer commented Oct 3, 2024

I added EOT support but I can't do that in a transparent way. EOT (end of transmission) is a terminal feature and not a general stream feature. So if you want a writable with EOT support you need to create it special using

wasm.createWritable({ eot: true, encoding: ... });

@dbaeumer
Copy link
Member

dbaeumer commented Oct 3, 2024

You can also create a normal stream using wasm.createWritable() and then call end() on it when done. That will close the stream as well.

Both will be available in a new version of the extension.

@andy0130tw
Copy link

andy0130tw commented Dec 15, 2024

In a typical UNIX system, the behavior we see after pressing Ctrl-D is not from terminal, but rather the shell program. I feel like that the proposed way of sending 0x04 into the pipe is not how most programs are designed to acknowledge the closing of the stream.

I think that it is the pseudo-terminal (PTY) that should provide the capability to capture and handle common shortcuts. VS Code does not emulate a shell process for the PTY, so it is reasonable that Ctrl-D is passed through. But it would be a DX improvement if it there is a single option to align it with conventional terminal behavior, for example, allowing the user to shutdown the process via Ctrl-C, or to end the stdin stream via Ctrl-D.

@andy0130tw
Copy link

I just came across this PR: 6af74e0. This was exactly the approach I just have proposed. Looking forward to seeing that roll out soon!

@dbaeumer dbaeumer added bug Issue identified by VS Code Team member as probable bug and removed info-needed Issue requires more information from poster labels Dec 18, 2024
@dbaeumer dbaeumer added this to the On Deck milestone Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug
Projects
None yet
Development

No branches or pull requests

4 participants