Repository navigation
component model async: increment error-context global ref count on lower (fixes #14674) - #15092
ToscaLorno wants to merge 1 commit into
Conversation
When an error-context handle is transferred across component instances via a future or stream, lowering it into the recipient instance inserts a local handle pointing to the existing global error context. However, lower_error_context_to_index previously omitted incrementing the cross-component global_ref_count. When the receiving instance subsequently called error-context.drop, the global ref count was prematurely decremented to zero, deallocating the underlying state while the sending instance still held its own handle. This patch updates lower_error_context_to_index to increment the global ref count on insertion, matching the semantics of error_context_transfer, and adds a regression test in ests/misc_testsuite/component-model/async/error-context-future.wast. Fixes bytecodealliance#14674
github-actions
Bot
added
the
wasmtime:api
|
Thanks for the PR, but I'm personally hesitant to try to fixup error-context piecemeal like this. I know LLMs are good at generating code, but the prompt here of "fix the bug" I feel is not complete enough. The error-context type is pretty under-baked right now and likely needs some deeper redesign. The logged issues we have are an attempt to catalogue known issues with it but I wouldn't recommend new contributors try to take them on |
|
Thanks for the context @alexcrichton! Completely makes sense that |
|
Closing this PR per the discussion above to leave space for the broader architectural redesign of error-context. Thanks again @alexcrichton! |
Summary
Fixes #14674.
When an
error-contexthandle is transferred across component instances via an async future or stream, lowering it into the recipient instance inserts a local handle entry pointing to the underlying global error context table. However,lower_error_context_to_indexpreviously omitted incrementing the cross-subcomponentglobal_ref_count.As a consequence, when the recipient instance subsequently invoked
error-context.drop, the global reference count was decremented to zero prematurely, deleting the global error context tracking while the sending instance still retained its handle.This patch updates
lower_error_context_to_indexto increment the global reference count on insertion, mirroring the semantics oferror_context_transfer.Testing
Added regression test
tests/misc_testsuite/component-model/async/error-context-future.wast, verifying that dropping the received error-context in a recipient instance does not invalidate or break subsequent drops in the sender.Ran test suite:
cargo test --test wast error-context-future: PASS (CraneliftNative + Winch)cargo test --test wast async: 266 passed; 0 failed; 0 ignored