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

Support deinstantiation (mod.Close) #293

Closed
codefromthecrypt opened this issue Feb 24, 2022 · 4 comments · Fixed by #360
Closed

Support deinstantiation (mod.Close) #293

codefromthecrypt opened this issue Feb 24, 2022 · 4 comments · Fixed by #360
Assignees

Comments

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Feb 24, 2022

We need to eventually handle long lived processes, and graceful shutdown. If we had a way to deinstantiate a module, I think we can also handle a WASI quit signal scoped to module instead of the host process (#290)

I think we should make a new type that implements ModuleExports called wazero.InstantiatedModule (we maybe need this for host also, but let's start with normal modules). The only additional function there would be .Close and this would have the following effects.

  • set a goroutine visible status on the internal moduleInstance closeCalled
    • this may need to be an array of length one to ensure visibility across goroutines, and 1 means closeCalled
  • make shared implementations err if they see this status which will cause existing functions to exit
    • make engine.call implementations check this status prior to invoking new functions and return an error (I think this handles table also as table is only call_indirect and not exposed otherwise)
    • make memory, global implementations check this status before read/write and return an error
  • ensure calls begin with a CancelContext added to the callEngine, and invoke that hook so that any host functions doing I/O finish.
    • this could possibly be limited to wrapping the context in wazero.StartWASICommand as maybe non-wasi shouldn't be doing I/O anyway
  • remove all the references for this module in Store added by instantiate
    • ensure nice errors when calling an imported function, memory, table or global that was removed
@codefromthecrypt
Copy link
Contributor Author

This would also be needed for wapc-go (cc @pkedy) because it is otherwise a memory leak. For example, modules are instantiated with instance counters, which I assume would eventually run out of memory depending on the scope and frequency of creation (remember memory is required by WASI and minimum memory is 64KB).

// Instantiate creates a single instance of the module with its own memory.
func (m *Module) Instantiate() (wapc.Instance, error) {
	moduleName := fmt.Sprintf("%d", atomic.AddUint64(&m.instanceCounter, 1))

@codefromthecrypt
Copy link
Contributor Author

There's a not really documented thing in AssemblyScript that would also needs this for abort handling. If someone can find docs on it please do!

// envAbort is the AssemblyScript abort handler.
// Ex in WebAssembly 1.0 (MVP) Text Format:
//	(import "env" "abort" (func $~lib/builtins/abort (param i32 i32 i32 i32)))
func (t *thing) envAbort(ctx wasm.ModuleContext, messageOffset, fileOffset, line, col uint32) {
	t.module.Close()
}

Note: AssemblyScript maybe maps this function to WASI sigabrt ...

@mathetake
Copy link
Member

yeah we also tweak Engine interface so that we can unmap the MMapped memory regions

mathetake added a commit that referenced this issue Mar 1, 2022
This commit refactors store.go and adds store.ReleaseModuleInstance.
Notably, this removes the "rollback" functions in InstantiateModule
which we had used to rollback the mutated state when we encounter
the initialization failure. Instead, we separate out the validation from
initialization and migrate most of the validation logics into module.go

As for ReleaseModuleInstance, that is necessary to complete #293,
will be leveraged to make it possible for store to be used for long-running
processes and environment.

Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
@mathetake
Copy link
Member

so the preliminary work #294 has just landed, so the next is to make store goroutine-safe

mathetake added a commit that referenced this issue Mar 14, 2022
This commit adds Module.Close API, which can be used to indicate that
the module will no longer be used and setting finalizers for the allocated
resources. Notably, this will set finalizers on compiledFunction to deallocate
mapped memory regions as well as removing them from the store-wide
storage. As a result, users can re-instantiate a module for the same name
while having the safety -- Calling Module.Close is safe even when there are
outstanding function calls as we never explicitly deallocate but indirectly
do that via Goruntine's finalizers.

resolves: #293

Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
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 a pull request may close this issue.

2 participants