Skip to content

Uni 56832 another crash fix attempt #6

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

Closed
wants to merge 2 commits into from
Closed

Uni 56832 another crash fix attempt #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1f1f358
uni-56832: working crash solution on osx
Aug 24, 2018
9c56428
uni-56832: fix Runtime.csproj so we can build on mono/osx.
Aug 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions src/runtime/Python.Runtime.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<?xml version="1.0" encoding="utf-8"?>
<Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003" ToolsVersion="12.0">
<Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003" ToolsVersion="4.0">
<PropertyGroup>
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
<Platform Condition=" '$(Platform)' == '' ">x86</Platform>
<ProjectGuid>{097B4AC0-74E9-4C58-BCF8-C69746EC8271}</ProjectGuid>
<OutputType>Library</OutputType>
<AssemblyName>Python.Runtime</AssemblyName>
Expand All @@ -22,35 +22,35 @@
</PropertyGroup>
<!--We can relax binding to platform because code references no any platform dependent assemblies-->
<!--This will allows to use any build of this assebly as a compile ref assebly-->
<!--<PropertyGroup Condition=" '$(Platform)' == 'x86'">
<PropertyGroup Condition=" '$(Platform)' == 'x86'">
<PlatformTarget>x86</PlatformTarget>
</PropertyGroup>
<PropertyGroup Condition=" '$(Platform)' == 'x64'">
<PlatformTarget>x64</PlatformTarget>
</PropertyGroup>-->
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)' == 'ReleaseMono'">
<DefineConstants Condition="'$(DefineConstants)' == ''">PYTHON2;PYTHON27;UCS4</DefineConstants>
<DefineConstants Condition="'$(DefineConstants)' == ''">MONO_OSX;PYTHON2;PYTHON27;UCS2</DefineConstants>
<Optimize>true</Optimize>
<DebugType>pdbonly</DebugType>
<DefineConstants>PYTHON2;PYTHON27;UCS2</DefineConstants>
<!--<DefineConstants>PYTHON2;PYTHON27;UCS2</DefineConstants>-->
<Prefer32Bit>false</Prefer32Bit>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)' == 'ReleaseMonoPY3'">
<DefineConstants Condition="'$(DefineConstants)' == ''">PYTHON3;PYTHON36;UCS4</DefineConstants>
<DefineConstants Condition="'$(DefineConstants)' == ''">MONO_OSX;PYTHON3;PYTHON36;UCS4</DefineConstants>
<Optimize>true</Optimize>
<DebugType>pdbonly</DebugType>
<Prefer32Bit>false</Prefer32Bit>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)' == 'DebugMono'">
<DebugSymbols>true</DebugSymbols>
<DefineConstants Condition="'$(DefineConstants)' == ''">PYTHON2;PYTHON27;UCS4;TRACE;DEBUG</DefineConstants>
<DefineConstants Condition="'$(DefineConstants)' == ''">MONO_OSX;PYTHON2;PYTHON27;UCS2;TRACE;DEBUG</DefineConstants>
<Optimize>false</Optimize>
<DebugType>full</DebugType>
<Prefer32Bit>false</Prefer32Bit>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)' == 'DebugMonoPY3'">
<DebugSymbols>true</DebugSymbols>
<DefineConstants Condition="'$(DefineConstants)' == ''">PYTHON3;PYTHON36;UCS4;TRACE;DEBUG</DefineConstants>
<DefineConstants Condition="'$(DefineConstants)' == ''">MONO_OSX;PYTHON3;PYTHON36;UCS4;TRACE;DEBUG</DefineConstants>
<Optimize>false</Optimize>
<DebugType>full</DebugType>
<Prefer32Bit>false</Prefer32Bit>
Expand Down Expand Up @@ -171,8 +171,14 @@
<TargetAssembly>$(TargetPath)</TargetAssembly>
<TargetAssemblyPdb>$(TargetDir)$(TargetName).pdb</TargetAssemblyPdb>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'ReleaseMono|AnyCPU' ">
<DebugType></DebugType>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'ReleaseMonoPY3|AnyCPU' ">
<DebugType></DebugType>
</PropertyGroup>
<Target Name="AfterBuild">
<Copy SourceFiles="$(TargetAssembly)" DestinationFolder="$(PythonBuildDir)" />
<!--Copy SourceFiles="$(TargetAssemblyPdb)" Condition="Exists('$(TargetAssemblyPdb)')" DestinationFolder="$(PythonBuildDir)" /-->
</Target>
</Project>
</Project>
17 changes: 0 additions & 17 deletions src/runtime/runtime.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,23 +476,6 @@ internal static void Shutdown()
Exceptions.Shutdown();
ImportHook.Shutdown();
Py_Finalize();

// Now unload the Python library from memory and load it again, providing a
// fresh interpreter. This prevents a crash (exception) on second domain reload
// https://stackoverflow.com/questions/2445536/unload-a-dll-loaded-using-dllimport
if (_PythonDll != "__Internal")
{
IntPtr dllLocal = NativeMethods.LoadLibrary(_PythonDll);

// Twice: a first one for the call to LoadLibrary above,
// a second one for the original call to LoadLibrary (should result in unloading the Python library from memory)
NativeMethods.FreeLibrary(dllLocal);
NativeMethods.FreeLibrary(dllLocal);

// Here the Python library is supposed to be unloaded.
// Load it again in order to get a fresh interpreter
NativeMethods.LoadLibrary(_PythonDll);
}
}

// called *without* the GIL acquired by clr._AtExit
Expand Down
35 changes: 32 additions & 3 deletions src/runtime/typemanager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ internal static IntPtr AllocateTypeObject(string name)
/// </summary>
internal static void InitializeSlots(IntPtr type, Type impl)
{
var seen = new Hashtable(8);
var seen = new HashSet<string>();
Type offsetType = typeof(TypeOffset);

while (impl != null)
Expand All @@ -473,7 +473,7 @@ internal static void InitializeSlots(IntPtr type, Type impl)
continue;
}

if (seen[name] != null)
if (seen.Contains(name))
{
continue;
}
Expand All @@ -484,11 +484,40 @@ internal static void InitializeSlots(IntPtr type, Type impl)
IntPtr slot = Interop.GetThunk(method);
Marshal.WriteIntPtr(type, offset, slot);

seen[name] = 1;
seen.Add(name);
}

impl = impl.BaseType;
}

// Hack: for some objects (Classbase and ExtensionType) we need gc
// support. We need to provide three functions:
// - tp_traverse(object, ...) returns 0
// - tp_clear(object) returns 0
// - tp_is_gc(type) returns non-zero
// In addition, these functions can get called after a domain reload,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Small detail but I think we should say "domain unload". Domain reload is kind of application-specific, it is not something that is part of the C# framework. In other words, it might be too "Unity-specific"

// so they must be implemented in native code.
//
// Py_GetVersion reliably returns non-zero and uses no arguments.
//
// PyAST_Check reads the object and returns zero unless it's a
// python AST node. You could violate this assumption by making a C#
// type that derives from ast.AST -- but it's not clear why you'd do that.
if (seen.Contains("tp_traverse"))
{
var lib = NativeMethods.LoadLibrary(Runtime.PythonDLL);
var zeroslot = NativeMethods.GetProcAddress(lib, "PyAST_Check");
var trueslot = NativeMethods.GetProcAddress(lib, "Py_GetVersion");
Copy link
Collaborator

Choose a reason for hiding this comment

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

What if the procs don't exist? I know it is unlikely, but should you catch an exception of some sort in that case?

Also, I think it would be good to make sure PyAST_Check returns 0 and Py_GetVersion returns non-zero. Is there a way you could add an assertion in Debug perhaps?


var offset = (int)offsetType.GetField("tp_traverse").GetValue(offsetType);
Marshal.WriteIntPtr(type, offset, zeroslot);

offset = (int)offsetType.GetField("tp_clear").GetValue(offsetType);
Marshal.WriteIntPtr(type, offset, zeroslot);

offset = (int)offsetType.GetField("tp_is_gc").GetValue(offsetType);
Marshal.WriteIntPtr(type, offset, trueslot);
}
}


Expand Down