Imported Resources don't provide a finalizer

[kaivol]

Currently, projections for imported resources implement IDisposable, but they do not provide a finalizer. This means that if the user forgets to manually dispose the resource, its [resource-drop] function is never called.

I believe this is incorrect, and the resource should provide a finalizer that calls the resource's [resource-drop] function.

---

Version:

<PackageReference Include="BytecodeAlliance.Componentize.DotNet.Wasm.SDK" Version="0.4.0-preview00007" />

[yowl]

Seems like a bug, but with wit-bindgen, rather than this tooling, so should be https://github.com/bytecodealliance/wit-bindgen/issues I think

[kaivol]

Okay, never mind, this seems to be intended: https://github.com/bytecodealliance/wit-bindgen/commit/c648fc7b84dea32e66f87b760832b7f7a79dd63c

Nevertheless, it might be helpful to mention in the readme that resources must be disposed/dropped manually. Perhaps recommending to enable the corresponding warnings (CA2000, CA1001, CA2213)?

Feel free to close this issue if this is not the correct repository for these suggestions.

[yowl]

You are right, I'd forgotten about the child resources problem.

[jsturtevant]

The warnings look reasonable to turn on, Do you have an example where this triggers?

Not sure the best place to document, but we can certainly add a note the readme here.

[kaivol]

Sorry, I missed your reply.

The warnings look reasonable to turn on, Do you have an example where this triggers?

Examples of what exactly do you mean?

[jsturtevant]

Do you have examples of code that trigger would trigger the warnings? (CA2000, CA1001, CA2213)? Looking for examples that solve the issue so can demonstrate, why you would need these warnings and the solution in the readme.

[kaivol]

Sure thing:

using DemoWorld.wit.imports;
    
static class Foo
{
    static void CA2000()
    {
        var resource = new IDemoImports.DemoResource();
        // Warning CA2000 : Call System.IDisposable.Dispose on object created
        // by 'new IDemoImports.DemoResource()' before all references to it are
        // out of scope
    }
}

class CA1001
// Warning CA1001 : Type 'CA1001' owns disposable field(s) '_resource' but is
// not disposable
{
    private readonly IDemoImports.DemoResource? _resource = new();
}

class CA2213 : IDisposable
// Warning CA2213 : 'CA2213' contains field '_resource' that is of IDisposable
// type 'DemoResource?', but it is never disposed. Change the Dispose method on
// 'CA2213' to call Close or Dispose on this field
{
    private readonly IDemoImports.DemoResource? _resource = new();
    
    public void Dispose() {}
}

WIT file:

package demo:demo;

world demo {
    import demo-imports: interface {
        resource demo-resource {
            constructor();
        }
    }
}

[jsturtevant]

Thanks! I will see if we can add these warnings to the new template at some point so can help inform users of the pitfalls.