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

bump wasmedge-sdk from 0.13.2 to 0.14.0 #777

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

CaptainVincent
Copy link
Contributor

The corresponding WasmEdge lib release is 0.14.1.

@jprendes
Copy link
Collaborator

@CaptainVincent , do you know why the returned exit_code is different from the expected value?

@CaptainVincent
Copy link
Contributor Author

Not sure yet, I'm still looking into this part.

@jprendes
Copy link
Collaborator

jprendes commented Jan 3, 2025

I've been looking into this.
Exit code 11 is usually related to a segmentation fault.

The code reaches the vm.run_func call, and aborts inside that function.
Furthermore, inside of run_func, the code reaches the executor.call_func call, and aborts inside that function.
Furthermore, inside of call_func, the code reaches the ffi::WasmEdge_ExecutorInvoke call, and aborts inside that function.

I'm not sure what could be leading to a segmentation fault inside the C++ codebase.

@CaptainVincent
Copy link
Contributor Author

I have tried to simplify the part where I integrate WasmEdge and run it in a pure environment. Based on cross-validation, it seems that some configurations are missing in the integration part. I will fix the exit code issue as soon as possible. The Windows part is more complicated, and I'm not sure if the issue is related to the FFI generated by bindgen, because based on my inspection, there doesn’t seem to be much difference in the headers for the release across different platforms.

@CaptainVincent CaptainVincent force-pushed the upgrade-wasmedge branch 3 times, most recently from 2b8f74b to b1df4e4 Compare January 13, 2025 19:14
@jprendes
Copy link
Collaborator

I think the windows issue is due to the wrong WasmEdge version installed in CI.
Unlike Linux, the standalone feature doesn't work on Windows, requiring us to install it manually in CI.

@jprendes
Copy link
Collaborator

@CaptainVincent , do you know what changed? Why is it working now?

@CaptainVincent CaptainVincent force-pushed the upgrade-wasmedge branch 2 times, most recently from ab0e0fd to 678a862 Compare January 14, 2025 02:05
@CaptainVincent
Copy link
Contributor Author

CaptainVincent commented Jan 14, 2025

To avoid crashes, I temporarily removed the following:

PluginManager::load(None)?;
PluginManager::auto_detect_plugins()?;

I will submit a separate PR to add support for the plugin feature. Currently, I am still debugging this functionality, which is needed to support plugins like wasi_nn and similar.

--

Additionally, I modified the setup-windows.sh script, but it still doesn’t seem to work for windows ci. Could it be that some caching mechanisms are causing the issue to persist? Because the Windows lint is working.

@CaptainVincent CaptainVincent marked this pull request as draft January 14, 2025 17:57
@CaptainVincent CaptainVincent marked this pull request as ready for review January 14, 2025 17:58
The corresponding WasmEdge lib release is 0.14.1

Signed-off-by: vincent <[email protected]>
@CaptainVincent
Copy link
Contributor Author

The previous force push failure related to Windows really seems to be about the cached issue. After I rebase it, it's all good now.

Copy link
Member

@Mossaka Mossaka left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@jprendes jprendes left a comment

Choose a reason for hiding this comment

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

LGTM

@jprendes jprendes merged commit 336aa1c into containerd:main Jan 14, 2025
72 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.

3 participants