From 0444100e78129562791629501bbf9185877d4616 Mon Sep 17 00:00:00 2001 From: Mark Hiner Date: Mon, 14 Dec 2015 14:42:53 -0600 Subject: [PATCH 1/7] JythonScriptLanguage: clean interpreter Uses PhantomReferences to clean up after the interpreter --- .../jython/JythonScriptLanguage.java | 94 ++++++++++++++++++- 1 file changed, 93 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/scijava/plugins/scripting/jython/JythonScriptLanguage.java b/src/main/java/org/scijava/plugins/scripting/jython/JythonScriptLanguage.java index 7cf4fda..236c4cc 100644 --- a/src/main/java/org/scijava/plugins/scripting/jython/JythonScriptLanguage.java +++ b/src/main/java/org/scijava/plugins/scripting/jython/JythonScriptLanguage.java @@ -31,14 +31,23 @@ package org.scijava.plugins.scripting.jython; +import java.lang.ref.PhantomReference; +import java.lang.ref.ReferenceQueue; +import java.util.ArrayList; +import java.util.LinkedList; +import java.util.List; + import javax.script.ScriptEngine; import org.python.core.PyNone; import org.python.core.PyObject; import org.python.core.PyString; +import org.python.util.PythonInterpreter; +import org.scijava.plugin.Parameter; import org.scijava.plugin.Plugin; import org.scijava.script.AdaptedScriptLanguage; import org.scijava.script.ScriptLanguage; +import org.scijava.thread.ThreadService; /** * An adapter of the Jython interpreter to the SciJava scripting interface. @@ -49,6 +58,14 @@ @Plugin(type = ScriptLanguage.class, name = "Python") public class JythonScriptLanguage extends AdaptedScriptLanguage { + private final LinkedList phantomReferences = + new LinkedList(); + private final ReferenceQueue queue = + new ReferenceQueue(); + + @Parameter + private ThreadService threadService; + public JythonScriptLanguage() { super("jython"); } @@ -56,7 +73,51 @@ public JythonScriptLanguage() { @Override public ScriptEngine getScriptEngine() { // TODO: Consider adapting the wrapped ScriptEngineFactory's ScriptEngine. - return new JythonScriptEngine(); + final JythonScriptEngine engine = new JythonScriptEngine(); + + synchronized (phantomReferences) { + phantomReferences.add(new JythonEnginePhantomReference(engine, queue)); + + // If we added references to an empty queue, we need to start + // a new polling thread + if (phantomReferences.size() == 1) { + threadService.run(new Runnable() { + + @Override + public void run() { + boolean done = false; + + // poll the queue + while (!done) { + try { + Thread.sleep(100); + + synchronized (phantomReferences) { + JythonEnginePhantomReference ref = + (JythonEnginePhantomReference) queue.poll(); + + // if we have a ref, clean it up + if (ref != null) { + ref.cleanup(); + phantomReferences.remove(ref); + } + + // Once we're done with our known phantom refs + // we can shut down this thread. + done = phantomReferences.size() == 0; + } + + } + catch (final Exception ex) { + // log exception, continue + } + } + } + }); + + } + } + return engine; } @Override @@ -74,4 +135,35 @@ public Object decode(final Object object) { return object; } + private static class JythonEnginePhantomReference extends + PhantomReference + { + + public PythonInterpreter interpreter; + + public JythonEnginePhantomReference(JythonScriptEngine engine, + ReferenceQueue queue) + { + super(engine, queue); + interpreter = engine.interpreter; + } + + public void cleanup() { + final List scriptLocals = new ArrayList(); + PythonInterpreter interp = interpreter; + if (interp == null) return; + + final PyObject locals = interp.getLocals(); + for (final PyObject item : locals.__iter__().asIterable()) { + final String localVar = item.toString(); + if (!localVar.contains("__name__") && !localVar.contains("__doc__")) { + + scriptLocals.add(item.toString()); + } + } + for (final String string : scriptLocals) { + interp.set(string, null); + } + } + } } From 1f8826d6472abb84cdf2177c438353455a5b96cc Mon Sep 17 00:00:00 2001 From: Mark Hiner Date: Mon, 14 Dec 2015 14:43:21 -0600 Subject: [PATCH 2/7] JythonBindings: don't store engine or module If the JythonScriptEngine or containing ScriptModule is set within the PythonInterpreter, we will get a circular reference chain preventing memory from the PythonInterpreter being reclaimed. To avoid completely discarding these objects, we can keep them in a shallow map to WeakReferences. --- .../scripting/jython/JythonBindings.java | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/scijava/plugins/scripting/jython/JythonBindings.java b/src/main/java/org/scijava/plugins/scripting/jython/JythonBindings.java index 3d6a493..1ed800e 100644 --- a/src/main/java/org/scijava/plugins/scripting/jython/JythonBindings.java +++ b/src/main/java/org/scijava/plugins/scripting/jython/JythonBindings.java @@ -31,8 +31,10 @@ package org.scijava.plugins.scripting.jython; +import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -42,6 +44,7 @@ import org.python.core.PyStringMap; import org.python.util.PythonInterpreter; +import org.scijava.script.ScriptModule; /** * A {@link Bindings} wrapper around Jython's local variables. @@ -52,6 +55,9 @@ public class JythonBindings implements Bindings { protected final PythonInterpreter interpreter; + private Map> shallowMap = + new HashMap>(); + public JythonBindings(final PythonInterpreter interpreter) { this.interpreter = interpreter; } @@ -81,6 +87,10 @@ public boolean containsValue(Object value) { @Override public Object get(Object key) { + if (shallowMap.containsKey(key)) { + return shallowMap.get(key).get(); + } + try { return interpreter.get((String)key); } catch (Error e) { @@ -91,18 +101,28 @@ public Object get(Object key) { @Override public Object put(String key, Object value) { final Object result = get(key); - try { - interpreter.set(key, value); - } catch (Error e) { - // ignore + + if (value instanceof ScriptModule || value instanceof JythonScriptEngine){ + shallowMap.put(key, new WeakReference(value)); + } + else { + try { + interpreter.set(key, value); + } + catch (Error e) { + // ignore + } } + return result; } @Override public Object remove(Object key) { final Object result = get(key); - if (result != null) interpreter.getLocals().__delitem__((String)key); + if (shallowMap.containsKey(key)) shallowMap.remove(key); + else if (result != null) interpreter.getLocals().__delitem__((String)key); + return result; } From 70e0c18c6461e784135534722e7699eaaa48cb61 Mon Sep 17 00:00:00 2001 From: Mark Hiner Date: Tue, 15 Dec 2015 08:52:28 -0600 Subject: [PATCH 3/7] Document PhantomReference solution Add extensive commenting to explain why PhantomReferences are needed to solve the memory leak identified in this implementation of the Python script language. Fixes http://fiji.sc/bugzilla/show_bug.cgi?id=1203 --- .../scripting/jython/JythonBindings.java | 16 +++++++ .../jython/JythonScriptLanguage.java | 43 +++++++++++++++++-- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/scijava/plugins/scripting/jython/JythonBindings.java b/src/main/java/org/scijava/plugins/scripting/jython/JythonBindings.java index 1ed800e..146a650 100644 --- a/src/main/java/org/scijava/plugins/scripting/jython/JythonBindings.java +++ b/src/main/java/org/scijava/plugins/scripting/jython/JythonBindings.java @@ -55,6 +55,22 @@ public class JythonBindings implements Bindings { protected final PythonInterpreter interpreter; + /* + * NB: In our JythonScriptLanguage we explain the need for cleaning + * up after a PythonInterpreter and declare the scope of a + * Python environment to be equal to the lifetime of its parent + * JythonScriptEngine. + * As triggering our cleaning method involves PhantomReferences + * we must ensure that JythonScriptEngines can actually be + * garbage collected when it is no longer in use. + * To do this, we have to prevent JythonScriptEngines from + * being passed to the PythonInterpreter - which would then + * create a hard reference to the ScriptEngine through the + * static PySystemState. + * Similarly we do not want to pass ScriptModules to the + * PythonInterpreter, as the ScriptModule has a hard + * reference to its ScriptEngine. + */ private Map> shallowMap = new HashMap>(); diff --git a/src/main/java/org/scijava/plugins/scripting/jython/JythonScriptLanguage.java b/src/main/java/org/scijava/plugins/scripting/jython/JythonScriptLanguage.java index 236c4cc..4d8bde1 100644 --- a/src/main/java/org/scijava/plugins/scripting/jython/JythonScriptLanguage.java +++ b/src/main/java/org/scijava/plugins/scripting/jython/JythonScriptLanguage.java @@ -53,11 +53,16 @@ * An adapter of the Jython interpreter to the SciJava scripting interface. * * @author Johannes Schindelin + * @author Mark Hiner * @see ScriptEngine */ @Plugin(type = ScriptLanguage.class, name = "Python") public class JythonScriptLanguage extends AdaptedScriptLanguage { + // List of PhantomReferences and corresponding ReferenceQueue to + // facilitate proper PhantomReference use. + // See http://resources.ej-technologies.com/jprofiler/help/doc/index.html + private final LinkedList phantomReferences = new LinkedList(); private final ReferenceQueue queue = @@ -76,10 +81,22 @@ public ScriptEngine getScriptEngine() { final JythonScriptEngine engine = new JythonScriptEngine(); synchronized (phantomReferences) { + // NB: This phantom reference is used to clean up any local variables + // created by evaluation of scripts via this ScriptEngine. We need + // to use PhantomReferences because the "scope" of a script extends + // beyond its eval method - a consumer may still want to inspect + // the state of variables after evaluation. + // By using PhantomReferences we are saying that the scope of + // script evaluation equals the lifetime of the ScriptEngine. phantomReferences.add(new JythonEnginePhantomReference(engine, queue)); - // If we added references to an empty queue, we need to start - // a new polling thread + // NB: The use of PhantomReferences requires a paired polling thread. + // We poll instead of blocking for input to avoid leaving lingering + // threads that would need to be interrupted - instead only starting + // threads running when there is actually a PhantomReference that + // will eventually be enqueued. + // Here we check if there is already a polling thread in operation - + // if not, we start a new thread. if (phantomReferences.size() == 1) { threadService.run(new Runnable() { @@ -93,6 +110,7 @@ public void run() { Thread.sleep(100); synchronized (phantomReferences) { + // poll the queue JythonEnginePhantomReference ref = (JythonEnginePhantomReference) queue.poll(); @@ -103,7 +121,7 @@ public void run() { } // Once we're done with our known phantom refs - // we can shut down this thread. + // we can let this thread shut down done = phantomReferences.size() == 0; } @@ -135,6 +153,10 @@ public Object decode(final Object object) { return object; } + /** + * Helper class to clean up {@link PythonInterpreter} local variables when a + * parent {@link JythonScriptEngine} leaves scope. + */ private static class JythonEnginePhantomReference extends PhantomReference { @@ -153,14 +175,27 @@ public void cleanup() { PythonInterpreter interp = interpreter; if (interp == null) return; + // NB: This method for cleaning up local variables was taken from: + // http://python.6.x6.nabble.com/Cleaning-up-PythonInterpreter-object-td1777184.html + // Because Python is an interpreted language, when a Python script creates new + // variables they stick around in a static org.python.core.PySystemState variable + // (defaultSystemState) the org.python.core.Py class. + // Thus an implicit static state is created by script evaluation, so we must manually + // clean up local variables known to the interpreter when the scope of an executed + // script is over. + // See original bug report for the memory leak that prompted this solution: + // http://fiji.sc/bugzilla/show_bug.cgi?id=1203 final PyObject locals = interp.getLocals(); for (final PyObject item : locals.__iter__().asIterable()) { final String localVar = item.toString(); + // Ignore __name__ and __doc__ variables, which are special and known not + // to be local variables created by evaluation. if (!localVar.contains("__name__") && !localVar.contains("__doc__")) { - + // Build list of variables to clean scriptLocals.add(item.toString()); } } + // Null out local variables for (final String string : scriptLocals) { interp.set(string, null); } From 7647ab13f8f16074ed24dad28a659fa1ca28b991 Mon Sep 17 00:00:00 2001 From: Mark Hiner Date: Tue, 15 Dec 2015 08:55:01 -0600 Subject: [PATCH 4/7] Jython polling thread: log errors Pass a LogService to the thread polling the PhantomReference queue so that exceptions can be logged. --- .../plugins/scripting/jython/JythonScriptLanguage.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/java/org/scijava/plugins/scripting/jython/JythonScriptLanguage.java b/src/main/java/org/scijava/plugins/scripting/jython/JythonScriptLanguage.java index 4d8bde1..f315a9b 100644 --- a/src/main/java/org/scijava/plugins/scripting/jython/JythonScriptLanguage.java +++ b/src/main/java/org/scijava/plugins/scripting/jython/JythonScriptLanguage.java @@ -43,6 +43,7 @@ import org.python.core.PyObject; import org.python.core.PyString; import org.python.util.PythonInterpreter; +import org.scijava.log.LogService; import org.scijava.plugin.Parameter; import org.scijava.plugin.Plugin; import org.scijava.script.AdaptedScriptLanguage; @@ -71,6 +72,9 @@ public class JythonScriptLanguage extends AdaptedScriptLanguage { @Parameter private ThreadService threadService; + @Parameter + private LogService logService; + public JythonScriptLanguage() { super("jython"); } @@ -79,6 +83,7 @@ public JythonScriptLanguage() { public ScriptEngine getScriptEngine() { // TODO: Consider adapting the wrapped ScriptEngineFactory's ScriptEngine. final JythonScriptEngine engine = new JythonScriptEngine(); + final LogService ls = logService; synchronized (phantomReferences) { // NB: This phantom reference is used to clean up any local variables @@ -128,6 +133,7 @@ public void run() { } catch (final Exception ex) { // log exception, continue + ls.error(ex); } } } From dacf3e98c590d038b6d824196267884976f80c1b Mon Sep 17 00:00:00 2001 From: Curtis Rueden Date: Tue, 15 Dec 2015 09:06:13 -0600 Subject: [PATCH 5/7] Move reference cleanup logic to its own class This isolates and encapsulates the cleanup logic, so the JythonScriptLanguage itself remains easy to understand. --- .../jython/JythonReferenceCleaner.java | 172 ++++++++++++++++++ .../jython/JythonScriptLanguage.java | 123 +------------ 2 files changed, 174 insertions(+), 121 deletions(-) create mode 100644 src/main/java/org/scijava/plugins/scripting/jython/JythonReferenceCleaner.java diff --git a/src/main/java/org/scijava/plugins/scripting/jython/JythonReferenceCleaner.java b/src/main/java/org/scijava/plugins/scripting/jython/JythonReferenceCleaner.java new file mode 100644 index 0000000..33d4e01 --- /dev/null +++ b/src/main/java/org/scijava/plugins/scripting/jython/JythonReferenceCleaner.java @@ -0,0 +1,172 @@ +/* + * #%L + * JSR-223-compliant Jython scripting language plugin. + * %% + * Copyright (C) 2008 - 2015 Board of Regents of the University of + * Wisconsin-Madison, Broad Institute of MIT and Harvard, and Max Planck + * Institute of Molecular Cell Biology and Genetics. + * %% + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * #L% + */ + +package org.scijava.plugins.scripting.jython; + +import java.lang.ref.PhantomReference; +import java.lang.ref.ReferenceQueue; +import java.util.ArrayList; +import java.util.LinkedList; +import java.util.List; + +import org.python.core.PyObject; +import org.python.util.PythonInterpreter; +import org.scijava.log.LogService; +import org.scijava.thread.ThreadService; + +/** + * A helper class for purging dangling references within a Jython interpreter + * after it executes a script. + * + * @author Mark Hiner + */ +public class JythonReferenceCleaner { + + // List of PhantomReferences and corresponding ReferenceQueue to + // facilitate proper PhantomReference use. + // See http://resources.ej-technologies.com/jprofiler/help/doc/index.html + + private final LinkedList phantomReferences = + new LinkedList(); + private final ReferenceQueue queue = + new ReferenceQueue(); + + /** Queues the future cleanup operation on a separate thread, */ + public synchronized void queueCleanup(final JythonScriptEngine engine, + final ThreadService threadService, final LogService log) + { + // NB: This phantom reference is used to clean up any local variables + // created by evaluation of scripts via this ScriptEngine. We need + // to use PhantomReferences because the "scope" of a script extends + // beyond its eval method - a consumer may still want to inspect + // the state of variables after evaluation. + // By using PhantomReferences we are saying that the scope of + // script evaluation equals the lifetime of the ScriptEngine. + phantomReferences.add(new JythonEnginePhantomReference(engine, queue)); + + // NB: The use of PhantomReferences requires a paired polling thread. + // We poll instead of blocking for input to avoid leaving lingering + // threads that would need to be interrupted - instead only starting + // threads running when there is actually a PhantomReference that + // will eventually be enqueued. + // Here we check if there is already a polling thread in operation - + // if not, we start a new thread. + if (phantomReferences.size() == 1) { + threadService.run(new Runnable() { + + @Override + public void run() { + boolean done = false; + + // poll the queue + while (!done) { + try { + Thread.sleep(100); + + synchronized (JythonReferenceCleaner.this) { + // poll the queue + JythonEnginePhantomReference ref = + (JythonEnginePhantomReference) queue.poll(); + + // if we have a ref, clean it up + if (ref != null) { + ref.cleanup(); + phantomReferences.remove(ref); + } + + // Once we're done with our known phantom refs + // we can let this thread shut down + done = phantomReferences.size() == 0; + } + + } + catch (final Exception ex) { + // log exception, continue + log.error(ex); + } + } + } + }); + } + } + + // -- Helper classes -- + + /** + * Helper class to clean up {@link PythonInterpreter} local variables when a + * parent {@link JythonScriptEngine} leaves scope. + */ + private static class JythonEnginePhantomReference extends + PhantomReference + { + + public PythonInterpreter interpreter; + + public JythonEnginePhantomReference(JythonScriptEngine engine, + ReferenceQueue queue) + { + super(engine, queue); + interpreter = engine.interpreter; + } + + public void cleanup() { + final List scriptLocals = new ArrayList(); + PythonInterpreter interp = interpreter; + if (interp == null) return; + + // NB: This method for cleaning up local variables was taken from: + // http://python.6.x6.nabble.com/Cleaning-up-PythonInterpreter-object-td1777184.html + // Because Python is an interpreted language, when a Python script creates new + // variables they stick around in a static org.python.core.PySystemState variable + // (defaultSystemState) the org.python.core.Py class. + // Thus an implicit static state is created by script evaluation, so we must manually + // clean up local variables known to the interpreter when the scope of an executed + // script is over. + // See original bug report for the memory leak that prompted this solution: + // http://fiji.sc/bugzilla/show_bug.cgi?id=1203 + final PyObject locals = interp.getLocals(); + for (final PyObject item : locals.__iter__().asIterable()) { + final String localVar = item.toString(); + // Ignore __name__ and __doc__ variables, which are special and known not + // to be local variables created by evaluation. + if (!localVar.contains("__name__") && !localVar.contains("__doc__")) { + // Build list of variables to clean + scriptLocals.add(item.toString()); + } + } + // Null out local variables + for (final String string : scriptLocals) { + interp.set(string, null); + } + } + } + +} diff --git a/src/main/java/org/scijava/plugins/scripting/jython/JythonScriptLanguage.java b/src/main/java/org/scijava/plugins/scripting/jython/JythonScriptLanguage.java index f315a9b..5912d02 100644 --- a/src/main/java/org/scijava/plugins/scripting/jython/JythonScriptLanguage.java +++ b/src/main/java/org/scijava/plugins/scripting/jython/JythonScriptLanguage.java @@ -31,18 +31,11 @@ package org.scijava.plugins.scripting.jython; -import java.lang.ref.PhantomReference; -import java.lang.ref.ReferenceQueue; -import java.util.ArrayList; -import java.util.LinkedList; -import java.util.List; - import javax.script.ScriptEngine; import org.python.core.PyNone; import org.python.core.PyObject; import org.python.core.PyString; -import org.python.util.PythonInterpreter; import org.scijava.log.LogService; import org.scijava.plugin.Parameter; import org.scijava.plugin.Plugin; @@ -60,14 +53,7 @@ @Plugin(type = ScriptLanguage.class, name = "Python") public class JythonScriptLanguage extends AdaptedScriptLanguage { - // List of PhantomReferences and corresponding ReferenceQueue to - // facilitate proper PhantomReference use. - // See http://resources.ej-technologies.com/jprofiler/help/doc/index.html - - private final LinkedList phantomReferences = - new LinkedList(); - private final ReferenceQueue queue = - new ReferenceQueue(); + private JythonReferenceCleaner cleaner = new JythonReferenceCleaner(); @Parameter private ThreadService threadService; @@ -83,64 +69,7 @@ public JythonScriptLanguage() { public ScriptEngine getScriptEngine() { // TODO: Consider adapting the wrapped ScriptEngineFactory's ScriptEngine. final JythonScriptEngine engine = new JythonScriptEngine(); - final LogService ls = logService; - - synchronized (phantomReferences) { - // NB: This phantom reference is used to clean up any local variables - // created by evaluation of scripts via this ScriptEngine. We need - // to use PhantomReferences because the "scope" of a script extends - // beyond its eval method - a consumer may still want to inspect - // the state of variables after evaluation. - // By using PhantomReferences we are saying that the scope of - // script evaluation equals the lifetime of the ScriptEngine. - phantomReferences.add(new JythonEnginePhantomReference(engine, queue)); - - // NB: The use of PhantomReferences requires a paired polling thread. - // We poll instead of blocking for input to avoid leaving lingering - // threads that would need to be interrupted - instead only starting - // threads running when there is actually a PhantomReference that - // will eventually be enqueued. - // Here we check if there is already a polling thread in operation - - // if not, we start a new thread. - if (phantomReferences.size() == 1) { - threadService.run(new Runnable() { - - @Override - public void run() { - boolean done = false; - - // poll the queue - while (!done) { - try { - Thread.sleep(100); - - synchronized (phantomReferences) { - // poll the queue - JythonEnginePhantomReference ref = - (JythonEnginePhantomReference) queue.poll(); - - // if we have a ref, clean it up - if (ref != null) { - ref.cleanup(); - phantomReferences.remove(ref); - } - - // Once we're done with our known phantom refs - // we can let this thread shut down - done = phantomReferences.size() == 0; - } - - } - catch (final Exception ex) { - // log exception, continue - ls.error(ex); - } - } - } - }); - - } - } + cleaner.queueCleanup(engine, threadService, logService); return engine; } @@ -159,52 +88,4 @@ public Object decode(final Object object) { return object; } - /** - * Helper class to clean up {@link PythonInterpreter} local variables when a - * parent {@link JythonScriptEngine} leaves scope. - */ - private static class JythonEnginePhantomReference extends - PhantomReference - { - - public PythonInterpreter interpreter; - - public JythonEnginePhantomReference(JythonScriptEngine engine, - ReferenceQueue queue) - { - super(engine, queue); - interpreter = engine.interpreter; - } - - public void cleanup() { - final List scriptLocals = new ArrayList(); - PythonInterpreter interp = interpreter; - if (interp == null) return; - - // NB: This method for cleaning up local variables was taken from: - // http://python.6.x6.nabble.com/Cleaning-up-PythonInterpreter-object-td1777184.html - // Because Python is an interpreted language, when a Python script creates new - // variables they stick around in a static org.python.core.PySystemState variable - // (defaultSystemState) the org.python.core.Py class. - // Thus an implicit static state is created by script evaluation, so we must manually - // clean up local variables known to the interpreter when the scope of an executed - // script is over. - // See original bug report for the memory leak that prompted this solution: - // http://fiji.sc/bugzilla/show_bug.cgi?id=1203 - final PyObject locals = interp.getLocals(); - for (final PyObject item : locals.__iter__().asIterable()) { - final String localVar = item.toString(); - // Ignore __name__ and __doc__ variables, which are special and known not - // to be local variables created by evaluation. - if (!localVar.contains("__name__") && !localVar.contains("__doc__")) { - // Build list of variables to clean - scriptLocals.add(item.toString()); - } - } - // Null out local variables - for (final String string : scriptLocals) { - interp.set(string, null); - } - } - } } From 378be657e4919be97a687b00b9fd6698aa106de5 Mon Sep 17 00:00:00 2001 From: Curtis Rueden Date: Tue, 15 Dec 2015 09:09:10 -0600 Subject: [PATCH 6/7] JythonReferenceCleaner: reformat comments This avoids them breaking the 80 character line limit, and makes them a little nicer to read (IMHO). --- .../jython/JythonReferenceCleaner.java | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/scijava/plugins/scripting/jython/JythonReferenceCleaner.java b/src/main/java/org/scijava/plugins/scripting/jython/JythonReferenceCleaner.java index 33d4e01..2f74836 100644 --- a/src/main/java/org/scijava/plugins/scripting/jython/JythonReferenceCleaner.java +++ b/src/main/java/org/scijava/plugins/scripting/jython/JythonReferenceCleaner.java @@ -68,7 +68,8 @@ public synchronized void queueCleanup(final JythonScriptEngine engine, // to use PhantomReferences because the "scope" of a script extends // beyond its eval method - a consumer may still want to inspect // the state of variables after evaluation. - // By using PhantomReferences we are saying that the scope of + // + // By using PhantomReferences we are saying that the scope of // script evaluation equals the lifetime of the ScriptEngine. phantomReferences.add(new JythonEnginePhantomReference(engine, queue)); @@ -77,6 +78,7 @@ public synchronized void queueCleanup(final JythonScriptEngine engine, // threads that would need to be interrupted - instead only starting // threads running when there is actually a PhantomReference that // will eventually be enqueued. + // // Here we check if there is already a polling thread in operation - // if not, we start a new thread. if (phantomReferences.size() == 1) { @@ -144,19 +146,24 @@ public void cleanup() { // NB: This method for cleaning up local variables was taken from: // http://python.6.x6.nabble.com/Cleaning-up-PythonInterpreter-object-td1777184.html - // Because Python is an interpreted language, when a Python script creates new - // variables they stick around in a static org.python.core.PySystemState variable - // (defaultSystemState) the org.python.core.Py class. - // Thus an implicit static state is created by script evaluation, so we must manually - // clean up local variables known to the interpreter when the scope of an executed - // script is over. - // See original bug report for the memory leak that prompted this solution: + // + // Because Python is an interpreted language, when a Python script + // creates new variables they stick around in a static + // org.python.core.PySystemState variable (defaultSystemState) + // the org.python.core.Py class. + // + // Thus an implicit static state is created by script evaluation, + // so we must manually clean up local variables known to the + // interpreter when the scope of an executed script is over. + // + // See original bug report for the leak that prompted this solution: // http://fiji.sc/bugzilla/show_bug.cgi?id=1203 + final PyObject locals = interp.getLocals(); for (final PyObject item : locals.__iter__().asIterable()) { final String localVar = item.toString(); - // Ignore __name__ and __doc__ variables, which are special and known not - // to be local variables created by evaluation. + // Ignore __name__ and __doc__ variables, which are special + // and known not to be local variables created by evaluation. if (!localVar.contains("__name__") && !localVar.contains("__doc__")) { // Build list of variables to clean scriptLocals.add(item.toString()); From 98fa04174681d7d74dd50ebe0ad4ecb04a3984cb Mon Sep 17 00:00:00 2001 From: Curtis Rueden Date: Tue, 15 Dec 2015 09:10:49 -0600 Subject: [PATCH 7/7] JythonReferenceCleaner: add missing final keywords --- .../scripting/jython/JythonReferenceCleaner.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/scijava/plugins/scripting/jython/JythonReferenceCleaner.java b/src/main/java/org/scijava/plugins/scripting/jython/JythonReferenceCleaner.java index 2f74836..f73a721 100644 --- a/src/main/java/org/scijava/plugins/scripting/jython/JythonReferenceCleaner.java +++ b/src/main/java/org/scijava/plugins/scripting/jython/JythonReferenceCleaner.java @@ -95,8 +95,8 @@ public void run() { synchronized (JythonReferenceCleaner.this) { // poll the queue - JythonEnginePhantomReference ref = - (JythonEnginePhantomReference) queue.poll(); + final JythonEnginePhantomReference ref = + (JythonEnginePhantomReference) queue.poll(); // if we have a ref, clean it up if (ref != null) { @@ -132,8 +132,8 @@ private static class JythonEnginePhantomReference extends public PythonInterpreter interpreter; - public JythonEnginePhantomReference(JythonScriptEngine engine, - ReferenceQueue queue) + public JythonEnginePhantomReference(final JythonScriptEngine engine, + final ReferenceQueue queue) { super(engine, queue); interpreter = engine.interpreter; @@ -141,7 +141,7 @@ public JythonEnginePhantomReference(JythonScriptEngine engine, public void cleanup() { final List scriptLocals = new ArrayList(); - PythonInterpreter interp = interpreter; + final PythonInterpreter interp = interpreter; if (interp == null) return; // NB: This method for cleaning up local variables was taken from: