feat(go): preventing concurrent reads/writes on streams and futures#1490
feat(go): preventing concurrent reads/writes on streams and futures#1490dicej merged 1 commit intobytecodealliance:mainfrom
Conversation
|
|
@dicej Looks like the CI/Check GH Actions workflow failed. I assume the fix for that is just re-running it? If so, would you mind doing that for me? |
👍 BTW, would you be able to add test coverage for this to the tests/runtime-async suite, perhaps by expanding |
|
@dicej When you have a moment, this is ready to be reviewed |
dicej
left a comment
There was a problem hiding this comment.
Looks good, thanks! Just a few doc tweak suggestions.
Signed-off-by: Andrew Steurer <[email protected]> fix: adding panic for empty dst slice, removing handle reassignment for futures Signed-off-by: Andrew Steurer <[email protected]> docs(go): adding package comments Signed-off-by: Andrew Steurer <[email protected]> feat(go): add future read concurrency test Signed-off-by: Andrew Steurer <[email protected]> feat(go): adding remaining concurrent read/write tests Signed-off-by: Andrew Steurer <[email protected]> doc(go): adding comments on drop methods Signed-off-by: Andrew Steurer <[email protected]> Update crates/go/src/package/wit_types/wit_stream.go Co-authored-by: Joel Dice <[email protected]> Update crates/go/src/package/wit_types/wit_future.go Co-authored-by: Joel Dice <[email protected]> Update crates/go/src/package/wit_types/wit_future.go Co-authored-by: Joel Dice <[email protected]>
Overview
This change is intended to prevent concurrent reads and writes on streams and futures
Closes #1460
Observations
Given the following guest application code:
I would expect there to be a panic and have there be a
"nil handle"message; however, this is the output I get:This is concerning because the Goroutine appears to be moving past the panic in the
wit_runtime.Handle.Take()method:@dicej any thoughts and/or suggestions?