Fix lifetime for pyExec results (#1540)

Currently if the result from pyExec is a PyProxy, it gets destroyed.
This switches to using `to_js` to handle this (it is better to use
than an explicit `create_proxy` since it automatically decides whether
to create a proxy or not).

I also added `destroyIfProxy` which checks if something is a `PyProxy`
and then destroys it. Each use of `pyExec` needs to call `destroyIfProxy`
on the result after it is done with it.
This commit is contained in:
Hood Chatham
2023-06-15 11:51:36 -07:00
committed by GitHub
parent f4936316ab
commit 79ad39260e
5 changed files with 62 additions and 3 deletions

View File

@@ -191,7 +191,7 @@ export function make_PyRepl(interpreter: InterpreterClient, app: PyScriptApp) {
pyReplTag: this,
result,
});
await interpreter._remote.destroyIfProxy(result);
this.autogenerateMaybe();
}

View File

@@ -36,6 +36,7 @@ export function make_PyScript(interpreter: InterpreterClient, app: PyScriptApp)
await app.plugins.beforePyScriptExec({ interpreter, src, pyScriptTag });
const { result } = await pyExec(interpreter, src, pyScriptTag);
await app.plugins.afterPyScriptExec({ interpreter, src, pyScriptTag, result });
await interpreter._remote.destroyIfProxy(result);
} finally {
releaseLock();
app.decrementPendingTags();

View File

@@ -4,7 +4,7 @@ from contextlib import contextmanager
from js import Object
from pyodide.code import eval_code
from pyodide.ffi import JsProxy
from pyodide.ffi import JsProxy, to_js
from ._event_loop import (
defer_user_asyncio,
@@ -103,7 +103,7 @@ def run_pyscript(code: str, id: str = None) -> JsProxy:
with display_target(id), defer_user_asyncio():
result = eval_code(code, globals=__main__.__dict__)
return Object.new(result=result)
return to_js({"result": result}, depth=1, dict_converter=Object.fromEntries)
__all__ = [

View File

@@ -274,6 +274,20 @@ export class RemoteInterpreter extends Object {
this.FS.writeFile(path, data, { canOwn: true });
}
destroyIfProxy(px: any): void {
if (this.interface.ffi) {
// Pyodide 0.23
if (px instanceof this.interface.ffi.PyProxy) {
px.destroy();
}
} else {
// older Pyodide
if (this.interface.isPyProxy(px)) {
px.destroy();
}
}
}
/**
* delegates clearing importlib's module path
* caches to the underlying interface

View File

@@ -674,3 +674,47 @@ class TestPyRepl(PyScriptTest):
"Are your filename and path correct?"
)
assert self.console.error.lines[-1] == errorMsg
@skip_worker("dont-care")
def test_repl_results(self):
self.writefile("loadReplSrc2.py", "2")
self.writefile("loadReplSrc3.py", "print('3')")
self.pyscript_run(
"""
<py-repl id="py-repl1" output="out1">
42
</py-repl>
<div id="out1"></div>
<py-repl id="py-repl2" output="out2">
c = [1,2,3]
from sys import getrefcount
# should print 2: 1 from the reference c and 1 since getrefcount
# holds a reference to its argument
print(getrefcount(c))
c
</py-repl>
<div id="out2"></div>
<py-repl id="py-repl3" output="out3">
# should also print 2: if it prints 3 that would mean that c was not properly
# released by py-repl
getrefcount(c)
</py-repl>
<div id="out3"></div>
"""
)
py_repl1 = self.page.locator("py-repl#py-repl1")
py_repl1.locator("button").click()
py_repl2 = self.page.locator("py-repl#py-repl2")
py_repl2.locator("button").click()
py_repl3 = self.page.locator("py-repl#py-repl3")
py_repl3.locator("button").click()
assert self.page.wait_for_selector("#out1").inner_text() == "42"
assert self.page.wait_for_selector("#out2").inner_text() == "2\n\n[1, 2, 3]"
# Check that c was released
assert self.page.wait_for_selector("#out3").inner_text() == "2"