Skip to content

Commit dacf3e9

Browse files
ctruedenctrueden
committed
Move reference cleanup logic to its own class
This isolates and encapsulates the cleanup logic, so the JythonScriptLanguage itself remains easy to understand.
1 parent 7647ab1 commit dacf3e9

File tree

2 files changed

+174
-121
lines changed

2 files changed

+174
-121
lines changed

‎src/main/java/org/scijava/plugins/scripting/jython/JythonReferenceCleaner.java‎

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
/*
2+
* #%L
3+
* JSR-223-compliant Jython scripting language plugin.
4+
* %%
5+
* Copyright (C) 2008 - 2015 Board of Regents of the University of
6+
* Wisconsin-Madison, Broad Institute of MIT and Harvard, and Max Planck
7+
* Institute of Molecular Cell Biology and Genetics.
8+
* %%
9+
* Redistribution and use in source and binary forms, with or without
10+
* modification, are permitted provided that the following conditions are met:
11+
*
12+
* 1. Redistributions of source code must retain the above copyright notice,
13+
* this list of conditions and the following disclaimer.
14+
* 2. Redistributions in binary form must reproduce the above copyright notice,
15+
* this list of conditions and the following disclaimer in the documentation
16+
* and/or other materials provided with the distribution.
17+
*
18+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
19+
* AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
20+
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
21+
* ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE
22+
* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
23+
* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
24+
* SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
25+
* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
26+
* CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
27+
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
28+
* POSSIBILITY OF SUCH DAMAGE.
29+
* #L%
30+
*/
31+
32+
package org.scijava.plugins.scripting.jython;
33+
34+
import java.lang.ref.PhantomReference;
35+
import java.lang.ref.ReferenceQueue;
36+
import java.util.ArrayList;
37+
import java.util.LinkedList;
38+
import java.util.List;
39+
40+
import org.python.core.PyObject;
41+
import org.python.util.PythonInterpreter;
42+
import org.scijava.log.LogService;
43+
import org.scijava.thread.ThreadService;
44+
45+
/**
46+
* A helper class for purging dangling references within a Jython interpreter
47+
* after it executes a script.
48+
*
49+
* @author Mark Hiner
50+
*/
51+
public class JythonReferenceCleaner {
52+
53+
// List of PhantomReferences and corresponding ReferenceQueue to
54+
// facilitate proper PhantomReference use.
55+
// See http://resources.ej-technologies.com/jprofiler/help/doc/index.html
56+
57+
private final LinkedList<JythonEnginePhantomReference> phantomReferences =
58+
new LinkedList<JythonEnginePhantomReference>();
59+
private final ReferenceQueue<JythonScriptEngine> queue =
60+
new ReferenceQueue<JythonScriptEngine>();
61+
62+
/** Queues the future cleanup operation on a separate thread, */
63+
public synchronized void queueCleanup(final JythonScriptEngine engine,
64+
final ThreadService threadService, final LogService log)
65+
{
66+
// NB: This phantom reference is used to clean up any local variables
67+
// created by evaluation of scripts via this ScriptEngine. We need
68+
// to use PhantomReferences because the "scope" of a script extends
69+
// beyond its eval method - a consumer may still want to inspect
70+
// the state of variables after evaluation.
71+
// By using PhantomReferences we are saying that the scope of
72+
// script evaluation equals the lifetime of the ScriptEngine.
73+
phantomReferences.add(new JythonEnginePhantomReference(engine, queue));
74+
75+
// NB: The use of PhantomReferences requires a paired polling thread.
76+
// We poll instead of blocking for input to avoid leaving lingering
77+
// threads that would need to be interrupted - instead only starting
78+
// threads running when there is actually a PhantomReference that
79+
// will eventually be enqueued.
80+
// Here we check if there is already a polling thread in operation -
81+
// if not, we start a new thread.
82+
if (phantomReferences.size() == 1) {
83+
threadService.run(new Runnable() {
84+
85+
@Override
86+
public void run() {
87+
boolean done = false;
88+
89+
// poll the queue
90+
while (!done) {
91+
try {
92+
Thread.sleep(100);
93+
94+
synchronized (JythonReferenceCleaner.this) {
95+
// poll the queue
96+
JythonEnginePhantomReference ref =
97+
(JythonEnginePhantomReference) queue.poll();
98+
99+
// if we have a ref, clean it up
100+
if (ref != null) {
101+
ref.cleanup();
102+
phantomReferences.remove(ref);
103+
}
104+
105+
// Once we're done with our known phantom refs
106+
// we can let this thread shut down
107+
done = phantomReferences.size() == 0;
108+
}
109+
110+
}
111+
catch (final Exception ex) {
112+
// log exception, continue
113+
log.error(ex);
114+
}
115+
}
116+
}
117+
});
118+
}
119+
}
120+
121+
// -- Helper classes --
122+
123+
/**
124+
* Helper class to clean up {@link PythonInterpreter} local variables when a
125+
* parent {@link JythonScriptEngine} leaves scope.
126+
*/
127+
private static class JythonEnginePhantomReference extends
128+
PhantomReference<JythonScriptEngine>
129+
{
130+
131+
public PythonInterpreter interpreter;
132+
133+
public JythonEnginePhantomReference(JythonScriptEngine engine,
134+
ReferenceQueue<JythonScriptEngine> queue)
135+
{
136+
super(engine, queue);
137+
interpreter = engine.interpreter;
138+
}
139+
140+
public void cleanup() {
141+
final List<String> scriptLocals = new ArrayList<String>();
142+
PythonInterpreter interp = interpreter;
143+
if (interp == null) return;
144+
145+
// NB: This method for cleaning up local variables was taken from:
146+
// http://python.6.x6.nabble.com/Cleaning-up-PythonInterpreter-object-td1777184.html
147+
// Because Python is an interpreted language, when a Python script creates new
148+
// variables they stick around in a static org.python.core.PySystemState variable
149+
// (defaultSystemState) the org.python.core.Py class.
150+
// Thus an implicit static state is created by script evaluation, so we must manually
151+
// clean up local variables known to the interpreter when the scope of an executed
152+
// script is over.
153+
// See original bug report for the memory leak that prompted this solution:
154+
// http://fiji.sc/bugzilla/show_bug.cgi?id=1203
155+
final PyObject locals = interp.getLocals();
156+
for (final PyObject item : locals.__iter__().asIterable()) {
157+
final String localVar = item.toString();
158+
// Ignore __name__ and __doc__ variables, which are special and known not
159+
// to be local variables created by evaluation.
160+
if (!localVar.contains("__name__") && !localVar.contains("__doc__")) {
161+
// Build list of variables to clean
162+
scriptLocals.add(item.toString());
163+
}
164+
}
165+
// Null out local variables
166+
for (final String string : scriptLocals) {
167+
interp.set(string, null);
168+
}
169+
}
170+
}
171+
172+
}

