Skip to content

Conversation

andreakarasho
Copy link
Contributor

The PR aims to fix #1377
Beside that other languages still leaking because of missing free calls in this case.

wit file:

package tests:component@0.1.0;

world test {
    export accept-string: func(s: string, b: string);
}

Generated code:

// Generated by `wit-bindgen` 0.46.0. DO NOT EDIT!
// <auto-generated />
#nullable enable

namespace TestWorld {

    public interface ITestWorld {
        static abstract void AcceptString(string s, string b);

    }

    namespace exports {
        public static class TestWorld
        {

            [global::System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute(EntryPoint = "accept-string")]
            public static unsafe void wasmExportAcceptString(nint p0, int p1, nint p2, int p3) {

                var str = global::System.Text.Encoding.UTF8.GetString((byte*)p0, p1);
                global::System.Runtime.InteropServices.NativeMemory.Free((void*)p0);

                var str0 = global::System.Text.Encoding.UTF8.GetString((byte*)p2, p3);
                global::System.Runtime.InteropServices.NativeMemory.Free((void*)p2);

                TestWorldImpl.AcceptString((str), (str0));

            }

        }
    }

}

@Copilot Copilot AI review requested due to automatic review settings September 20, 2025 21:18
@andreakarasho andreakarasho changed the title [C#] Fixes mem leak: https://github.com/bytecodealliance/wit-bindgen/issues/1377 [C#] Fixes mem leak on string pointer Sep 20, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Fix memory leak in C# codegen for string lifting by freeing the unmanaged buffer after decoding to a managed string.

  • Introduce a temporary variable for the lifted string and immediately free the unmanaged pointer.
  • Adjust result handling to use the temporary variable rather than an inline expression.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@andreakarasho andreakarasho marked this pull request as draft September 20, 2025 21:44
@andreakarasho andreakarasho marked this pull request as ready for review September 20, 2025 21:59
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@andreakarasho andreakarasho marked this pull request as draft September 20, 2025 22:20
@pavelsavara pavelsavara added the gen-c# Related to the C# code generator label Sep 21, 2025
@andreakarasho andreakarasho marked this pull request as ready for review September 21, 2025 21:10
@andreakarasho
Copy link
Contributor Author

So the final solution is to check for size > 0.
Marked the PR ready.

@andreakarasho
Copy link
Contributor Author

Covered cases ListCanonLift and ListLift too

@pavelsavara
Copy link
Collaborator

LGMT, @yowl , @jsturtevant I would leave the approval with one of you

@jsturtevant jsturtevant added this pull request to the merge queue Sep 23, 2025
Merged via the queue into bytecodealliance:main with commit 6a1e68a Sep 23, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gen-c# Related to the C# code generator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

C#: parameters are leaking
3 participants