Skip to content

Comments

Do not panic on host calls when CM concurrency support is disabled#12659

Merged
alexcrichton merged 2 commits intobytecodealliance:mainfrom
fitzgen:concurrency-disabled-host-call-panic
Feb 25, 2026
Merged

Do not panic on host calls when CM concurrency support is disabled#12659
alexcrichton merged 2 commits intobytecodealliance:mainfrom
fitzgen:concurrency-disabled-host-call-panic

Conversation

@fitzgen
Copy link
Member

@fitzgen fitzgen commented Feb 24, 2026

No description provided.

@fitzgen fitzgen requested a review from a team as a code owner February 24, 2026 22:13
@fitzgen fitzgen requested review from alexcrichton and removed request for a team February 24, 2026 22:13
Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Ah this was part of a recent refactor where these calls can't be skipped so they can't just return Ok(()) and work correctly. Can you add calls to self.enter_call_not_concurrent() and self.exit_call_not_concurrent() in the disabled case?

For a test for this, can you add a test with concurrency_supoprt disabled and a host call with a borrow<T> resource is done? I think it should panic and/or cause problems with the PR as-is, but if not I can help whip up a more involved test.

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

er, sorry, wrong button

@fitzgen fitzgen requested a review from alexcrichton February 24, 2026 23:15
@fitzgen fitzgen force-pushed the concurrency-disabled-host-call-panic branch from 57e29f0 to 1b13d60 Compare February 24, 2026 23:16
@alexcrichton alexcrichton added this pull request to the merge queue Feb 25, 2026
Merged via the queue into bytecodealliance:main with commit 2e3d0ec Feb 25, 2026
45 checks passed
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