Skip to content

Commit 83e8dd5

Browse files
amos402amos402
authored
Merge pull request #2 from Unity-Technologies/soft-shutdown-review-comments
Soft shutdown review comments
2 parents b203674 + 598cb77 commit 83e8dd5

17 files changed

Lines changed: 250 additions & 179 deletions

‎src/embed_tests/TestDomainReload.cs‎

Lines changed: 66 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,13 @@ public static void DomainReloadAndGC()
7979

8080
#region CrossDomainObject
8181

82-
class CrossDomianObjectStep1 : CrossCaller
82+
class CrossDomainObjectStep1 : CrossCaller
8383
{
8484
public override ValueType Execute(ValueType arg)
8585
{
8686
try
8787
{
88+
// Create a C# user-defined object in Python. Asssing some values.
8889
Type type = typeof(Python.EmbeddingTest.Domain.MyClass);
8990
string code = string.Format(@"
9091
import clr
@@ -133,6 +134,7 @@ public override ValueType Execute(ValueType arg)
133134
{
134135
IntPtr tp = Runtime.Runtime.PyObject_TYPE(handle);
135136
IntPtr tp_clear = Marshal.ReadIntPtr(tp, TypeOffset.tp_clear);
137+
Assert.That(tp_clear, Is.Not.Null);
136138

137139
using (PyObject obj = new PyObject(handle))
138140
{
@@ -164,10 +166,15 @@ public override ValueType Execute(ValueType arg)
164166
}
165167
}
166168

169+
/// <summary>
170+
/// Create a C# custom object in a domain, in python code.
171+
/// Unload the domain, create a new domain.
172+
/// Make sure the C# custom object created in the previous domain has been re-created
173+
/// </summary>
167174
[Test]
168175
public static void CrossDomainObject()
169176
{
170-
RunDomainReloadSteps<CrossDomianObjectStep1, CrossDomainObjectStep2>();
177+
RunDomainReloadSteps<CrossDomainObjectStep1, CrossDomainObjectStep2>();
171178
}
172179

173180
#endregion
@@ -193,19 +200,26 @@ def test_obj_call():
193200
const string name = "test_domain_reload_mod";
194201
using (Py.GIL())
195202
{
203+
// Create a new module
196204
IntPtr module = PyRuntime.PyModule_New(name);
197205
Assert.That(module != IntPtr.Zero);
198206
IntPtr globals = PyRuntime.PyObject_GetAttrString(module, "__dict__");
199207
Assert.That(globals != IntPtr.Zero);
200208
try
201209
{
210+
// import builtins
211+
// module.__dict__[__builtins__] = builtins
202212
int res = PyRuntime.PyDict_SetItemString(globals, "__builtins__",
203213
PyRuntime.PyEval_GetBuiltins());
204214
PythonException.ThrowIfIsNotZero(res);
205215

216+
// Execute the code in the module's scope
206217
PythonEngine.Exec(code, globals);
218+
// import sys
219+
// modules = sys.modules
207220
IntPtr modules = PyRuntime.PyImport_GetModuleDict();
208-
res = PyRuntime.PyDict_SetItemString(modules, name, modules);
221+
// modules[name] = module
222+
res = PyRuntime.PyDict_SetItemString(modules, name, module);
209223
PythonException.ThrowIfIsNotZero(res);
210224
}
211225
catch
@@ -244,6 +258,11 @@ public override ValueType Execute(ValueType arg)
244258

245259

246260
[Test]
261+
/// <summary>
262+
/// Create a new Python module, define a function in it.
263+
/// Unload the domain, load a new one.
264+
/// Make sure the function (and module) still exists.
265+
/// </summary>
247266
public void TestClassReference()
248267
{
249268
RunDomainReloadSteps<ReloadClassRefStep1, ReloadClassRefStep2>();
@@ -261,7 +280,12 @@ void ExecTest()
261280
{
262281
try
263282
{
283+
PythonEngine.Initialize();
264284
var numRef = CreateNumReference();
285+
Assert.True(numRef.IsAlive);
286+
PythonEngine.Shutdown(); // <- "run" 1 ends
287+
PythonEngine.Initialize(); // <- "run" 2 starts
288+
265289
GC.Collect();
266290
GC.WaitForPendingFinalizers(); // <- this will put former `num` into Finalizer queue
267291
Finalizer.Instance.Collect(forceDispose: true);
@@ -302,7 +326,11 @@ void ExecTest()
302326
{
303327
try
304328
{
329+
PythonEngine.Initialize();
305330
var objRef = CreateConcreateObject();
331+
Assert.True(objRef.IsAlive);
332+
PythonEngine.Shutdown(); // <- "run" 1 ends
333+
PythonEngine.Initialize(); // <- "run" 2 starts
306334
GC.Collect();
307335
GC.WaitForPendingFinalizers();
308336
Finalizer.Instance.Collect(forceDispose: true);
@@ -336,25 +364,17 @@ void ErrorHandler(object sender, Finalizer.ErrorArgs e)
336364

337365
private static WeakReference CreateNumReference()
338366
{
339-
PythonEngine.Initialize();
340367
var num = 3216757418.ToPython();
341368
Assert.AreEqual(num.Refcount, 1);
342369
WeakReference numRef = new WeakReference(num, false);
343-
PythonEngine.Shutdown(); // <- "run" 1 ends
344-
PythonEngine.Initialize(); // <- "run" 2 starts
345-
num = null;
346370
return numRef;
347371
}
348372

349373
private static WeakReference CreateConcreateObject()
350374
{
351-
PythonEngine.Initialize();
352375
var obj = new Domain.MyClass().ToPython();
353376
Assert.AreEqual(obj.Refcount, 1);
354377
WeakReference numRef = new WeakReference(obj, false);
355-
PythonEngine.Shutdown();
356-
PythonEngine.Initialize();
357-
obj = null;
358378
return numRef;
359379
}
360380

@@ -380,6 +400,15 @@ public object Call(string methodName, params object[] args)
380400
return method.Invoke(null, args);
381401
}
382402
}
403+
404+
static T CreateInstanceInstanceAndUnwrap<T>(AppDomain domain)
405+
{
406+
Type type = typeof(T);
407+
var theProxy = (T)domain.CreateInstanceAndUnwrap(
408+
type.Assembly.FullName,
409+
type.FullName);
410+
return theProxy;
411+
}
383412

384413
/// <summary>
385414
/// Create a domain, run the assembly in it (the RunPython function),
@@ -392,14 +421,13 @@ static void RunAssemblyAndUnload(string domainName)
392421
AppDomain domain = CreateDomain(domainName);
393422
// Create a Proxy object in the new domain, where we want the
394423
// assembly (and Python .NET) to reside
395-
Type type = typeof(Proxy);
396-
var theProxy = (Proxy)domain.CreateInstanceAndUnwrap(
397-
type.Assembly.FullName,
398-
type.FullName);
424+
var theProxy = CreateInstanceInstanceAndUnwrap<Proxy>(domain);
399425

426+
theProxy.Call("InitPython", ShutdownMode.Soft);
400427
// From now on use the Proxy to call into the new assembly
401428
theProxy.RunPython();
402429

430+
theProxy.Call("ShutdownPython");
403431
Console.WriteLine($"[Program.Main] Before Domain Unload on {domainName}");
404432
AppDomain.Unload(domain);
405433
Console.WriteLine($"[Program.Main] After Domain Unload on {domainName}");
@@ -459,17 +487,13 @@ static void RunDomainReloadSteps<T1, T2>() where T1 : CrossCaller where T2 : Cro
459487
ValueType arg = null;
460488
Type type = typeof(Proxy);
461489
{
462-
AppDomain domain = CreateDomain("test_domain_reload");
490+
AppDomain domain = CreateDomain("test_domain_reload_1");
463491
try
464492
{
465-
var theProxy = (Proxy)domain.CreateInstanceAndUnwrap(
466-
type.Assembly.FullName,
467-
type.FullName);
493+
var theProxy = CreateInstanceInstanceAndUnwrap<Proxy>(domain);
468494
theProxy.Call("InitPython", ShutdownMode.Reload);
469495

470-
var caller = (T1)domain.CreateInstanceAndUnwrap(
471-
typeof(T1).Assembly.FullName,
472-
typeof(T1).FullName);
496+
var caller = CreateInstanceInstanceAndUnwrap<T1>(domain);
473497
arg = caller.Execute(arg);
474498

475499
theProxy.Call("ShutdownPython");
@@ -481,17 +505,13 @@ static void RunDomainReloadSteps<T1, T2>() where T1 : CrossCaller where T2 : Cro
481505
}
482506

483507
{
484-
AppDomain domain = CreateDomain("test_domain_reload");
508+
AppDomain domain = CreateDomain("test_domain_reload_2");
485509
try
486510
{
487-
var theProxy = (Proxy)domain.CreateInstanceAndUnwrap(
488-
type.Assembly.FullName,
489-
type.FullName);
511+
var theProxy = CreateInstanceInstanceAndUnwrap<Proxy>(domain);
490512
theProxy.Call("InitPython", ShutdownMode.Reload);
491513

492-
var caller = (T2)domain.CreateInstanceAndUnwrap(
493-
typeof(T2).Assembly.FullName,
494-
typeof(T2).FullName);
514+
var caller = CreateInstanceInstanceAndUnwrap<T2>(domain);
495515
caller.Execute(arg);
496516
theProxy.Call("ShutdownPythonCompletely");
497517
}
@@ -523,39 +543,26 @@ public static void RunPython()
523543
{
524544
AppDomain.CurrentDomain.DomainUnload += OnDomainUnload;
525545
string name = AppDomain.CurrentDomain.FriendlyName;
526-
Console.WriteLine(string.Format("[{0} in .NET] In PythonRunner.RunPython", name));
527-
var mode = PythonEngine.DefaultShutdownMode;
528-
if (mode == ShutdownMode.Normal)
529-
{
530-
mode = ShutdownMode.Soft;
531-
}
532-
PythonEngine.Initialize(mode: mode);
533-
try
546+
Console.WriteLine("[{0} in .NET] In PythonRunner.RunPython", name);
547+
using (Py.GIL())
534548
{
535-
using (Py.GIL())
549+
try
536550
{
537-
try
538-
{
539-
var pyScript = string.Format("import clr\n"
540-
+ "print('[{0} in python] imported clr')\n"
541-
+ "clr.AddReference('System')\n"
542-
+ "print('[{0} in python] allocated a clr object')\n"
543-
+ "import gc\n"
544-
+ "gc.collect()\n"
545-
+ "print('[{0} in python] collected garbage')\n",
546-
name);
547-
PythonEngine.Exec(pyScript);
548-
}
549-
catch (Exception e)
550-
{
551-
Console.WriteLine(string.Format("[{0} in .NET] Caught exception: {1}", name, e));
552-
throw;
553-
}
551+
var pyScript = string.Format("import clr\n"
552+
+ "print('[{0} in python] imported clr')\n"
553+
+ "clr.AddReference('System')\n"
554+
+ "print('[{0} in python] allocated a clr object')\n"
555+
+ "import gc\n"
556+
+ "gc.collect()\n"
557+
+ "print('[{0} in python] collected garbage')\n",
558+
name);
559+
PythonEngine.Exec(pyScript);
560+
}
561+
catch (Exception e)
562+
{
563+
Console.WriteLine(string.Format("[{0} in .NET] Caught exception: {1}", name, e));
564+
throw;
554565
}
555-
}
556-
finally
557-
{
558-
PythonEngine.BeginAllowThreads();
559566
}
560567
}
561568

‎src/embed_tests/pyinitialize.cs‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,11 @@ public static void StartAndStopTwice()
2424
public static void LoadDefaultArgs()
2525
{
2626
using (new PythonEngine())
27-
using (var argv = new PyList(Runtime.Runtime.PySys_GetObject("argv")))
2827
{
29-
Runtime.Runtime.XIncref(argv.Handle);
30-
Assert.AreNotEqual(0, argv.Length());
28+
using(var argv = new PyList(Runtime.Runtime.PySys_GetObject("argv")))
29+
{
30+
Assert.AreNotEqual(0, argv.Length());
31+
}
3132
}
3233
}
3334

@@ -36,11 +37,12 @@ public static void LoadSpecificArgs()
3637
{
3738
var args = new[] { "test1", "test2" };
3839
using (new PythonEngine(args))
39-
using (var argv = new PyList(Runtime.Runtime.PySys_GetObject("argv")))
4040
{
41-
Runtime.Runtime.XIncref(argv.Handle);
42-
Assert.AreEqual(args[0], argv[0].ToString());
43-
Assert.AreEqual(args[1], argv[1].ToString());
41+
using (var argv = new PyList(Runtime.Runtime.PySys_GetObject("argv")))
42+
{
43+
Assert.AreEqual(args[0], argv[0].ToString());
44+
Assert.AreEqual(args[1], argv[1].ToString());
45+
}
4446
}
4547
}
4648

‎src/runtime/classbase.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -297,15 +297,15 @@ public static void tp_dealloc(IntPtr ob)
297297
public static int tp_clear(IntPtr ob)
298298
{
299299
ManagedType self = GetManagedObject(ob);
300-
if (self.pyHandle != self.tpHandle)
300+
if (!self.IsTypeObject())
301301
{
302302
ClearObjectDict(ob);
303303
}
304304
self.tpHandle = IntPtr.Zero;
305305
return 0;
306306
}
307307

308-
protected override void OnSave(PyObjectSerializeContext context)
308+
protected override void OnSave(InterDomainContext context)
309309
{
310310
base.OnSave(context);
311311
if (pyHandle != tpHandle)
@@ -316,7 +316,7 @@ protected override void OnSave(PyObjectSerializeContext context)
316316
}
317317
}
318318

319-
protected override void OnLoad(PyObjectSerializeContext context)
319+
protected override void OnLoad(InterDomainContext context)
320320
{
321321
base.OnLoad(context);
322322
if (pyHandle != tpHandle)

0 commit comments

Comments
 (0)