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

Fix memory leaks for return values and some out params. #1200

Conversation

jkoritzinsky
Copy link
Member

Fix some memory leaks we had where we didn't generate code to free native resources for out parameters that use custom native type marshalling or all return values.

Also set a flag in the csproj that enables a nice debugging innerloop option for roslyn components in the csproj settings (in our case running the generator on the integration tests)

Fix some memory leaks we had where we didn't generate code to free native resources for out parameters that use custom native type marshalling or all return values.

Also set a flag in the csproj that enables a nice debugging innerloop option for roslyn components in the csproj settings (in our case running the generator on the integration tests)
@jkoritzinsky jkoritzinsky added the area-DllImportGenerator Source Generated stubs for P/Invokes in C# label Jun 2, 2021
@jkoritzinsky jkoritzinsky added this to the 6.0 milestone Jun 2, 2021
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT left a comment

Choose a reason for hiding this comment

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

How are we testing our clean up logic? I am fine adding this in a future PR. I can imagine using some of the MarshalUsing logic in some cases. Thoughts?

@jkoritzinsky
Copy link
Member Author

Currently, we aren't doing a good job of testing the cleanup logic (I found this when inspecting codegen from #1197).

With a specially-designed managed/native type pair, we can set something up to explicitly test that we free when expected, at least for the custom native type marshalling (the MarshalUsing scenario). For the other marshallers like the built-in string marshalling, our only testing is manual spot checks. That's one of the reasons I'd like to try moving more of the marshallers to be based on the custom native type marshalling and not directly in the stub if possible. #1197 and some follow-up work I have in a branch in my fork move us more in that direction.

@jkoritzinsky jkoritzinsky merged commit b11a26a into dotnet:feature/DllImportGenerator Jun 3, 2021
@jkoritzinsky jkoritzinsky deleted the seal-return-out-leaks branch June 3, 2021 02:16
jkoritzinsky added a commit to jkoritzinsky/runtime that referenced this pull request Sep 20, 2021
…melab#1200)

Fix some memory leaks we had where we didn't generate code to free native resources for out parameters that use custom native type marshalling or all return values.

Also set a flag in the csproj that enables a nice debugging innerloop option for roslyn components in the csproj settings (in our case running the generator on the integration tests)

Commit migrated from dotnet/runtimelab@b11a26a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-DllImportGenerator Source Generated stubs for P/Invokes in C#
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants