Skip to content

Imported Resources don't provide a finalizer #58

Description

@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" />

Activity

  1. yowl commented on Nov 17, 2024

    @yowl
    Contributor

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

  2. kaivol commented on Nov 19, 2024

    @kaivol
    Author

    Okay, never mind, this seems to be intended: bytecodealliance/wit-bindgen@c648fc7

    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.

  3. yowl commented on Nov 19, 2024

    @yowl
    Contributor

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

  4. jsturtevant commented on Nov 26, 2024

    @jsturtevant
    Contributor

    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.

  5. kaivol commented on Mar 12, 2025

    @kaivol
    Author

    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?

  6. jsturtevant commented on Mar 12, 2025

    @jsturtevant
    Contributor

    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.

  7. kaivol commented on Mar 14, 2025

    @kaivol
    Author

    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();
            }
        }
    }
  8. jsturtevant commented on Mar 20, 2025

    @jsturtevant
    Contributor

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions