Skip to content

Crash when calling back through method with ref ValueType parameter #965

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

Closed
zooba opened this issue Oct 2, 2019 · 6 comments
Closed

Crash when calling back through method with ref ValueType parameter #965

zooba opened this issue Oct 2, 2019 · 6 comments
Labels

Comments

@zooba
Copy link

zooba commented Oct 2, 2019

Hi. In response to this query I had one of my colleagues debug an issue, and he found that PythonNet does not correctly generate code when calling into a function with a "ref ValueType" parameter. In his words:

The problem is here in the PythonNet code:

        for (var i = 0; i < parameters.Length; ++i)
        {
            il.Emit(OpCodes.Ldloc_0);
            il.Emit(OpCodes.Ldc_I4, i);
            il.Emit(OpCodes.Ldarg, i + 1);
            if (parameterTypes[i].IsValueType)
            {
                il.Emit(OpCodes.Box, parameterTypes[i]);
            }
            il.Emit(OpCodes.Stelem, typeof(object));
        }

The code is trying to store all of the incoming arguments in an array of type Object[]. The cases are:

  1. If the incoming argument is of reference type, the reference can simply be stored in the array.
  2. If the incoming argument is of struct type, the argument needs to be boxed into an object, which can then be stored in the array.
  3. If the incoming argument is a “byref” (i.e., “ref” keyword in C#), more complicated handling is needed, including:
    o On the way in, the byref needs to read (via ldobj if the underlying type is a struct, or ldind.ref otherwise) to load the actual argument data that needs to flow through.
    o On the way out, the byref needs to written (via stobj or stind.ref) to make sure any updates applied by the callee are reflected back to the caller.

The code above handles #1 and #2, but doesn’t handle #3.


Hopefully that's enough for someone to create or contribute a fix.

@filmor
Copy link
Member

filmor commented Oct 4, 2019

Thank you very much for the detailed analysis :)

A hard crash is of course not acceptable, so we'll need to fix this soon. I'll see if I understand enough of this to do something about the issue.

@filmor filmor added this to the 2.5.0 milestone Oct 4, 2019
@filmor filmor added the bug label Oct 4, 2019
@freakboy3742
Copy link

@filmor I don't know the status of fiscal sponsorship for Python.net, but if this is a problem where a fix can be expedited by throwing money at the problem, I may be in a position to supply that money.

@filmor
Copy link
Member

filmor commented Oct 10, 2019

The status of fiscal sponsorship for Python.NET is that there is none. The main issue with throwing money at the problem is finding someone to throw money at. It's not like we have a wealth of programmers with a lot of free time eager to work on problems that are not necessarily their own here.

@lostmsu
Copy link
Member

lostmsu commented Mar 10, 2020

@freakboy3742 I am sure if you list a bounty here somebody may pick it up.

@freakboy3742
Copy link

I have no idea what amount of money would get this job done; if you're interested in doing this as work-for-hire, get in touch.

@lostmsu
Copy link
Member

lostmsu commented Feb 21, 2021

This should have been fixed by #1364

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants