Skip to content

Change stream results to indicate cancellation - #498

Merged
lukewagner merged 1 commit into
mainfrom
copy-result
Apr 14, 2025
Merged

Change stream results to indicate cancellation#498
lukewagner merged 1 commit into
mainfrom
copy-result

Conversation

@lukewagner

Copy link
Copy Markdown
Member

As first discussed in #444 and resolving #490, this PR explicitly propagates whether an async stream/future read/write completed or was cancelled. The Buffer.MAX_LENGTH is lowered to 228-1 to match Table.MAX_LENGTH and leave a nice 4-bit space for any future additions.

(Originally, I was going to put the 2-bit "result" field in the MSBs such that Buffer.MAX_LENGTH could be increased if we wanted in the future, but I noticed this is the reverse of the canon lower result, which seemed gratuitously inconsistent. Also, I realized that once we change the CABI to take advantage of multi-return, this issue will go away, since we'll just return two i32s. I'm not wed to this detail, though.)

@lukewagner
lukewagner requested a review from alexcrichton April 10, 2025 22:04
Comment on lines +3617 to +3587
otherwise, completed successfully. In all cases, any number of elements (from
`0` to `n`) may have *first* been copied into or out of the buffer passed to
the `read` or `write` and so this number is packed into the `i32` result.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to provide a guarantee that if CANCELLED is returned then the number of elements transferred is always 0?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that it was indeed possible to receive CANCELLED and >0 elements. Hypothetically this could occur even with component-to-component streaming if a few partial writes succeed but then, when the reader tries to cancel, the writer is (in a hypothetical future with threads) on a different thread and the runtime is holding a lock while doing a large copy between buffers, and maybe we allow in this case the cancellation to return BLOCKED, and then it seems like we'd expect CANCELLED with >0 elements (as opposed to COMPLETED; the idea being that COMPLETED means "on its own", which is perhaps a distinction without a difference; I dunno?)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok makes sense to me, and I don't think this should be too too hard to bind in various languages. There's going to be some subtelty to interpreting the return values but I don't think that'll be too bad.

@alexcrichton

Copy link
Copy Markdown
Collaborator

Originally, I was going to put the 2-bit "result" field in the MSBs such

I was sort of having second thoughts about this too so I think lining things up seems reasonable. As you say getting multi-value means that we can lift all these restrictions anyway

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