Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

Three Interop bug fixes#3488

Merged
shrah merged 1 commit intodotnet:masterfrom
shrah:bugfix
May 3, 2017
Merged

Three Interop bug fixes#3488
shrah merged 1 commit intodotnet:masterfrom
shrah:bugfix

Conversation

@shrah
Copy link
Contributor

@shrah shrah commented May 3, 2017

This change contains the following bug fixes:

  • Call GC.KeepAlive for delegate marshalling. Sergiy encountered a
    scenario where the delegate got collected when we tried to do a reverse
    P/Invoke. This change adds a GC.KeepAlive at the end of the marshalling
    stub to ensure the delegate is not collected during the duration of the
    call.
  • For ForwardDelegateCreation Stub set the correct calling convention.
    Also set the IsPInvoke property of the ForwardDelegateCreation stub to
    be true so that the JIT flag CORJIT_FLAG_IL_STUB is set which is
    necessary for RyuJit to inline the call.
  • Handle the scenarios when MarshalAs can be null.

This change contains the following bug fixes:
* Call GC.KeepAlive for delegate marshalling. Sergiy encountered a
scenario where the delegate got collected when we tried to do a reverse
P/Invoke. This change adds a GC.KeepAlive at the end of the marshalling
stub to ensure the delegate is not collected during the duration of the
call.
* For ForwardDelegateCreation Stub set the correct calling convention.
Also set the IsPInvoke property of the ForwardDelegateCreation stub to
be true so that the JIT flag CORJIT_FLAG_IL_STUB is set which is
necessary for RyuJit to inline the call.
* Handle the scenarios when MarshalAs can be null.
&& MarshallerType == MarshallerType.Argument)
{
LoadManagedValue(codeStream);
codeStream.Emit(ILOpcode.call, _ilCodeStreams.Emitter.NewToken(InteropTypes.GetGC(Context).GetKnownMethod("KeepAlive", null)));
Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking about this a bit more, we should only need to emit the GC.KeepAlive call in the unmarshal stream. CoreCLR does the same thing.

Copy link
Contributor

@yizhang82 yizhang82 left a comment

Choose a reason for hiding this comment

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

LGTM

@shrah shrah merged commit 8d44b6d into dotnet:master May 3, 2017
@shrah shrah deleted the bugfix branch May 3, 2017 07:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants