Skip to content

Commit

Permalink
Clean up globals defined in Python source, make sure their __del__ is…
Browse files Browse the repository at this point in the history
… called
  • Loading branch information
jameslan committed Oct 18, 2020
1 parent 58bebdf commit 54a6742
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 15 deletions.
33 changes: 22 additions & 11 deletions Src/IronPython/Runtime/PythonContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ public PythonContext(ScriptDomainManager/*!*/ manager, IDictionary<string, objec
try {
HookAssemblyResolve();
} catch (System.Security.SecurityException) {
// We may not have SecurityPermissionFlag.ControlAppDomain.
// We may not have SecurityPermissionFlag.ControlAppDomain.
// If so, we will not look up sys.path for module loads
}
}
Expand Down Expand Up @@ -1238,7 +1238,7 @@ private void UnhookAssemblyResolve() {
try {
AppDomain.CurrentDomain.AssemblyResolve -= _resolveHolder.AssemblyResolveEvent;
} catch (System.Security.SecurityException) {
// We may not have SecurityPermissionFlag.ControlAppDomain.
// We may not have SecurityPermissionFlag.ControlAppDomain.
// If so, we will not look up sys.path for module loads
}
}
Expand Down Expand Up @@ -1295,6 +1295,17 @@ public override void Shutdown() {
}
#endif
}

// clean up globals from modules
foreach (var module in SystemStateModules.Values) {
if (module is PythonModule pyModule) {
pyModule.Cleanup(SharedContext);
}
}

// allow finalizers to run before shutdown
GC.Collect();
GC.WaitForPendingFinalizers();
}

// TODO: ExceptionFormatter service
Expand Down Expand Up @@ -1569,7 +1580,7 @@ public override TService GetService<TService>(params object[] args) {

/// <summary>
/// Returns (and creates if necessary) the PythonService that is associated with this PythonContext.
///
///
/// The PythonService is used for providing remoted convenience helpers for the DLR hosting APIs.
/// </summary>
internal Hosting.PythonService GetPythonService(Microsoft.Scripting.Hosting.ScriptEngine engine) {
Expand Down Expand Up @@ -1678,7 +1689,7 @@ private Dictionary<string, Type> CreateBuiltinTable() {

if (Environment.OSVersion.Platform == PlatformID.Unix) {
// we make our nt package show up as a posix package
// on unix platforms. Because we build on top of the
// on unix platforms. Because we build on top of the
// CLI for all file operations we should be good from
// there, but modules that check for the presence of
// names (e.g. os) will do the right thing.
Expand Down Expand Up @@ -2529,8 +2540,8 @@ private CallSite<Func<CallSite, object, T>> MakeConvertSite<T>(ConversionResultK

/// <summary>
/// Invokes the specified operation on the provided arguments and returns the new resulting value.
///
/// operation is usually a value from StandardOperators (standard CLR/DLR operator) or
///
/// operation is usually a value from StandardOperators (standard CLR/DLR operator) or
/// OperatorStrings (a Python specific operator)
/// </summary>
internal object Operation(PythonOperationKind operation, object self, object other) {
Expand Down Expand Up @@ -2987,13 +2998,13 @@ internal CultureInfo NumericCulture {
/// <summary>
/// Sets the current command dispatcher for the Python command line. The previous dispatcher
/// is returned. Null can be passed to remove the current command dispatcher.
///
///
/// The command dispatcher will be called with a delegate to be executed. The command dispatcher
/// should invoke the target delegate in the desired context.
///
///
/// A common use for this is to enable running all REPL commands on the UI thread while the REPL
/// continues to run on a non-UI thread.
///
///
/// The ipy.exe REPL will call into PythonContext.DispatchCommand to dispatch each execution to
/// the correct thread. Other REPLs can do the same to support this functionality as well.
/// </summary>
Expand Down Expand Up @@ -3094,8 +3105,8 @@ public int Compare(object x, object y) {
/// <summary>
/// Gets a function which can be used for comparing two values using the normal
/// Python semantics.
///
/// If type is null then a generic comparison function is returned. If type is
///
/// If type is null then a generic comparison function is returned. If type is
/// not null a comparison function is returned that's used for just that type.
/// </summary>
internal IComparer GetComparer(Type type) {
Expand Down
34 changes: 30 additions & 4 deletions Src/IronPython/Runtime/PythonModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.Dynamic;
using System.Linq;
using System.Runtime.CompilerServices;
using System.Threading;

Expand All @@ -30,6 +31,7 @@ namespace IronPython.Runtime {
public class PythonModule : IDynamicMetaObjectProvider, IPythonMembersList {
private readonly PythonDictionary _dict;
private Scope _scope;
private bool _cleanedUp = false;

public PythonModule() {
_dict = new PythonDictionary();
Expand All @@ -49,7 +51,7 @@ internal PythonModule(PythonContext context, Scope scope) {

/// <summary>
/// Creates a new PythonModule with the specified dictionary.
///
///
/// Used for creating modules for builtin modules which don't have any code associated with them.
/// </summary>
internal PythonModule(PythonDictionary dict) {
Expand Down Expand Up @@ -253,7 +255,7 @@ public DynamicMetaObject GetMember(PythonGetMemberBinder member, DynamicMetaObje

private DynamicMetaObject GetMemberWorker(DynamicMetaObjectBinder binder, DynamicMetaObject codeContext) {
string name = GetGetMemberName(binder);
var tmp = Expression.Variable(typeof(object), "res");
var tmp = Expression.Variable(typeof(object), "res");

return new DynamicMetaObject(
Expression.Block(
Expand Down Expand Up @@ -355,7 +357,7 @@ IList<string> IMembersList.GetMemberNames() {
}

#endregion

internal class DebugProxy {
private readonly PythonModule _module;

Expand All @@ -375,6 +377,30 @@ public List<ObjectDebugView> Members {
}
}


/// <summary>
/// Cleanup globals so that they could be garbage collected.
/// Note that it cleans python sourced modules only,
/// because C# modules are more fundamental and their globals may be required when other python object's finalizer is executing.
/// </summary>
public void Cleanup(CodeContext context) {
if (_cleanedUp) {
return;
}

_cleanedUp = true;

if (!_dict.ContainsKey("__file__")) {
return; // not from Python source, skip clean up
}

foreach (var key in _dict.Keys.ToList()) {
var obj = _dict[key];
if (obj is PythonModule module) {
module.Cleanup(context);
} else if (key is string name) {
__delattr__(context, name);
}
}
}
}
}
41 changes: 41 additions & 0 deletions Tests/test_regressions.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def test_cp1234(): ...
import os
import sys
import unittest
from subprocess import check_output

from iptest import IronPythonTestCase, is_cli, is_mono, is_netcoreapp, is_posix, run_test, skipUnlessIronPython, stdout_trapper

Expand Down Expand Up @@ -1467,4 +1468,44 @@ def get_class_class(cls):
self.assertEqual(o.get_self_class(), o.get_class())
self.assertEqual(o.get_class(), o.get_class_class())

@unittest.skipIf(is_mono, 'https://github.com/IronLanguages/ironpython3/issues/937')
def test_ipy3_gh985(self):
"""https://github.com/IronLanguages/ironpython3/issues/985"""
code = """
import sys
class Foo:
def __init__(self, name):
self.name = name
def __del__(self):
sys.stdout.write(self.name + '\\n')
def func():
foo = Foo('Foo 1')
bar = Foo('Foo 2')
del bar
func()
foo = Foo('Foo 3')
foo = Foo('Foo 4')
"""

test_script_name = os.path.join(self.test_dir, 'ipy3_gh985.py')
f = open(test_script_name, 'w')
try:
f.write(code)
f.close()

output = check_output([sys.executable, test_script_name]).decode(sys.stdout.encoding).strip()
self.assertEqual(set(output.split(os.linesep)), {'Foo 1', 'Foo 2', 'Foo 3', 'Foo 4'})
finally:
os.unlink(test_script_name)


run_test(__name__)

0 comments on commit 54a6742

Please sign in to comment.