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

feat: Add Host Functions support for .NET SDK #239

Merged
merged 19 commits into from Apr 11, 2023

Conversation

mhmd-azeez
Copy link
Collaborator

@mhmd-azeez mhmd-azeez commented Jan 23, 2023

  • Write p/invoke wrappers for new types/functions
    • ExtismValType
    • ExtismFunction
    • ExtismCurrentPlugin
    • ExtismValUnion
    • ExtismVal
    • ExtismFunctionType
    • extism_current_plugin_memory
    • extism_current_plugin_memory_alloc
    • extism_current_plugin_memory_length
    • extism_current_plugin_memory_free
    • extism_function_new
    • extism_function_free
  • Write higher level code that allows user to register a function
  • Expand the sample code and tests to use host functions

@bhelx
Copy link
Contributor

bhelx commented Mar 3, 2023

Just taking a moment to come back and look at this as we prepare for a new release. Seems like we have our type definitions but no code yet. @mhmd-azeez do you want me to take a crack at some of the code? Also @ubiquitous-dev have you made any progress on host functions in #264 or are you just waiting on this PR?

@mhmd-azeez
Copy link
Collaborator Author

Hi @bhelx, sorry it has been hard to find some time to work on this. Yes, please go ahead. I can give you commit access to the repo so that you can push to the branch directly

@bhelx
Copy link
Contributor

bhelx commented Mar 3, 2023

No worries at all! hard for me to find the time as well. I will take a look next week. Any changes I make i will PR into your branch.

@mhmd-azeez
Copy link
Collaborator Author

Okay, you already have commit access. If you have any questions please let me know

@ubiquitous-dev
Copy link

ubiquitous-dev commented Mar 4, 2023

Hi @bhelx - the PR #264 is a superset of this PR - it has all the code from here, plus a working asynchronous / cancellable plugin execution engine (depends on the plugin-cancel implementation we've been testing with @zshipko), as well as additional test coverage. I have already added some host function tests based on the Java library's host functions tests, and would love some help on that branch for extending the Host Functions support further. I'll give you commit access to my repo as well.

@mhmd-azeez
Copy link
Collaborator Author

@bhelx Host Functions now work in the .NET SDK :D But I'll need to clean it up a little bit and do more tests. Also, the extism panics somewhere, but I have no idea what's going on:

Hello again!

thread '<unnamed>' panicked at 'called `Option::unwrap()` on a `None` value', /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74\library\std\src\thread\mod.rs:1458:40
stack backtrace:
   0:     0x7ffbc3cdcd42 - set_vmctx_memory
   1:     0x7ffbc3cf8dfb - set_vmctx_memory
   2:     0x7ffbc3cd55ea - set_vmctx_memory
   3:     0x7ffbc3cdca8b - set_vmctx_memory
   4:     0x7ffbc3cdf3b9 - set_vmctx_memory
   5:     0x7ffbc3cdf03b - set_vmctx_memory
   6:     0x7ffbc3cdfc50 - set_vmctx_memory
   7:     0x7ffbc3cdf99b - set_vmctx_memory
   8:     0x7ffbc3cdd75f - set_vmctx_memory
   9:     0x7ffbc3cdf690 - set_vmctx_memory
  10:     0x7ffbc3da8be5 - _jit_debug_register_code
  11:     0x7ffbc3da8c9c - _jit_debug_register_code
  12:     0x7ffbc35e5270 - extism_version
  13:     0x7ffbc35d085d - extism_version
  14:     0x7ffbc35c87a0 - extism_version
  15:     0x7ffbc35d0516 - extism_version
  16:     0x7ffc91623a83 - _sys_nerr
  17:     0x7ffc915f006e - o_free
  18:     0x7ffc915eda1d - execute_onexit_table
  19:     0x7ffbc3d6fcf9 - _jit_debug_register_code
  20:     0x7ffbc3d6fe1e - _jit_debug_register_code
  21:     0x7ffc939f868f - RtlActivateActivationContextUnsafeFast
  22:     0x7ffc93a21096 - LdrShutdownProcess
  23:     0x7ffc93a20c8d - RtlExitUserProcess
  24:     0x7ffc923182bb - ExitProcess
  25:     0x7ffc915ebac0 - exit
  26:     0x7ffc915ebcd9 - exit
  27:     0x7ff742fa421f - <unknown>
  28:     0x7ffc923126bd - BaseThreadInitThunk
  29:     0x7ffc93a2a9f8 - RtlUserThreadStart

extism\dotnet\samples\Extism.Sdk.Sample\bin\Debug\net7.0\Extism.Sdk.Sample.exe (process 8780) exited with code -1073740791.

@mhmd-azeez
Copy link
Collaborator Author

@bhelx okay, by looking at the go implementation, I was able to fix some of the marshalling code and now both input and output work. But we still get the panic.

Hello from .NET!
Hello again!
Input: {"count": 3}
Output: {"count": 3}
thread '<unnamed>' panicked at 'called `Option::unwrap()` on a `None` value', /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74\library\std\src\thread\mod.rs:1458:40
stack backtrace:
   0:     0x7ffb98abcd42 - set_vmctx_memory
   1:     0x7ffb98ad8dfb - set_vmctx_memory
   2:     0x7ffb98ab55ea - set_vmctx_memory
   3:     0x7ffb98abca8b - set_vmctx_memory
   4:     0x7ffb98abf3b9 - set_vmctx_memory
   5:     0x7ffb98abf03b - set_vmctx_memory
   6:     0x7ffb98abfc50 - set_vmctx_memory
   7:     0x7ffb98abf99b - set_vmctx_memory
   8:     0x7ffb98abd75f - set_vmctx_memory
   9:     0x7ffb98abf690 - set_vmctx_memory
  10:     0x7ffb98b88be5 - _jit_debug_register_code
  11:     0x7ffb98b88c9c - _jit_debug_register_code
  12:     0x7ffb983c5270 - extism_version
  13:     0x7ffb983b085d - extism_version
  14:     0x7ffb983a87a0 - extism_version
  15:     0x7ffb983b0516 - extism_version
  16:     0x7ffc91623a83 - _sys_nerr
  17:     0x7ffc915f006e - o_free
  18:     0x7ffc915eda1d - execute_onexit_table
  19:     0x7ffb98b4fcf9 - _jit_debug_register_code
  20:     0x7ffb98b4fe1e - _jit_debug_register_code
  21:     0x7ffc939f868f - RtlActivateActivationContextUnsafeFast
  22:     0x7ffc93a21096 - LdrShutdownProcess
  23:     0x7ffc93a20c8d - RtlExitUserProcess
  24:     0x7ffc923182bb - ExitProcess
  25:     0x7ffc915ebac0 - exit
  26:     0x7ffc915ebcd9 - exit
  27:     0x7ff64d55421f - <unknown>
  28:     0x7ffc923126bd - BaseThreadInitThunk
  29:     0x7ffc93a2a9f8 - RtlUserThreadStart

D:\oss\extism\dotnet\samples\Extism.Sdk.Sample\bin\Debug\net7.0\Extism.Sdk.Sample.exe (process 11600) exited with code -1073740791.

@lukevp
Copy link

lukevp commented Mar 25, 2023

I haven’t tested this out yet, but just reading the stack trace, looks like the extism_version function is the issue (maybe it’s involved in logging or something?)

I’m not a rust expert, but this code seems to indicate that an env var must be set for extism_version to resolve to a string. Can you try setting that env var? I’m also curious if you call extism_version() directly from your .net project, does that also have an issue?

@zshipko
Copy link
Contributor

zshipko commented Mar 28, 2023

Thanks @mhmd-azeez! I am not getting that panic when I run the example program on Ubuntu. I am in the process of getting a Windows dev environment set up, hopefully I can try it out on Windows at some point.

Does the sample program use the system extism installation or the one in the repo? Like @lukevp mentioned, the backtrace you posted looks kind of strange because it happens on extism_version, not anything host function related.

@mhmd-azeez
Copy link
Collaborator Author

mhmd-azeez commented Mar 28, 2023

Hello @zshipko the sample uses the extism binaries published in the nuget package published here: https://www.nuget.org/packages/Extism.runtime.win-x64

@mhmd-azeez
Copy link
Collaborator Author

@zshipko it seems like this bug was introduced in v0.2.0 of the runtime and it only happens on windows. I have created an issue for it: #299

@mhmd-azeez
Copy link
Collaborator Author

Hello @nilslice @bhelx I am done with this PR. Please review it and if you think it's good, you can merge it.

@mhmd-azeez mhmd-azeez marked this pull request as ready for review April 7, 2023 21:18
@nilslice
Copy link
Member

nilslice commented Apr 8, 2023

Awesome. We will review ASAP. Thank you!

Copy link
Contributor

@bhelx bhelx left a comment

Choose a reason for hiding this comment

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

Alright, works on my machine! Code looks good. Great work! Really excited about this one. I'm gonna aim to release this early next week, might be earlier though.

@bhelx bhelx merged commit 670f364 into extism:main Apr 11, 2023
3 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.

None yet

6 participants