Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: mockingbirdnest/Principia
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: d6da6cabbaf0
Choose a base ref
...
head repository: mockingbirdnest/Principia
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 040ca5b4d45a
Choose a head ref
  • 13 commits
  • 8 files changed
  • 1 contributor

Commits on Apr 5, 2020

  1. Copy the full SHA
    097f168 View commit details

Commits on Apr 6, 2020

  1. Copy the full SHA
    b315a9e View commit details
  2. Copy the full SHA
    fed175f View commit details
  3. Copy the full SHA
    5ab5779 View commit details
  4. Fix all the marshallers.

    pleroy committed Apr 6, 2020
    Copy the full SHA
    4dc2e2a View commit details
  5. New generated files.

    pleroy committed Apr 6, 2020
    Copy the full SHA
    0dc90dd View commit details
  6. Remove generated files.

    pleroy committed Apr 6, 2020
    Copy the full SHA
    c9ca9aa View commit details
  7. Fix UTF-16.

    pleroy committed Apr 6, 2020
    Copy the full SHA
    83f850a View commit details

Commits on Apr 7, 2020

  1. After egg's first review.

    pleroy committed Apr 7, 2020
    Copy the full SHA
    c20dffa View commit details
  2. After egg's review, part 2.

    pleroy committed Apr 7, 2020
    Copy the full SHA
    025372a View commit details
  3. Copy the full SHA
    be8e1ab View commit details
  4. Another round of review.

    pleroy committed Apr 7, 2020
    Copy the full SHA
    f566a28 View commit details
  5. Merge pull request #2521 from pleroy/2339b

    Add an intermediate class, MonoMarshaler, to hide Mono bugs
    pleroy authored Apr 7, 2020
    Copy the full SHA
    040ca5b View commit details
17 changes: 5 additions & 12 deletions ksp_plugin_adapter/class_marshalers.cs
Original file line number Diff line number Diff line change
@@ -10,7 +10,7 @@ private static IntPtr At(this IntPtr pointer, long offset) {
return new IntPtr(pointer.ToInt64() + offset);
}

internal class InBodyParametersMarshaler : ICustomMarshaler {
internal class InBodyParametersMarshaler : MonoMarshaler {

[StructLayout(LayoutKind.Sequential)]
internal class BodyParametersRepresentation {
@@ -34,7 +34,7 @@ public static ICustomMarshaler GetInstance(string s) {
return instance_;
}

public void CleanUpNativeData(IntPtr native_data) {
public override void CleanUpNativeDataImplementation(IntPtr native_data) {
var representation = new BodyParametersRepresentation();
Marshal.PtrToStructure(native_data, representation);
for (int i = 0; i < representation.geopotential_size; ++i) {
@@ -47,7 +47,8 @@ public void CleanUpNativeData(IntPtr native_data) {
Marshal.FreeHGlobal(native_data);
}

public IntPtr MarshalManagedToNative(object managed_object) {
public override IntPtr MarshalManagedToNativeImplementation(
object managed_object) {
var parameters = managed_object as BodyParameters;
Debug.Assert(parameters != null, nameof(parameters) + " != null");
var representation = new BodyParametersRepresentation{
@@ -83,18 +84,10 @@ public IntPtr MarshalManagedToNative(object managed_object) {
return buffer;
}

public object MarshalNativeToManaged(IntPtr native_data) {
public override object MarshalNativeToManaged(IntPtr native_data) {
throw Log.Fatal("InBodyParametersMarshaler.MarshalNativeToManaged");
}

public void CleanUpManagedData(object managed_data) {
throw Log.Fatal("InBodyParametersMarshaler.CleanUpManagedData");
}

int ICustomMarshaler.GetNativeDataSize() {
return -1;
}

private static readonly InBodyParametersMarshaler instance_ =
new InBodyParametersMarshaler();
}
30 changes: 10 additions & 20 deletions ksp_plugin_adapter/disposable_marshaller.cs
Original file line number Diff line number Diff line change
@@ -4,57 +4,47 @@
namespace principia {
namespace ksp_plugin_adapter {

internal class DisposableIteratorMarshaller : ICustomMarshaler {
void ICustomMarshaler.CleanUpManagedData(object managed_object) {}

int ICustomMarshaler.GetNativeDataSize() {
return -1;
}

internal class DisposableIteratorMarshaller : MonoMarshaler {
public static ICustomMarshaler GetInstance(string s) {
return instance_;
}

void ICustomMarshaler.CleanUpNativeData(IntPtr native_data) {}
public override void CleanUpNativeDataImplementation(IntPtr native_data) {}

IntPtr ICustomMarshaler.MarshalManagedToNative(object managed_object) {
public override IntPtr MarshalManagedToNativeImplementation(
object managed_object) {
if (managed_object == null) {
return IntPtr.Zero;
}
var disposable_iterator = managed_object as DisposableIterator;
return disposable_iterator.IntPtr;
}

object ICustomMarshaler.MarshalNativeToManaged(IntPtr iterator) {
public override object MarshalNativeToManaged(IntPtr iterator) {
return new DisposableIterator(iterator);
}

private static readonly DisposableIteratorMarshaller instance_ =
new DisposableIteratorMarshaller();
}

internal class DisposablePlanetariumMarshaller : ICustomMarshaler {
void ICustomMarshaler.CleanUpManagedData(object managed_object) {}

int ICustomMarshaler.GetNativeDataSize() {
return -1;
}

internal class DisposablePlanetariumMarshaller : MonoMarshaler {
public static ICustomMarshaler GetInstance(string s) {
return instance_;
}

void ICustomMarshaler.CleanUpNativeData(IntPtr native_data) {}
public override void CleanUpNativeDataImplementation(IntPtr native_data) {}

IntPtr ICustomMarshaler.MarshalManagedToNative(object managed_object) {
public override IntPtr MarshalManagedToNativeImplementation(
object managed_object) {
if (managed_object == null) {
return IntPtr.Zero;
}
var disposable_planetarium = managed_object as DisposablePlanetarium;
return disposable_planetarium.IntPtr;
}

object ICustomMarshaler.MarshalNativeToManaged(IntPtr planetarium) {
public override object MarshalNativeToManaged(IntPtr planetarium) {
return new DisposablePlanetarium(planetarium);
}

1 change: 1 addition & 0 deletions ksp_plugin_adapter/ksp_plugin_adapter.csproj
Original file line number Diff line number Diff line change
@@ -133,6 +133,7 @@
<Compile Include="reference_frame_selector.cs" />
<Compile Include="source_location.cs" />
<Compile Include="style.cs" />
<Compile Include="mono_marshaler.cs" />
<Compile Include="utf8_marshaler.cs" />
<Compile Include="utf16_marshaler.cs" />
<Compile Include="window_renderer.cs" />
67 changes: 67 additions & 0 deletions ksp_plugin_adapter/mono_marshaler.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
using System;
using System.Collections.Generic;
using System.Runtime.InteropServices;

namespace principia {
namespace ksp_plugin_adapter {

// It seems that, for out parameters and returned value, Mono calls
// CleanUpNativeData on data that was not allocated by MarshalManagedToNative,
// see https://github.com/mono/mono/blob/941a335ea0f20c22a02a7947945f53787a56b2d3/mono/metadata/marshal-ilgen.c#L4555
// This appears to violate the contract documented by .Net, see
// https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.icustommarshaler?view=netframework-4.8#implementing-the-icustommarshaler-interface
// although in reality it seems that the .Net tests themselves don't expect
// CleanUpManagedData and GetNativeDataSize to be called, see
// https://github.com/dotnet/runtime/blob/4f9ae42d861fcb4be2fcd5d3d55d5f227d30e723/src/coreclr/tests/src/Interop/ICustomMarshaler/Primitives/ICustomMarshaler.cs#L147.
// Amid all this confusion, this class attempts to maintain a sane behavior,
// i.e., one that is close enough to the .Net documentation.
internal abstract class MonoMarshaler : ICustomMarshaler {
// Subclasses must override the following methods to implement the contract
// defined by .Net.
public abstract void CleanUpNativeDataImplementation(IntPtr native_data);
public abstract IntPtr MarshalManagedToNativeImplementation(
object managed_object);

// We have no evidence that this method is ever called.
void ICustomMarshaler.CleanUpManagedData(object managed_object) {}

void ICustomMarshaler.CleanUpNativeData(IntPtr native_data) {
IntPtr actual_native_data = IntPtr.Zero;
lock (allocated_intptrs_) {
if (allocated_intptrs_.Contains(native_data)) {
actual_native_data = native_data;
allocated_intptrs_.Remove(native_data);
}
}
if (actual_native_data != IntPtr.Zero) {
CleanUpNativeDataImplementation(actual_native_data);
}
}

// We have no evidence that this method is ever called.
int ICustomMarshaler.GetNativeDataSize() {
return -1;
}

IntPtr ICustomMarshaler.MarshalManagedToNative(object managed_object) {
IntPtr result = MarshalManagedToNativeImplementation(managed_object);
lock (allocated_intptrs_) {
if (result != IntPtr.Zero) {
allocated_intptrs_.Add(result);
}
}
return result;
}

public abstract object MarshalNativeToManaged(IntPtr native_data);

// Since Mono calls CleanUpNativeData with IntPtrs that have not been
// allocated by MarshalManagedToNative, we track here all the values returned
// by MarshalManagedToNative and only process in CleanUpNativeData those that
// are found in this set.
private static readonly HashSet<IntPtr> allocated_intptrs_ =
new HashSet<IntPtr>();
}

} // namespace ksp_plugin_adapter
} // namespace principia
20 changes: 5 additions & 15 deletions ksp_plugin_adapter/optional_marshaler.cs
Original file line number Diff line number Diff line change
@@ -5,7 +5,7 @@
namespace principia {
namespace ksp_plugin_adapter {

internal class OptionalMarshaler<T> : ICustomMarshaler where T : struct {
internal class OptionalMarshaler<T> : MonoMarshaler where T : struct {
// In addition to implementing the |ICustomMarshaler| interface, custom
// marshalers must implement a static method called |GetInstance| that accepts
// a |String| as a parameter and has a return type of |ICustomMarshaler|,
@@ -14,22 +14,12 @@ public static ICustomMarshaler GetInstance(string s) {
return instance_;
}

void ICustomMarshaler.CleanUpManagedData(object managed_object) {}

void ICustomMarshaler.CleanUpNativeData(IntPtr native_data) {
if (native_data == IntPtr.Zero) {
return;
}
public override void CleanUpNativeDataImplementation(IntPtr native_data) {
Marshal.FreeHGlobal(native_data);
}

int ICustomMarshaler.GetNativeDataSize() {
// I think this is supposed to return -1, and I also think it doesn't
// matter, but honestly I'm not sure...
return -1;
}

IntPtr ICustomMarshaler.MarshalManagedToNative(object managed_object) {
public override IntPtr MarshalManagedToNativeImplementation(
object managed_object) {
if (managed_object == null) {
// This is not our job.
throw Log.Fatal("The runtime returns null for null objects");
@@ -55,7 +45,7 @@ IntPtr ICustomMarshaler.MarshalManagedToNative(object managed_object) {
return ptr;
}

object ICustomMarshaler.MarshalNativeToManaged(IntPtr native_data) {
public override object MarshalNativeToManaged(IntPtr native_data) {
if (native_data == IntPtr.Zero) {
return null;
} else {
40 changes: 30 additions & 10 deletions ksp_plugin_adapter/utf16_marshaler.cs
Original file line number Diff line number Diff line change
@@ -4,31 +4,51 @@
namespace principia {
namespace ksp_plugin_adapter {

internal class OutOwnedUTF16Marshaler : ICustomMarshaler {
void ICustomMarshaler.CleanUpManagedData(object managed_object) {}
// A marshaler that knows how to encode/decode UTF-16 strings.
internal abstract class UTF16Marshaler : MonoMarshaler {
public override void CleanUpNativeDataImplementation(IntPtr native_data) {
throw Log.Fatal("use |MarshalAs(UnmanagedType.LPWStr)| for in parameters");
}

public override IntPtr MarshalManagedToNativeImplementation(
object managed_object) {
throw Log.Fatal("use |MarshalAs(UnmanagedType.LPWStr)| for in parameters");
}

int ICustomMarshaler.GetNativeDataSize() {
return -1;
public object MarshalNativeToManagedImplementation(IntPtr native_data) {
return Marshal.PtrToStringUni(native_data);
}
}

// A marshaler for UTF-16 strings whose ownership is taken from C++. Useful for
// out parameters and returned values.
internal class OwnershipTransferUTF16Marshaler : UTF16Marshaler {
public static ICustomMarshaler GetInstance(string s) {
return instance_;
}

void ICustomMarshaler.CleanUpNativeData(IntPtr native_data) {
public override object MarshalNativeToManaged(IntPtr native_data) {
var result = MarshalNativeToManagedImplementation(native_data);
Interface.DeleteU16String(ref native_data);
return result;
}

IntPtr ICustomMarshaler.MarshalManagedToNative(object managed_object) {
throw Log.Fatal("use |MarshalAs(UnmanagedType.LPWStr)| for in parameters");
private static readonly OwnershipTransferUTF16Marshaler instance_ =
new OwnershipTransferUTF16Marshaler();
}

// A marshaler for UTF-16 strings whose ownership is not taken from C++.
internal class NoOwnershipTransferUTF16Marshaler : UTF16Marshaler {
public static ICustomMarshaler GetInstance(string s) {
return instance_;
}

object ICustomMarshaler.MarshalNativeToManaged(IntPtr native_data) {
public override object MarshalNativeToManaged(IntPtr native_data) {
return Marshal.PtrToStringUni(native_data);
}

private static readonly OutOwnedUTF16Marshaler instance_ =
new OutOwnedUTF16Marshaler();
private static readonly NoOwnershipTransferUTF16Marshaler instance_ =
new NoOwnershipTransferUTF16Marshaler();
}

} // namespace ksp_plugin_adapter
Loading