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.

Sign up for GitHub

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

Jump to bottom

[C#] Using C API fails at runtime when targeting .NET 4.7.2 #36812

Closed
CurtHagenlocher opened this issue Jul 21, 2023 · 3 comments · Fixed by #36813
Closed

[C#] Using C API fails at runtime when targeting .NET 4.7.2 #36812

CurtHagenlocher opened this issue Jul 21, 2023 · 3 comments · Fixed by #36813
Assignees
@CurtHagenlocher
Labels
Component: C# Critical Fix Bugfixes for security vulnerabilities, crashes, or invalid data. Priority: Blocker Marks a blocker for the release Type: bug
Milestone
13.0.0

Comments

@CurtHagenlocher
Copy link
Contributor

Describe the bug, including details regarding any error messages, version, and platform.

The code fails with an error message along the lines of "Cannot marshal 'parameter #1': Pointers cannot reference marshaled structures. Use ByRef instead."

Component(s)

C#
@github-actions github-actions bot mentioned this issue Jul 21, 2023
Merged
@westonpace westonpace added Priority: Blocker Marks a blocker for the release Critical Fix Bugfixes for security vulnerabilities, crashes, or invalid data. labels Jul 21, 2023
westonpace pushed a commit that referenced this issue Jul 21, 2023
@CurtHagenlocher
GH-36812: [C#] Fix C API support to work with .NET desktop framework (#…
b557e85 
…36813)

### What changes are included in this PR?

The C API support in the C# library has been modified to work correctly on .NET 4.7.2.
The tests have been modified to work correctly on .NET 4.7.2, though that platform is disabled by default as the Python interop seem to cause a hang when unloading the xUnit AppDomain.

**This PR contains a "Critical Fix".**
* Closes: #36812

Authored-by: Curt Hagenlocher <[email protected]>
Signed-off-by: Weston Pace <[email protected]>
@westonpace westonpace modified the milestones: 14.0.0, 13.0.0 Jul 21, 2023
@kou
Copy link
Member

kou commented Jul 24, 2023

Can we verify this case in our verification script?

@CurtHagenlocher
Copy link
Contributor Author

I have a separate PR (#36818) which enables testing under .NET 4.7.2 under Windows. (Unfortunately, I didn't figure out how to do this in time to add it to the previous PR.) Arguably, a better approach would be to build for 4.7.2 on every platform but enable the test only under Windows. Unfortunately, I didn't have the time to figure out how to modify the CI scripts to manage that; they currently seem to run all the tests that are built which means that they would try to run the 4.7.2 tests even on platforms where it's not supported.

raulcd pushed a commit that referenced this issue Jul 24, 2023
@CurtHagenlocher @raulcd
GH-36812: [C#] Fix C API support to work with .NET desktop framework (#…
bc75d67 
…36813)

The C API support in the C# library has been modified to work correctly on .NET 4.7.2.
The tests have been modified to work correctly on .NET 4.7.2, though that platform is disabled by default as the Python interop seem to cause a hang when unloading the xUnit AppDomain.

**This PR contains a "Critical Fix".**
* Closes: #36812

Authored-by: Curt Hagenlocher <[email protected]>
Signed-off-by: Weston Pace <[email protected]>
@CurtHagenlocher CurtHagenlocher mentioned this issue Jul 24, 2023
Closed
@kou
Copy link
Member

kou commented Jul 24, 2023

I see.
Thanks!

R-JunmingChen pushed a commit to R-JunmingChen/arrow that referenced this issue Aug 20, 2023
@CurtHagenlocher @R-JunmingChen
apacheGH-36812: [C#] Fix C API support to work with .NET desktop fram…
8d041ee 
…ework (apache#36813)

### What changes are included in this PR?

The C API support in the C# library has been modified to work correctly on .NET 4.7.2.
The tests have been modified to work correctly on .NET 4.7.2, though that platform is disabled by default as the Python interop seem to cause a hang when unloading the xUnit AppDomain.

**This PR contains a "Critical Fix".**
* Closes: apache#36812

Authored-by: Curt Hagenlocher <[email protected]>
Signed-off-by: Weston Pace <[email protected]>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
@CurtHagenlocher @loicalleyne
apacheGH-36812: [C#] Fix C API support to work with .NET desktop fram…
88a2c91 
…ework (apache#36813)

### What changes are included in this PR?

The C API support in the C# library has been modified to work correctly on .NET 4.7.2.
The tests have been modified to work correctly on .NET 4.7.2, though that platform is disabled by default as the Python interop seem to cause a hang when unloading the xUnit AppDomain.

**This PR contains a "Critical Fix".**
* Closes: apache#36812

Authored-by: Curt Hagenlocher <[email protected]>
Signed-off-by: Weston Pace <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@CurtHagenlocher CurtHagenlocher

Labels
Component: C# Critical Fix Bugfixes for security vulnerabilities, crashes, or invalid data. Priority: Blocker Marks a blocker for the release Type: bug
Projects
None yet
Milestone
13.0.0
Development

Successfully merging a pull request may close this issue.

GH-36812: [C#] Fix C API support to work with .NET desktop framework CurtHagenlocher/arrow
3 participants
@kou @CurtHagenlocher @westonpace