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

Proposal: add output-stream::(blocking-)close #86

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

badeend
Copy link
Contributor

@badeend badeend commented Aug 22, 2024

Today, there is no generic way to gracefully close a stream, other than to drop it and hope for the best.

Wasi-sockets already has an ad-hoc workaround in the form of tcp-socket::shutdown(send). Similarly (but not exactly the same), wasi-http has outgoing-body::finish.

I'm currently sketching out a TLS interface for WASI, where the TLS client/server does not perform any I/O on its own but instead acts as a stream transformer: cleartext data goes in, TLS data comes out, and vice versa. The stream transformer should not have to know where its inputs & outputs come from and go to. One thing that differentiates TLS from plain TCP is that a graceful shutdown requires additional I/O (a "close_notify" message to be sent).

One way to go about this is to add yet another set of bespoke methods on the TLS client/server resource type to close the output streams.
But preferably the end-of-stream indication should be propagated automagically.

I propose to add the ability to explicitly shut down a writable stream, asynchronously. This is not too far off from how existing programming environments work:

@sunfishcode
Copy link
Member

If I understand this correctly, this would add an effective requirement for consumers of APIs that have output-stream inputs to call close before dropping their handles, or risk being silently incompatible with some types of streams. Since Preview 2 doesn't have close, no current users are calling close.

For example, the get-stdout function returns an output-stream. If this output-stream can be a TLS stream that requires a close call, that would mean that all users of stdout will need to change their code to call close on stdout, or risk being silently incompatible with some types of streams.

Besides compatibility with Preview 2 APIs, there's also the question of how this could be implemented. No major programming language today expects users to do the equivalent of calling close on stdout, so there aren't any clear places where toolchains could insert the close call automatically.

@badeend
Copy link
Contributor Author

badeend commented Aug 26, 2024

If I understand this correctly, this would add an effective requirement for consumers of APIs that have output-stream inputs to call close before dropping their handles, or risk being silently incompatible with some types of streams.

I conceptualize close as the async variant of drop. To prevent issues for existing p2 applications, the existing drop would indeed need to be reinterpreted as being: blocking_close+drop.
Now, blocking inside drop sounds scary, but remember that none of the existing output streams in WASI 0.2.0 & 0.2.1 have anything blocking to do on close, so dropping them without closing will in practice continue to be a non-blocking operation.
Only consumers targeting a future WASI version might actually encounter blocking drops, but by then they'll also have access to the async close.

No major programming language today expects users to do the equivalent of calling close on stdout, so there aren't any clear places where toolchains could insert the close call automatically.

In the case of stdin/out/err specifically, they shouldn't have to be closed. The program may exit with those resources still alive. It is then up to the host to properly await the disposal of the backing implementation. This is already the case regardless of whether close is exposed to the guest or not, right?

@sunfishcode
Copy link
Member

I'm concerned about close and blocking-close being fallible. If we reinterpret the existing drop as blocking_close + drop, what should it do if the blocking-close part fails?

@badeend
Copy link
Contributor Author

badeend commented Aug 26, 2024

I'd say: Discard it.
For similar reason as above: none of the existing WASI 0.2.0 & 0.2.1 output stream close implementations can fail (*), so dropping them will in practice never discard an error.
Only consumers targeting a future WASI version might actually swallow an error on drop, but by then they'll also have access to the blocking-close.

(*) = Actually, POSIX' close can fail. And many programming environments straight up ignore it, which seems to further validate this solution (?)

@sunfishcode
Copy link
Member

The fact that many applications ignore errors from close complicates this issue. If applications don't check for errors from close, then they wouldn't be portable to a system with a fallible close that they're expected to call.

@sunfishcode
Copy link
Member

From talking with @lukewagner about Preview 3, it's likely that Preview 3 streams won't have an equivalent of a fallible close function, so if possible, I think it makes sense to look at adding bespoke methods to the TLS resource to handle this, in anticipation of Preview 3.

As background, the idea for Preview 3 streams is that there won't be a separate input-stream and output-stream that can't be directly composed with each other. There's just a stream type, and if it's used as an argument type then the callee can use it like an input stream, and if it's used as a return type, then a callee creates an output stream and returns it (for comparison, gRPC does a similar thing with stream types.

Pure stream transfomers could have a signture like func(input: stream<u8, error>) -> stream<u8, error>, allowing the output of one to be passed directly to the input of another, and multiple transformers could be directly composed like f(g(h())). In that case, if f experiences a failure when processing data from g, instead of communicating the error back to g, it would return the error back to its caller. That way, there isn't generally a need to make g be aware of errors in f or to wait for completion in f.

That's quite different than the simple resource-based output-stream in Preview 2, so those considerations don't directly apply to Preview 2, but unless there reasons it won't work, it's nice to have Preview 2 anticipate Preview 3 where practical.

@badeend
Copy link
Contributor Author

badeend commented Oct 27, 2024

After discussing this further in person, my current understanding is that with Preview 3 streams:

  • Data flows from the producer to the consumer. (duh.. 🤪 )
  • Closure/failure flows from the producer to the consumer as well. Effectively writing a final result<_, error> into the stream.
  • Cancellation flows from the consumer to the producer. (contrary to the previous two bullets)
  • Writing into a stream can only fail because of cancellation.
  • Reading from a stream can fail with whatever final result<_, error> value was offered by the producer.
  • Whether a component instance is the producer or the consumer of a stream depends on the stream's position in the function's signature:
    • if the stream is passed as an argument to a function: the caller is the producer/writer, the callee is the consumer/reader,
    • if the stream is returned from a function: the callee is the producer/writer, the caller is the consumer/reader,

Mapping this back to Preview 2 streams:

input-stream

input-stream is a pretty clean match for the 'consumer' side as-is. Reading from input-stream can either yield some data or a final stream-error, which in turn can be normal closure (stream-error::closed) or failure (stream-error::last-operation-failed). Additionally, dropping the input-stream can be considered canceling it. One caveat is that dropping the input-stream in P2 is synchronous, whereas P3 cancellation is potentially asynchronous.

In terms of WIT, a P2 signature of:

receive: func() -> input-stream;

can be trivially converted to P3 like so:

receive: func() -> stream<u8, error>;

output-stream

Even ignoring the whole check-write/write/flush dance, output-stream doesn't cleanly map to any single thing in P3. The main incompatibility is in the direction closure & errors flow. In P3 it is the producer who closes the stream (optionally with an error). In P2, the producer receives errors/closure.

Given a P2 WIT signature of:

send: func() -> output-stream;

the P3 equivalent is not:

send: func(data: stream<u8, error>);

but rather:

send: func(data: stream<u8>) -> future<result<_, error>>; // The future resolves successfully when the stream has closed and all data has been sent out. If an error occurs during sending, the input stream is canceled and the future resolves with the error.

where today's output-stream encapsulates both the stream parameter and the returned future. The P2 write method can be expressed in P3 roughly as:

async fn p2_write(&mut self, data: List<u8>) -> result<_, error> {
    match self.p3_stream.write(data).await {
      Ok(()) => Ok(()),
      Err(()) => // We've been canceled. Let's find out why:
        match self.p3_future.await {
          Ok(()) => StreamError::Closed,
          Err(e) => StreamError::LastOperationFailed(e),
        }
    }
}

close

Getting back to the subject of close: I'd say that the proposed P2 (blocking-)close function is equivalent to P3:

async fn p2_close(&mut self) -> result<_, error> {
    self.p3_stream.close(); // This is indeed not async nor fallible.
    self.p3_future.await // This is where asynchronicity & fallibility comes from.
}

Strictly speaking, for full P3-compatibility, the P2 close function should even have an optional Error argument:

async fn p2_close(&mut self, error: Option<Error>) -> result<_, error> {
    self.p3_stream.close(error);
    self.p3_future.await
}

Note that the P2 close operation itself is not fallible. Calling close definitely closes the stream. The result return type is used to surface errors that have happened between the previous write and calling close.

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.

2 participants