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

Cleanup Interop in System.Drawing.Common to follow repo guidelines #49469

Closed
safern opened this issue Mar 11, 2021 · 5 comments
Closed

Cleanup Interop in System.Drawing.Common to follow repo guidelines #49469

safern opened this issue Mar 11, 2021 · 5 comments
Labels
area-System.Drawing help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@safern
Copy link
Member

safern commented Mar 11, 2021

I just submitted a PR to cleanup some of the System.Drawing.Common Interop. Having that model lead to a bug where we called a Windows PInvoke in Unix causing an exception.

I partially cleaned this up as part of the fix in: #49405

@ghost
Copy link

ghost commented Mar 11, 2021

Tagging subscribers to this area: @safern, @tarekgh
See info in area-owners.md if you want to be subscribed.

Issue Details

I just submitted a PR to cleanup some of the System.Drawing.Common Interop. Having that model lead to a bug where we called a Windows PInvoke in Unix causing an exception.

I partially cleaned this up as part of the fix in: #49405

Author: safern
Assignees: -
Labels:

area-System.Drawing

Milestone: -

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Mar 11, 2021
@safern safern added this to the 6.0.0 milestone Mar 11, 2021
@safern safern added help wanted [up-for-grabs] Good issue for external contributors and removed untriaged New issue has not been triaged by the area owner labels Mar 11, 2021
@huoyaoyuan
Copy link
Member

Working on it. @safern some questions related to interop:

It's targeting down level frameworks. Can function pointers be used for interop in this case?

It's using HandleRef heavily. However, HandleRef is documented to be replaced by SafeHandle. Is such a refactor required?

@safern
Copy link
Member Author

safern commented Jun 7, 2021

It's targeting down level frameworks. Can function pointers be used for interop in this case?
It's using HandleRef heavily. However, HandleRef is documented to be replaced by SafeHandle. Is such a refactor required?

@jkotas could you help with those questions? I don't know the answer.

@jkotas
Copy link
Member

jkotas commented Jun 7, 2021

It's targeting down level frameworks. Can function pointers be used for interop in this case?

Function pointers can be used for targets prior .NET 5 with some limitations. In particular, the unmanaged calling convention has to be always explicitly specified prior to .NET 5.

HandleRef is documented to be replaced by SafeHandle. Is such a refactor required?

No, it is not strictly required to avoid HandleRefs. The code written using SafeHandles tends to be more robust (less chance of handle leaks or use-after-free bugs).

@safern safern modified the milestones: 6.0.0, 7.0.0 Jul 22, 2021
@maryamariyan maryamariyan added help wanted [up-for-grabs] Good issue for external contributors and removed help wanted [up-for-grabs] Good issue for external contributors labels Dec 22, 2021
@ViktorHofer
Copy link
Member

Closing as dup of #51565

@ViktorHofer ViktorHofer closed this as not planned Won't fix, can't repro, duplicate, stale Aug 4, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Drawing help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

No branches or pull requests

5 participants