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: Implement a basic Extism Go SDK #1

Merged
merged 41 commits into from
Aug 3, 2023
Merged

feat: Implement a basic Extism Go SDK #1

merged 41 commits into from
Aug 3, 2023

Conversation

mhmd-azeez
Copy link
Collaborator

@mhmd-azeez mhmd-azeez commented Jul 18, 2023

Implements https://github.com/dylibso/JIRA/issues/35

  • ideally see that the extism kernel is working when embedded into the runtime (see that all CI tests pass)
  • in a new repository (extism/go-sdk), implement the full Go SDK API (where it makes sense, @zshipko can clarify here a bit) & make improvements to the APIs as you go.
    • we try to break API compatibility as infrequently as possible, so this is an opportunity to really improve things with breaking changes already expected.
  • the new Go SDK will use Wazero runtime, which is pure Go and needs no shared library (bye-bye libextism.so)
  • an incomplete list of runtime functionality should include:
    • Extism Manifest support
      • Byte Array, File, Url wasm sources
      • Cache http wasm plugins As far as I can see, this is not being used in the Rust runtime either.
      • Allowed Hosts
      • Config
      • Allowed Paths
    • Logging
    • HTTP (extism_http_request), configured to disallow every request by default, and allow hostnames as defined in the Manifest
      • extism_http_request
      • extism_http_status_code
      • disallow every request by default
    • WASI option
      • allow enabling
      • configure allowed file path
    • timeouts / cancellation (Go's context package should make this more straightforward)
      • test
    • Config
    • Persistent Variables
    • Host Functions
      • Allow registering host functions in the env module
      • Think about host functions in modules we don't own (from wasm binaries). Maybe we can wrap the guest module in a host module just like the env module. I think we should not support this until a user explicitly asks for it, so that we understand the use case better
      • Allow registering host functions in other modules
    • Handle runtime initialization (i.e. call _start function)
    • Write a test suite
    • Come up with a reliable way to update extism-runtime.wasm
    • Write docs for all public functions/properties
    • Create a CI workflow
    • Write a proper README for the repo, and a README per plugin example

Questions

@mhmd-azeez mhmd-azeez requested review from bhelx, zshipko and nilslice July 18, 2023 13:40
@mhmd-azeez mhmd-azeez self-assigned this Jul 18, 2023
@mhmd-azeez
Copy link
Collaborator Author

I added support for loading a plugin from a manifest, but not sure if the approach is correct or if it's the best way to do it, please let me know if you have any feedback.

@mhmd-azeez
Copy link
Collaborator Author

mhmd-azeez commented Jul 18, 2023

Hey @zshipko, I have been trying to implement config_get, but have ran into a couple of roadblocks:

	_, err = rt.NewHostModuleBuilder("env").
		NewFunctionBuilder().
		WithGoModuleFunction(api.GoModuleFunc(func(ctx context.Context, m api.Module, stack []uint64) {
			extism := ext // TODO: is there a better way?

			mem := extism.Memory()
			offset := api.DecodeU32(stack[0])
			nameLength := uint32(0)                   // TODO: how do we get the length of the block?
			buffer, _ := mem.Read(offset, nameLength) // TODO: handle fail

			name := string(buffer)
			value := "value" + name // TODO: how do we get a handle to the plugin

			buffer = []byte(value)
			res, _ := extism.ExportedFunction("alloc").Call(ctx, uint64(len(buffer))) // TODO: handle fail

			out := res[0]
			mem.Write(uint32(out), buffer)

			stack[0] = out
		}), []api.ValueType{api.ValueTypeI32}, []api.ValueType{api.ValueTypeI32}).
		Export("config_get").
		Instantiate(ctx)

Namely:

  1. We need to implement length in the wasm runtime, as far as I know, there is no way to know the length of a block currently
  2. We need to get a handle to the plugin to which the Module belongs, so that we can access the Config map from it

@zshipko
Copy link
Contributor

zshipko commented Jul 18, 2023

We need to implement length in the wasm runtime, as far as I know, there is no way to know the length of a block currently

There is already extism_length(i64) -> i64 that can be used for this

We need to get a handle to the plugin to which the Module belongs, so that we can access the Config map from it

I will have to take a closer look at the code to be sure, but it might be possible to use context.WithValue for this. You could set the "CurrentPlugin" key on the context to a Plugin* value in Plugin.Call before the WASM function call.

extism.go Outdated Show resolved Hide resolved
@mhmd-azeez
Copy link
Collaborator Author

We need to implement length in the wasm runtime, as far as I know, there is no way to know the length of a block currently

There is already extism_length(i64) -> i64 that can be used for this

Ah, I don't know how I couldn't see it. Thanks!

We need to get a handle to the plugin to which the Module belongs, so that we can access the Config map from it

I will have to take a closer look at the code to be sure, but it might be possible to use context.WithValue for this. You could set the "CurrentPlugin" key on the context to a Plugin* value in Plugin.Call before the WASM function call.

Yup, that works

@mhmd-azeez
Copy link
Collaborator Author

mhmd-azeez commented Jul 19, 2023

@zshipko I have faced a new roadblock: wazero can't initialize two modules with the same name. And we are implmenting some PDK functions in the kernel module and some of them in a hosted module (for example: extism_config_get). They both need to be in the env namespace

I have tried these work-arounds:

  1. Try to see if there is any way to merge two api.Modules or two CompiledModules: couldn't find any way to do that.
  2. Try to find a way to edit the imports of a module so that I can change the namespace of some of the imported functions: the imports are private fields.
  3. Try to wrap the kernel module in a hosted function module. Unforutunately, calling a host function from the host doesn't seem to be supported. I even tried their own example:
env, err := r.NewHostModuleBuilder("env").
	NewFunctionBuilder().
	WithFunc(func(v uint32) {
		fmt.Println("log_i32 >>", v)
	}).
	Export("log_i32").
	NewFunctionBuilder().
	WithFunc(func() uint32 {
		if envYear, err := strconv.ParseUint(os.Getenv("CURRENT_YEAR"), 10, 64); err == nil {
			return uint32(envYear) // Allow env-override to prevent annual test maintenance!
		}
		return uint32(time.Now().Year())
	}).
	Export("current_year").
	Instantiate(ctx)
if err != nil {
	log.Panicln(err)
}

// try to call `current_year`
rr, err := env.ExportedFunction("current_year").Call(ctx)
if err != nil {
	log.Panicln(err)
}

output:

Exception 0xc0000005 0x0 0x8 0x1a1699f007e
PC=0x1a1699f007e

runtime: g 6: unknown pc 0x1a1699f007e
Exception 0xc0000005 0x0 0x8 0x1a1699f007e
PC=0x1a1699f007e

runtime: g 6: unknown pc 0x1a1699f007e
stack: frame={sp:0xc00007dab0, fp:0x0} stack=[0xc00007a000,0xc00007e000)
0x000000c00007d9b0:  0x000000c000062af0  0x000000000068a113 <runtime.systemstack+0x0000000000000033> 
0x000000c00007d9c0:  0x000000000062e746 <runtime.persistentalloc+0x0000000000000066>  0x000000c00007da20 
0x000000c00007d9d0:  0x0000000000635a30 <runtime.(*mspan).initHeapBits+0x00000000000000b0>  0x000000000068a113 <runtime.systemstack+0x0000000000000033> 
0x000000c00007d9e0:  0x0000000000647905 <runtime.(*mheap).alloc+0x0000000000000065>  0x000000c00007d9f8 
0x000000c00007d9f0:  0x000001a163d6a9d8  0x0000000000647960 <runtime.(*mheap).alloc.func1+0x0000000000000000> 
0x000000c00007da00:  0x0000000000a1e6a0  0x0000000000000001
0x000000c00007da10:  0x000000c00007d9f0  0x0000000000000022
0x000000c00007da20:  0x000000c00007da60  0x000000000063786d <runtime.(*mcentral).grow+0x000000000000008d>
0x000000c00007da30:  0x0000000000650f74 <runtime.(*spanSet).push+0x0000000000000154>  0x000000c000162100
0x000000c00007da40:  0x0000000000868ff8  0x000001a163d6a9d8
0x000000c00007da50:  0x00000000000000f0  0x0000000000000001
0x000000c00007da60:  0x000000c00007dab8  0x0000000000637453 <runtime.(*mcentral).cacheSpan+0x00000000000000d3>
0x000000c00007da70:  0x000001a169859a40  0x00000000007a9be0 <github.com/tetratelabs/wazero/internal/wasm.(*Store).GetFunctionTypeID.func1+0x0000000000000000>
0x000000c00007da80:  0x000000000085a240  0x000000c00007db40
0x000000c00007da90:  0x0000000000636170 <runtime.heapBitsSetType+0x0000000000000290>  0x000000000063776d <runtime.(*mcentral).uncacheSpan+0x000000000000008d>
0x000000c00007daa0:  0x0000000000a32dc8  0x000001a163d6a9d8
0x000000c00007dab0: <0x00000000007d08b1 <github.com/tetratelabs/wazero/internal/engine/compiler.(*callEngine).execWasmFunction+0x0000000000000071>  0x000001a1699f0072
0x000000c00007dac0:  0x000000c00017a000  0x000000c000162100
0x000000c00007dad0:  0x00000000006419a7 <runtime.(*gcControllerState).heapGoalInternal+0x0000000000000027>  0x000000c000162100
0x000000c00007dae0:  0x0000000000400000  0x000000c00007db28
0x000000c00007daf0:  0x0000000000641ae5 <runtime.(*gcControllerState).trigger+0x0000000000000025>  0x0000000000a5cec0
0x000000c00007db00:  0x000000000062d83e <runtime.(*mcache).nextFree+0x000000000000009e>  0x000001a163d6a9d8
0x000000c00007db10:  0x000000c00007db59  0x000000000062eee5 <runtime.makemap_small+0x0000000000000025>
0x000000c00007db20:  0x0000000000000000  0x000000c00007db40
0x000000c00007db30:  0x000000c000162100  0x000000c00007dc00
0x000000c00007db40:  0x00000000007cfb65 <github.com/tetratelabs/wazero/internal/engine/compiler.(*callEngine).call+0x00000000000002e5>  0x000000c00017a000
0x000000c00007db50:  0x00000000008b6b48  0x000000c000016100
0x000000c00007db60:  0x00000000000000f0  0x0101000000857060
0x000000c00007db70:  0x000001c00007db88  0x000001a163d6a9d8
0x000000c00007db80:  0x000000c000162100  0x000000c00007e550
0x000000c00007db90:  0x0000000000a07d20  0x0000000000000000
0x000000c00007dba0:  0x0000000000000000  0x0000000000000000
runtime: g 6: unknown pc 0x1a1699f007e
stack: frame={sp:0xc00007dab0, fp:0x0} stack=[0xc00007a000,0xc00007e000)
0x000000c00007d9b0:  0x000000c000062af0  0x000000000068a113 <runtime.systemstack+0x0000000000000033>
0x000000c00007d9c0:  0x000000000062e746 <runtime.persistentalloc+0x0000000000000066>  0x000000c00007da20
0x000000c00007d9d0:  0x0000000000635a30 <runtime.(*mspan).initHeapBits+0x00000000000000b0>  0x000000000068a113 <runtime.systemstack+0x0000000000000033>
0x000000c00007d9e0:  0x0000000000647905 <runtime.(*mheap).alloc+0x0000000000000065>  0x000000c00007d9f8
0x000000c00007d9f0:  0x000001a163d6a9d8  0x0000000000647960 <runtime.(*mheap).alloc.func1+0x0000000000000000>
0x000000c00007da00:  0x0000000000a1e6a0  0x0000000000000001
0x000000c00007da10:  0x000000c00007d9f0  0x0000000000000022
0x000000c00007da20:  0x000000c00007da60  0x000000000063786d <runtime.(*mcentral).grow+0x000000000000008d>
0x000000c00007da30:  0x0000000000650f74 <runtime.(*spanSet).push+0x0000000000000154>  0x000000c000162100
0x000000c00007da40:  0x0000000000868ff8  0x000001a163d6a9d8
0x000000c00007da50:  0x00000000000000f0  0x0000000000000001
0x000000c00007da60:  0x000000c00007dab8  0x0000000000637453 <runtime.(*mcentral).cacheSpan+0x00000000000000d3>
0x000000c00007da70:  0x000001a169859a40  0x00000000007a9be0 <github.com/tetratelabs/wazero/internal/wasm.(*Store).GetFunctionTypeID.func1+0x0000000000000000>
0x000000c00007da80:  0x000000000085a240  0x000000c00007db40
0x000000c00007da90:  0x0000000000636170 <runtime.heapBitsSetType+0x0000000000000290>  0x000000000063776d <runtime.(*mcentral).uncacheSpan+0x000000000000008d>
0x000000c00007daa0:  0x0000000000a32dc8  0x000001a163d6a9d8
0x000000c00007dab0: <0x00000000007d08b1 <github.com/tetratelabs/wazero/internal/engine/compiler.(*callEngine).execWasmFunction+0x0000000000000071>  0x000001a1699f0072
0x000000c00007dac0:  0x000000c00017a000  0x000000c000162100
0x000000c00007dad0:  0x00000000006419a7 <runtime.(*gcControllerState).heapGoalInternal+0x0000000000000027>  0x000000c000162100
0x000000c00007dae0:  0x0000000000400000  0x000000c00007db28
0x000000c00007daf0:  0x0000000000641ae5 <runtime.(*gcControllerState).trigger+0x0000000000000025>  0x0000000000a5cec0
0x000000c00007db00:  0x000000000062d83e <runtime.(*mcache).nextFree+0x000000000000009e>  0x000001a163d6a9d8
0x000000c00007db10:  0x000000c00007db59  0x000000000062eee5 <runtime.makemap_small+0x0000000000000025>
0x000000c00007db20:  0x0000000000000000  0x000000c00007db40
0x000000c00007db30:  0x000000c000162100  0x000000c00007dc00
0x000000c00007db40:  0x00000000007cfb65 <github.com/tetratelabs/wazero/internal/engine/compiler.(*callEngine).call+0x00000000000002e5>  0x000000c00017a000
0x000000c00007db50:  0x00000000008b6b48  0x000000c000016100
0x000000c00007db60:  0x00000000000000f0  0x0101000000857060
0x000000c00007db70:  0x000001c00007db88  0x000001a163d6a9d8
0x000000c00007db80:  0x000000c000162100  0x000000c00007e550
0x000000c00007db90:  0x0000000000a07d20  0x0000000000000000
0x000000c00007dba0:  0x0000000000000000  0x0000000000000000
created by testing.(*T).Run
        C:/Program Files/Go/src/testing/testing.go:1629 +0x3ea

goroutine 1 [chan receive]:
runtime.gopark(0xa07d20?, 0xc00000a070?, 0x90?, 0x6a?, 0xc00011ba28?)
        C:/Program Files/Go/src/runtime/proc.go:381 +0xd6 fp=0xc00011b9a8 sp=0xc00011b988 pc=0x65c436
runtime.chanrecv(0xc0000660e0, 0xc00011baa7, 0x1)
        C:/Program Files/Go/src/runtime/chan.go:583 +0x49d fp=0xc00011ba38 sp=0xc00011b9a8 pc=0x626d5d
runtime.chanrecv1(0xa07220?, 0x821b60?)
        C:/Program Files/Go/src/runtime/chan.go:442 +0x18 fp=0xc00011ba60 sp=0xc00011ba38 pc=0x626898
testing.(*T).Run(0xc000057520, {0x86a191?, 0x68ef13?}, 0x87dbd0)
        C:/Program Files/Go/src/testing/testing.go:1630 +0x405 fp=0xc00011bb20 sp=0xc00011ba60 pc=0x709045
testing.runTests.func1(0xa07d20?)
        C:/Program Files/Go/src/testing/testing.go:2036 +0x45 fp=0xc00011bb70 sp=0xc00011bb20 pc=0x70b205
testing.tRunner(0xc000057520, 0xc00011bc88)
        C:/Program Files/Go/src/testing/testing.go:1576 +0x10b fp=0xc00011bbc0 sp=0xc00011bb70 pc=0x70818b
testing.runTests(0xc00007e460?, {0x9f3200, 0x1, 0x1}, {0x20?, 0x100c000128588?, 0xa07540?})
        C:/Program Files/Go/src/testing/testing.go:2034 +0x489 fp=0xc00011bcb8 sp=0xc00011bbc0 pc=0x70b0e9
testing.(*M).Run(0xc00007e460)
        C:/Program Files/Go/src/testing/testing.go:1906 +0x63a fp=0xc00011bf00 sp=0xc00011bcb8 pc=0x709a5a
main.main()
        _testmain.go:47 +0x1aa fp=0xc00011bf80 sp=0xc00011bf00 pc=0x804fca
runtime.main()
        C:/Program Files/Go/src/runtime/proc.go:250 +0x1f7 fp=0xc00011bfe0 sp=0xc00011bf80 pc=0x65c017
runtime.goexit()
        C:/Program Files/Go/src/runtime/asm_amd64.s:1598 +0x1 fp=0xc00011bfe8 sp=0xc00011bfe0 pc=0x68c3a1

goroutine 2 [force gc (idle)]:
runtime.gopark(0x0?, 0x0?, 0x0?, 0x0?, 0x0?)
        C:/Program Files/Go/src/runtime/proc.go:381 +0xd6 fp=0xc000059fb0 sp=0xc000059f90 pc=0x65c436
runtime.goparkunlock(...)
        C:/Program Files/Go/src/runtime/proc.go:387
runtime.forcegchelper()
        C:/Program Files/Go/src/runtime/proc.go:305 +0xb2 fp=0xc000059fe0 sp=0xc000059fb0 pc=0x65c252
runtime.goexit()
        C:/Program Files/Go/src/runtime/asm_amd64.s:1598 +0x1 fp=0xc000059fe8 sp=0xc000059fe0 pc=0x68c3a1
created by runtime.init.6
        C:/Program Files/Go/src/runtime/proc.go:293 +0x25

goroutine 3 [GC sweep wait]:
runtime.gopark(0x0?, 0x0?, 0x0?, 0x0?, 0x0?)
        C:/Program Files/Go/src/runtime/proc.go:381 +0xd6 fp=0xc00005bf80 sp=0xc00005bf60 pc=0x65c436
runtime.goparkunlock(...)
        C:/Program Files/Go/src/runtime/proc.go:387
runtime.bgsweep(0x0?)
        C:/Program Files/Go/src/runtime/mgcsweep.go:278 +0x8e fp=0xc00005bfc8 sp=0xc00005bf80 pc=0x64472e
runtime.gcenable.func1()
        C:/Program Files/Go/src/runtime/mgc.go:178 +0x26 fp=0xc00005bfe0 sp=0xc00005bfc8 pc=0x639946
runtime.goexit()
        C:/Program Files/Go/src/runtime/asm_amd64.s:1598 +0x1 fp=0xc00005bfe8 sp=0xc00005bfe0 pc=0x68c3a1
created by runtime.gcenable
        C:/Program Files/Go/src/runtime/mgc.go:178 +0x6b

goroutine 4 [GC scavenge wait]:
runtime.gopark(0xc000066000?, 0x8b3538?, 0x1?, 0x0?, 0x0?)
        C:/Program Files/Go/src/runtime/proc.go:381 +0xd6 fp=0xc00006df70 sp=0xc00006df50 pc=0x65c436
runtime.goparkunlock(...)
        C:/Program Files/Go/src/runtime/proc.go:387
runtime.(*scavengerState).park(0xa075c0)
        C:/Program Files/Go/src/runtime/mgcscavenge.go:400 +0x53 fp=0xc00006dfa0 sp=0xc00006df70 pc=0x642633
runtime.bgscavenge(0x0?)
        C:/Program Files/Go/src/runtime/mgcscavenge.go:628 +0x45 fp=0xc00006dfc8 sp=0xc00006dfa0 pc=0x642c25
runtime.gcenable.func2()
        C:/Program Files/Go/src/runtime/mgc.go:179 +0x26 fp=0xc00006dfe0 sp=0xc00006dfc8 pc=0x6398e6
runtime.goexit()
        C:/Program Files/Go/src/runtime/asm_amd64.s:1598 +0x1 fp=0xc00006dfe8 sp=0xc00006dfe0 pc=0x68c3a1
created by runtime.gcenable
        C:/Program Files/Go/src/runtime/mgc.go:179 +0xaa

goroutine 5 [finalizer wait]:
runtime.gopark(0x65c7b2?, 0x65c185?, 0x0?, 0x0?, 0xc00005df70?)
        C:/Program Files/Go/src/runtime/proc.go:381 +0xd6 fp=0xc00005de28 sp=0xc00005de08 pc=0x65c436
runtime.runfinq()
        C:/Program Files/Go/src/runtime/mfinal.go:193 +0x107 fp=0xc00005dfe0 sp=0xc00005de28 pc=0x6389a7
runtime.goexit()
        C:/Program Files/Go/src/runtime/asm_amd64.s:1598 +0x1 fp=0xc00005dfe8 sp=0xc00005dfe0 pc=0x68c3a1
created by runtime.createfing
        C:/Program Files/Go/src/runtime/mfinal.go:163 +0x45
rax     0x0
rbx     0x8b6b48
rcx     0xc000016100
rdi     0xc000162100
rsi     0x1a1699f0072
rbp     0xc00007db38
rsp     0xc00007dab0
r8      0xc00007e550
r9      0x0
r10     0x0
r11     0x0
r12     0xc000162100
r13     0xc00017a000
r14     0xc000178000
r15     0x3
rip     0x1a1699f007e
rflags  0x10206
cs      0x33
fs      0x53
gs      0x2b
FAIL    github.com/tetratelabs/wazero/examples/import-go        0.651s
FAIL

Should we file an issue on wazero for that?

One other way would be to change the namespace of the non-kernel functions. But this would be a breaking change for all of the PDKs

@zshipko
Copy link
Contributor

zshipko commented Jul 19, 2023

Yeah the lack of a linker similar to wasmtime makes this confusing.

I think

Try to wrap the kernel module in a hosted function module

makes the most sense in terms of how to proceed. I will experiment with that a little today and make a PR against this branch if I come up with something.

If being able to add imports to a module is just a matter of adding a new function to wazero that could be a viable option as well.

@mhmd-azeez
Copy link
Collaborator Author

If being able to add imports to a module is just a matter of adding a new function to wazero that could be a viable option as well.

That'd be great if possible

Comment on lines +9 to +15
//export __wasm_call_ctors
func __wasm_call_ctors()

//export _initialize
func _initialize() {
__wasm_call_ctors()
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@zshipko we can export our own _initialize in TinyGo

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome!

@nilslice nilslice mentioned this pull request Jul 30, 2023
@mhmd-azeez mhmd-azeez marked this pull request as ready for review July 30, 2023 19:08
@mhmd-azeez
Copy link
Collaborator Author

There are still some things to do, but I am marking the PR ready for review

@mhmd-azeez
Copy link
Collaborator Author

I have changed the host function callback signature to include userData and use the same stack used by wazero. But tbh, I don't think we don't need userData anymore here as there are more idiomatic ways to pass in some state

@mhmd-azeez
Copy link
Collaborator Author

mhmd-azeez commented Aug 1, 2023

For the CI, do we need to test on multiple OSes? since the whole thing is written in pure Go

@nilslice
Copy link
Member

nilslice commented Aug 1, 2023

I have changed the host function callback signature to include userData and use the same stack used by wazero. But tbh, I don't think we don't need userData anymore here as there are more idiomatic ways to pass in some state

@mhmd-azeez - is this statement true for all SDK languages, or just Go? We just need to be mindful of the compatibility across hosts/guests -- I'm certain you are already thinking of this :) just clarifying!

@nilslice
Copy link
Member

nilslice commented Aug 1, 2023

For the CI, do we need to test on multiple OSes? since the whole thing is written in pure Go

I think starting with ubuntu-latest is probably enough, but I don't think it hurts to use windows-latest, macos-latest as well in a matrix. We can follow up with a PR with these if you want.

@nilslice
Copy link
Member

nilslice commented Aug 1, 2023

To test this SDK, I think it would be nice to update this repo to use it: https://github.com/extism/extism-fsnotify

@mhmd-azeez
Copy link
Collaborator Author

@mhmd-azeez - is this statement true for all SDK languages, or just Go? We just need to be mindful of the compatibility across hosts/guests -- I'm certain you are already thinking of this :) just clarifying!

If you mean compatibility between Host APIs, I was under the impression that the host SDK APIs didn't have to match each other, as long as they're all capable of doing the same set of operations. So this statement would be true for any language that supports closures.

And if you mean compatibility between the GO SDK and the guest PDKs, the userData parameter is only being used by the host, and the guest has no knowledge of it, so removing it shouldn't affect compatibility issues for the guests

But in the end, I don't have a strong opinion about userData and I am happy to let it stay

@zshipko
Copy link
Contributor

zshipko commented Aug 3, 2023

I think the userData argument is mostly because of the C API, it doesn't make as much sense in this case.

@zshipko
Copy link
Contributor

zshipko commented Aug 3, 2023

Also, this PR is looking great! Unless @nilslice disagrees or there are some major API design changes you're working on I think you're good to merge this when you feel ready.

Let me know if there's anything you need specific feedback on before merging!

@nilslice
Copy link
Member

nilslice commented Aug 3, 2023

I'm on the same page as @zshipko - it's got enough eyes on it to merge and we can follow up with some more narrowly scoped PRs

Great work!!

@bhelx
Copy link
Contributor

bhelx commented Aug 3, 2023 via email

@mhmd-azeez
Copy link
Collaborator Author

Great, in this case I will merge this PR and continue working on follow up PRs

@mhmd-azeez mhmd-azeez merged commit 9151eaf into main Aug 3, 2023
@mhmd-azeez mhmd-azeez deleted the feat/go-sdk branch August 3, 2023 11:51
mhmd-azeez added a commit to extism/extism that referenced this pull request Jul 25, 2024
This is a rough POC for allowing people to whitelist a dir as readonly.
When the source path is prefixed with `ro:`, the dir is considered as
readonly. This preserved backward compatibility. This suggestion came up
in extism/go-sdk#1 (comment)

Readonly:
```rs
let manifest = Manifest::new([url])
        .with_allowed_path("ro:D:/x/rust/fs/data".to_string(), "/data")
        .with_config_key("path", "/data/data.txt");
```

```
trying to read file:
"Hello World at 1719851282.5109031sHello World at 1719851299.0819795sHello World at 1719851317.8934608s\n"
-----------------------------------------------------
trying to write file:
thread '<unnamed>' panicked at src\lib.rs:24:34:
called `Result::unwrap()` on an `Err` value: Os { code: 58, kind: Unsupported, message: "Not supported" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'main' panicked at runtime\examples\fs.rs:27:6:
called `Result::unwrap()` on an `Err` value: error while executing at wasm backtrace:
    0: 0x234d2 - fs.wasm!__rust_start_panic
    1: 0x232a1 - fs.wasm!rust_panic
    2: 0x231da - fs.wasm!std::panicking::rust_panic_with_hook::hd3fb69bc0aea298a
    3: 0x22467 - fs.wasm!std::panicking::begin_panic_handler::{{closure}}::h4d99b90b43f79472
    4: 0x223ca - fs.wasm!std::sys_common::backtrace::__rust_end_short_backtrace::h5691573a73161cb1
    5: 0x22bca - fs.wasm!rust_begin_unwind
    6: 0x303e9 - fs.wasm!core::panicking::panic_fmt::hdb62f5cdb45533e4
    7: 0x3234d - fs.wasm!core::result::unwrap_failed::h30d23efcc9e41efc
    8: 0x36c2 - fs.wasm!fs::try_write::inner::h0b3b0df8e129f5cc
    9: 0x29cd - fs.wasm!try_write
   10: 0x35e4a - fs.wasm!try_write.command_export
note: using the `WASMTIME_BACKTRACE_DETAILS=1` environment variable may show more debugging information

Caused by:
    wasm trap: wasm `unreachable` instruction executed

Stack backtrace:
   0: std::backtrace_rs::backtrace::dbghelp64::trace
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library\std\src\..\..\backtrace\src\backtrace\dbghelp64.rs:99
   1: std::backtrace_rs::backtrace::trace_unsynchronized
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library\std\src\..\..\backtrace\src\backtrace\mod.rs:66
   2: std::backtrace::Backtrace::create
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library\std\src\backtrace.rs:331
   3: std::backtrace::Backtrace::capture
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library\std\src\backtrace.rs:296
   4: anyhow::error::impl$1::from<wasmtime_environ::trap_encoding::Trap>
             at C:\Users\muham\.cargo\registry\src\index.crates.io-6f17d22bba15001f\anyhow-1.0.86\src\error.rs:565
   5: core::convert::impl$3::into<wasmtime_environ::trap_encoding::Trap,anyhow::Error>
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6\library\core\src\convert\mod.rs:759
   6: wasmtime_environ::impl$1::into_anyhow<wasmtime_environ::trap_encoding::Trap>
             at C:\Users\muham\.cargo\registry\src\index.crates.io-6f17d22bba15001f\wasmtime-environ-22.0.0\src\lib.rs:90
   7: wasmtime::runtime::trap::from_runtime_box
             at C:\Users\muham\.cargo\registry\src\index.crates.io-6f17d22bba15001f\wasmtime-22.0.0\src\runtime\trap.rs:118
   8: wasmtime::runtime::func::invoke_wasm_and_catch_traps::closure$0<extism::current_plugin::CurrentPlugin,wasmtime::runtime::func::impl$1::call_unchecked_raw::closure_env$0<extism::current_plugin::CurrentPlugin> >
             at C:\Users\muham\.cargo\registry\src\index.crates.io-6f17d22bba15001f\wasmtime-22.0.0\src\runtime\func.rs:1597
   9: enum2$<core::result::Result<tuple$<>,alloc::boxed::Box<wasmtime::runtime::vm::traphandlers::Trap,alloc::alloc::Global> > >::map_err<tuple$<>,alloc::boxed::Box<wasmtime::runtime::vm::traphandlers::Trap,alloc::alloc::Global>,anyhow::Error,wasmtime::runtime:
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6\library\core\src\result.rs:829
  10: wasmtime::runtime::func::invoke_wasm_and_catch_traps<extism::current_plugin::CurrentPlugin,wasmtime::runtime::func::impl$1::call_unchecked_raw::closure_env$0<extism::current_plugin::CurrentPlugin> >
             at C:\Users\muham\.cargo\registry\src\index.crates.io-6f17d22bba15001f\wasmtime-22.0.0\src\runtime\func.rs:1597
  11: wasmtime::runtime::func::Func::call_unchecked_raw<extism::current_plugin::CurrentPlugin>
             at C:\Users\muham\.cargo\registry\src\index.crates.io-6f17d22bba15001f\wasmtime-22.0.0\src\runtime\func.rs:1063
  12: wasmtime::runtime::func::Func::call_unchecked<ref_mut$<wasmtime::runtime::store::context::StoreContextMut<extism::current_plugin::CurrentPlugin> > >
             at C:\Users\muham\.cargo\registry\src\index.crates.io-6f17d22bba15001f\wasmtime-22.0.0\src\runtime\func.rs:1049
  13: wasmtime::runtime::func::Func::call_impl_do_call<extism::current_plugin::CurrentPlugin>
             at C:\Users\muham\.cargo\registry\src\index.crates.io-6f17d22bba15001f\wasmtime-22.0.0\src\runtime\func.rs:1243
  14: wasmtime::runtime::func::Func::call<ref_mut$<wasmtime::runtime::store::Store<extism::current_plugin::CurrentPlugin> > >
             at C:\Users\muham\.cargo\registry\src\index.crates.io-6f17d22bba15001f\wasmtime-22.0.0\src\runtime\func.rs:1002
  15: extism::plugin::Plugin::raw_call<ref$<str$>,ref$<str$> >
             at .\src\plugin.rs:753
  16: extism::plugin::Plugin::call<ref$<str$>,ref$<str$>,ref$<str$> >
             at .\src\plugin.rs:900
  17: fs::main
             at .\examples\fs.rs:25
  18: core::ops::function::FnOnce::call_once<void (*)(),tuple$<> >
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6\library\core\src\ops\function.rs:250
  19: core::hint::black_box
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6\library\core\src\hint.rs:337
  20: std::sys_common::backtrace::__rust_begin_short_backtrace<void (*)(),tuple$<> >
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6\library\std\src\sys_common\backtrace.rs:155
  21: std::rt::lang_start::closure$0<tuple$<> >
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6\library\std\src\rt.rs:166
  22: std::rt::lang_start_internal
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library\std\src\rt.rs:148
  23: std::rt::lang_start<tuple$<> >
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6\library\std\src\rt.rs:165
  24: main
  25: invoke_main
             at D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78
  26: __scrt_common_main_seh
             at D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
  27: BaseThreadInitThunk
  28: RtlUserThreadStart
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library\std\src\panicking.rs:645
   1: core::panicking::panic_fmt
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library\core\src\panicking.rs:72
   2: core::result::unwrap_failed
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library\core\src\result.rs:1654
   3: enum2$<core::result::Result<ref$<str$>,anyhow::Error> >::unwrap
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6\library\core\src\result.rs:1077
   4: fs::main
             at .\examples\fs.rs:25
   5: core::ops::function::FnOnce::call_once<void (*)(),tuple$<> >
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6\library\core\src\ops\function.rs:250
   6: core::hint::black_box
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6\library\core\src\hint.rs:337
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
error: process didn't exit successfully: `D:\dylibso\extism\target\debug\examples\fs.exe` (exit code: 101)
```

Writable:
```rs
    let manifest = Manifest::new([url])
        .with_allowed_path("D:/x/rust/fs/data".to_string(), "/data")
        .with_config_key("path", "/data/data.txt");

```

```
trying to read file:
"Hello World at 1719851282.5109031sHello World at 1719851299.0819795sHello World at 1719851317.8934608s\n"
-----------------------------------------------------
trying to write file:
"Hello World at 1719851282.5109031sHello World at 1719851299.0819795sHello World at 1719851317.8934608s\nHello World at 1719851500.7803263s\n"
done!
```
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.

4 participants