Issue2026

classification
Title: Memory leak when instantiating multiple PyScriptEngine objects in the same thread
Type: Severity: normal
Components: Core Versions: Jython 2.7, Jython 2.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ducky427, fwierzbicki, janvh, zyasoft
Priority: Keywords:

Created on 2013-03-15.17:13:42 by janvh, last changed 2014-07-10.00:00:15 by zyasoft.

Messages
msg7935 (view) Author: Jan Van Hoecke (janvh) Date: 2013-03-15.17:22:50
We executing Jython scripts using a thread pool. And have noticed a memory leak when instantiating multiple instances of the PyScriptEngine in the same thread.

Versions tested: 2.5.3 / 2.5.4-rc1 / 2.7-b1 
All have the same issue.
The problem did not occur on 2.5.1. It is linked with the introduction of a ThreadLocal object in the PythonInterpreter.

The following code can be used to reproduce it:

public static void testScriptingLeakDetail() throws InterruptedException {
    ExecutorService pool = Executors.newSingleThreadExecutor();

    int i = 0;
    while (true) {
        i++;
        if (i % 10 == 0) {
            System.out.println("Iteration " + i);
        }

        pool.submit(new Runnable() {
            @Override
            public void run() {
                PyScriptEngineFactory factory = new PyScriptEngineFactory();
                PyScriptEngine engine = (PyScriptEngine) factory.getScriptEngine();
                try {
                    engine.eval("s = 'hello' + ' world'");
                } catch (ScriptException e) {
                }
            }
        });
        Thread.sleep(10);
    }
}

The leak seems to be linked to the factory.getScriptEngine() instantiating the PythonInterpreter using ThreadLocal objects. 
The ThreadLocalMaps of the threads in our Thread pool seems to be filling up with PyScriptEngineScope objects.
This would go unnoticed if the Threads would be created as required and disposed of afterwards, but because we are using a thread pool, the threads (and their thread local map) get recycled.

I have tried getting to the bottom of the problem, but so far have not found a solution.
The only fix I can suggest is making the protected ThreadLocal<PyObject> threadLocals static.
msg8673 (view) Author: Jim Baker (zyasoft) Date: 2014-06-18.17:49:52
Target beta 4
msg8771 (view) Author: Jim Baker (zyasoft) Date: 2014-06-19.22:39:29
So this has an easy solution. We simply need to remove the ThreadLocal in PythonInterpreter#cleanup. Here's the full diff:

diff -r 4de0eff57af5 src/org/python/jsr223/PyScriptEngine.java
--- a/src/org/python/jsr223/PyScriptEngine.java	Thu Jun 19 19:38:55 2014 +0100
+++ b/src/org/python/jsr223/PyScriptEngine.java	Thu Jun 19 16:37:03 2014 -0600
@@ -17,7 +17,7 @@
 import javax.script.SimpleBindings;
 import org.python.util.PythonInterpreter;
 
-public class PyScriptEngine extends AbstractScriptEngine implements Compilable, Invocable {
+public class PyScriptEngine extends AbstractScriptEngine implements Compilable, Invocable, AutoCloseable {
 
     private final PythonInterpreter interp;
     private final ScriptEngineFactory factory;
@@ -231,4 +231,8 @@
             return PyScriptEngine.this.eval(code, ctx);
         }
     }
+
+    public void close() {
+        interp.close();
+    }
 }
diff -r 4de0eff57af5 src/org/python/util/PythonInterpreter.java
--- a/src/org/python/util/PythonInterpreter.java	Thu Jun 19 19:38:55 2014 +0100
+++ b/src/org/python/util/PythonInterpreter.java	Thu Jun 19 16:37:03 2014 -0600
@@ -25,7 +25,7 @@
  * The PythonInterpreter class is a standard wrapper for a Jython interpreter
  * for embedding in a Java application.
  */
-public class PythonInterpreter {
+public class PythonInterpreter implements AutoCloseable {
 
     // Defaults if the interpreter uses thread-local state
     protected PySystemState systemState;
@@ -352,6 +352,11 @@
         } catch (PyException pye) {
             // fall through
         }
+        threadLocals.remove();
         sys.cleanup();
     }
+
+    public void close() {
+        cleanup();
+    }
 }

I modified your test program slightly to use the try-with-resource form introduced by Java 7:

import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import javax.script.ScriptException;
import org.python.jsr223.PyScriptEngine;
import org.python.jsr223.PyScriptEngineFactory;

public class exhaust {

    public static void main(String[] argv) {
	ExecutorService pool = Executors.newSingleThreadExecutor();

	int i = 0;
	while (true) {
	    i++;
	    if (i % 10 == 0) {
		System.out.println("Iteration " + i);
	    }

	    pool.submit(new Runnable() {
		    @Override
		    public void run() {
			PyScriptEngineFactory factory = new PyScriptEngineFactory();
			try (PyScriptEngine engine = (PyScriptEngine) factory.getScriptEngine()) {
			    engine.eval("s = 'hello' + ' world'");
			} catch (ScriptException e) {
			}
		    }
		});
            try {
		Thread.sleep(10);
	    } catch (InterruptedException e) {
		break;
	    }
	}
    }
}

Sorry it took so long for us to take a look!
msg8772 (view) Author: Rohit (ducky427) Date: 2014-06-19.22:52:12
Would this address the issue raised by Steve Yegge on the mailing list on 10th May:

http://thread.gmane.org/gmane.comp.lang.jython.user/10370
msg8773 (view) Author: Jim Baker (zyasoft) Date: 2014-06-19.23:01:14
I will add Closeable to the above interfaces implemented; also this adds to the documentation of the API since we expect such closing to be idempotent. Lastly, there's a small amount of work before this can be pushed into 2.7 trunk, mostly around idempotence.

Given that this is a memory leak, we should also backport this to 2.5.4.
msg8774 (view) Author: Jim Baker (zyasoft) Date: 2014-06-19.23:02:33
Rohit, indeed it seems very applicable. Thanks for pointing out this thread, given that somehow I managed to miss reading it.
msg8775 (view) Author: Rohit (ducky427) Date: 2014-06-19.23:25:47
Thanks a lot! I also had a similar issue.
msg8832 (view) Author: Jim Baker (zyasoft) Date: 2014-06-28.03:21:27
Fixed as of http://hg.python.org/jython/rev/d8233316cf09
History
Date User Action Args
2014-07-10 00:00:15zyasoftsetstatus: pending -> closed
2014-06-28 03:21:27zyasoftsetstatus: open -> pending
resolution: fixed
messages: + msg8832
2014-06-19 23:25:47ducky427setmessages: + msg8775
2014-06-19 23:02:33zyasoftsetmessages: + msg8774
2014-06-19 23:01:14zyasoftsetmessages: + msg8773
2014-06-19 22:52:12ducky427setnosy: + ducky427
messages: + msg8772
2014-06-19 22:39:29zyasoftsetmessages: + msg8771
2014-06-18 17:49:52zyasoftsetnosy: + zyasoft
messages: + msg8673
2013-03-15 19:17:38fwierzbickisetnosy: + fwierzbicki
2013-03-15 17:22:50janvhsetmessages: + msg7935
2013-03-15 17:13:42janvhcreate