‎src/main/java/org/scijava/plugins/scripting/jython/JythonScriptLanguage.java‎

Lines changed: 2 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,11 @@
3131

3232
package org.scijava.plugins.scripting.jython;
3333

34-
import java.lang.ref.PhantomReference;
35-
import java.lang.ref.ReferenceQueue;
36-
import java.util.ArrayList;
37-
import java.util.LinkedList;
38-
import java.util.List;
39-
4034
import javax.script.ScriptEngine;
4135

4236
import org.python.core.PyNone;
4337
import org.python.core.PyObject;
4438
import org.python.core.PyString;
45-
import org.python.util.PythonInterpreter;
4639
import org.scijava.log.LogService;
4740
import org.scijava.plugin.Parameter;
4841
import org.scijava.plugin.Plugin;
@@ -60,14 +53,7 @@
6053
@Plugin(type = ScriptLanguage.class, name = "Python")
6154
public class JythonScriptLanguage extends AdaptedScriptLanguage {
6255

63-
// List of PhantomReferences and corresponding ReferenceQueue to
64-
// facilitate proper PhantomReference use.
65-
// See http://resources.ej-technologies.com/jprofiler/help/doc/index.html
66-
67-
private final LinkedList<JythonEnginePhantomReference> phantomReferences =
68-
new LinkedList<JythonEnginePhantomReference>();
69-
private final ReferenceQueue<JythonScriptEngine> queue =
70-
new ReferenceQueue<JythonScriptEngine>();
56+
private JythonReferenceCleaner cleaner = new JythonReferenceCleaner();
7157

7258
@Parameter
7359
private ThreadService threadService;
@@ -83,64 +69,7 @@ public JythonScriptLanguage() {
8369
public ScriptEngine getScriptEngine() {
8470
// TODO: Consider adapting the wrapped ScriptEngineFactory's ScriptEngine.
8571
final JythonScriptEngine engine = new JythonScriptEngine();
86-
final LogService ls = logService;
87-
88-
synchronized (phantomReferences) {
89-
// NB: This phantom reference is used to clean up any local variables
90-
// created by evaluation of scripts via this ScriptEngine. We need
91-
// to use PhantomReferences because the "scope" of a script extends
92-
// beyond its eval method - a consumer may still want to inspect
93-
// the state of variables after evaluation.
94-
// By using PhantomReferences we are saying that the scope of
95-
// script evaluation equals the lifetime of the ScriptEngine.
96-
phantomReferences.add(new JythonEnginePhantomReference(engine, queue));
97-
98-
// NB: The use of PhantomReferences requires a paired polling thread.
99-
// We poll instead of blocking for input to avoid leaving lingering
100-
// threads that would need to be interrupted - instead only starting
101-
// threads running when there is actually a PhantomReference that
102-
// will eventually be enqueued.
103-
// Here we check if there is already a polling thread in operation -
104-
// if not, we start a new thread.
105-
if (phantomReferences.size() == 1) {
106-
threadService.run(new Runnable() {
107-
108-
@Override
109-
public void run() {
110-
boolean done = false;
111-
112-
// poll the queue
113-
while (!done) {
114-
try {
115-
Thread.sleep(100);
116-
117-
synchronized (phantomReferences) {
118-
// poll the queue
119-
JythonEnginePhantomReference ref =
120-
(JythonEnginePhantomReference) queue.poll();
121-
122-
// if we have a ref, clean it up
123-
if (ref != null) {
124-
ref.cleanup();
125-
phantomReferences.remove(ref);
126-
}
127-
128-
// Once we're done with our known phantom refs
129-
// we can let this thread shut down
130-
done = phantomReferences.size() == 0;
131-
}
132-
133-
}
134-
catch (final Exception ex) {
135-
// log exception, continue
136-
ls.error(ex);
137-
}
138-
}
139-
}
140-
});
141-
142-
}
143-
}
72+
cleaner.queueCleanup(engine, threadService, logService);
14473
return engine;
14574
}
14675

@@ -159,52 +88,4 @@ public Object decode(final Object object) {
15988
return object;
16089
}
16190

162-
/**
163-
* Helper class to clean up {@link PythonInterpreter} local variables when a
164-
* parent {@link JythonScriptEngine} leaves scope.
165-
*/
166-
private static class JythonEnginePhantomReference extends
167-
PhantomReference<JythonScriptEngine>
168-
{
169-
170-
public PythonInterpreter interpreter;
171-
172-
public JythonEnginePhantomReference(JythonScriptEngine engine,
173-
ReferenceQueue<JythonScriptEngine> queue)
174-
{
175-
super(engine, queue);
176-
interpreter = engine.interpreter;
177-
}
178-
179-
public void cleanup() {
180-
final List<String> scriptLocals = new ArrayList<String>();
181-
PythonInterpreter interp = interpreter;
182-
if (interp == null) return;
183-
184-
// NB: This method for cleaning up local variables was taken from:
185-
// http://python.6.x6.nabble.com/Cleaning-up-PythonInterpreter-object-td1777184.html
186-
// Because Python is an interpreted language, when a Python script creates new
187-
// variables they stick around in a static org.python.core.PySystemState variable
188-
// (defaultSystemState) the org.python.core.Py class.
189-
// Thus an implicit static state is created by script evaluation, so we must manually
190-
// clean up local variables known to the interpreter when the scope of an executed
191-
// script is over.
192-
// See original bug report for the memory leak that prompted this solution:
193-
// http://fiji.sc/bugzilla/show_bug.cgi?id=1203
194-
final PyObject locals = interp.getLocals();
195-
for (final PyObject item : locals.__iter__().asIterable()) {
196-
final String localVar = item.toString();
197-
// Ignore __name__ and __doc__ variables, which are special and known not
198-
// to be local variables created by evaluation.
199-
if (!localVar.contains("__name__") && !localVar.contains("__doc__")) {
200-
// Build list of variables to clean
201-
scriptLocals.add(item.toString());
202-
}
203-
}
204-
// Null out local variables
205-
for (final String string : scriptLocals) {
206-
interp.set(string, null);
207-
}
208-
}
209-
}
21091
}

0 commit comments

Comments
 (0)